Feat/datadog js monitoring #483
Conversation
The `APP_VERSION` needs to be the same for in the javascript initialisation AND the command line tool that uploads the sourcemaps into datadog. Using an evironment var allows us to set the `APP_VERSION` variable in the build process, use it in the command line and pass it into the app js.
davidsauntson
force-pushed
the
feat/datadog-js-monitoring
branch
from
4b996fd to
5b0a663
Compare
| @@ -10,7 +10,8 @@ image: | |||
| tag: "main" | |||
|
|
|||
| # plain env vars. Take precedence over any secrets with the same name! | |||
| envVars: {} | |||
| envVars: | |||
| APP_VERSION: ${{ github.sha }} | |||
There was a problem hiding this comment.
I wonder if it would be helpful for this to match the backend app version? That uses the Docker image tag (which I think is the image hash?) rather than the GitHub SHA:
TBH, I hardly ever use the version information in Datadog, so might not be worth the effort of making them match if it's hard.
There was a problem hiding this comment.
Hmm, I'm a bit confused as to what this would resolved to - AFAICT it's the helm chart version. Would this exist at the point the datadog upload command is run?
There was a problem hiding this comment.
Example Datadog trace. It has a version tag of bbd79ab1fa0ddc974590d31466119d7a58d3b52e, which matches the Docker image tag for the current latest image.
The Datadog upload step happens after the dev deployment, so the image tag should be available somewhere in GHA.
Alternatively, we could use the git SHA in both places? I think either one (Docker hash or git SHA) would be helpful if you're trying to work out which version of the code is responsible for a specific trace.
| import { datadogLogs } from '@datadog/browser-logs' | ||
|
|
||
| const initDatadog = () => { | ||
| const ddEnv = window.location.hostname === 'www.citizensadvice.org.uk' ? "prod" : "dev"; |
There was a problem hiding this comment.
Does this app have review environments? I'm wondering if it would be better to get the environment from the DOM (and ultimately an environment variable) rather than hardcoding the prod URL like this.
There was a problem hiding this comment.
Yep I could make it an env var like the version number. I hadn't thought to upload the files for review apps, at the moment it would only run on successful deployment to qa and prod
Actually I think this will require a rethink of the workflow structure, because the datadog env is set using matrix.stage for qa and prod builds. This doesn't exist during the new datadog upload workflow because it's run after both have completed (deploy to qa and prod are in the same workflow).
app/javascript/modules/datadog.js
Outdated
|
|
||
| if (ddEnv && appVersion) { | ||
| datadogLogs.init({ | ||
| clientToken: 'pub32a7538113d7910e4304b8aa13ff4521', |
There was a problem hiding this comment.
Could you add a comment explaining that this is designed to be a public token - it's not a security issue to have this in the code?
Are client tokens linked domain name or something? What stops someone from spamming our logs? 🤔
There was a problem hiding this comment.
Yep I can add a comment.
It looks like datadog are aware this could be used to send spam data - DataDog/browser-sdk#853 and DataDog/browser-sdk#853 (comment)
I'm not sure there is a way around this really? Could we build smth that forwards errors to our own endpoint and maybe use the DD API to create logs? Then we'd have more control over what ends up in the logs and from where, but I'm not sure it's worth the effort?
There was a problem hiding this comment.
Discussed on Slack 👍
Thanks! I've added some responses. I'd like to check the datadog command and parameters work in the I'd like to propose:
Is that ok? The spamming question remains but I can't see an easy fix that isn't a mini-project in its own right. I might be missing something tho. |
To upload the js and sourcemaps to datadog, we need an
APP_VERSIONthat is consistent across the CI run and available to the app itself, so it can be sent along with the errors raised in the client side js. To do this, I have:APP_VERSIONand set it to the commit shametatag in the app--release-versionflag in thedatadog uploadcommandI've also turned off deployment to prod whilst I test this by raising the deliberate error in datadog (I don't want to have console errors in prod during testing).