diff --git a/other/Dockerfile b/other/Dockerfile index ab91676..9ccf309 100644 --- a/other/Dockerfile +++ b/other/Dockerfile @@ -76,6 +76,14 @@ RUN --mount=type=secret,id=SENTRY_AUTH_TOKEN,required=true \ # Finally, build the production image with minimal footprint FROM base +# `ARG` does not cross a `FROM`, so the value the build stage declared has to be +# redeclared here or the running container has no idea which commit it is. It is +# the release name the server SDK tags its events with (server/utils/monitoring.ts), +# so without it every server-side event arrives with no release and cannot be +# correlated to a deploy. See #15. +ARG COMMIT_SHA +ENV COMMIT_SHA=$COMMIT_SHA + ENV FLY="true" ENV INTERNAL_PORT="8080" ENV PORT="8081" diff --git a/server/utils/monitoring.ts b/server/utils/monitoring.ts index 8b22705..0c21f9a 100644 --- a/server/utils/monitoring.ts +++ b/server/utils/monitoring.ts @@ -5,6 +5,18 @@ export function init() { Sentry.init({ dsn: process.env.SENTRY_DSN, environment: process.env.NODE_ENV, + // Set explicitly rather than left to the bundler plugin's injected global. + // That injection only reaches the client bundle — there is no global scope + // in Node for the plugin to write to, and @sentry/node has no + // SENTRY_RELEASE env fallback of its own (grep the package: no hits). So + // without this line the server sends events with no release at all, and + // since ~99% of this project's error volume is server-side, that is + // nearly all of it. See #15. + // + // COMMIT_SHA is declared as an ARG/ENV in *both* stages of other/Dockerfile; + // ARG does not cross a FROM, so the build-stage declaration alone leaves + // this undefined at runtime. + release: process.env.COMMIT_SHA, tracesSampleRate: process.env.NODE_ENV === 'production' ? 1 : 0, denyUrls: [ /\/resources\/healthcheck/, diff --git a/tests/sentry-release.test.ts b/tests/sentry-release.test.ts index 9c5ab92..3b0662c 100644 --- a/tests/sentry-release.test.ts +++ b/tests/sentry-release.test.ts @@ -53,6 +53,25 @@ const BUILD_ONLY = [ const read = (path: string) => readFileSync(path, 'utf8') +/** + * The client gets `SENTRY_RELEASE` injected into its bundle by the Sentry plugin; + * the server gets nothing. There is no global scope in Node for the plugin to + * write to, and @sentry/node has no SENTRY_RELEASE env fallback of its own, so + * the release name has to be handed to the server SDK explicitly. + * + * Mocked rather than asserted with a regex over monitoring.ts, because `init()` + * is callable and the thing worth protecting is the value that reaches the SDK — + * not the text of the line that sets it. + */ +const { sentry } = vi.hoisted(() => ({ sentry: { init: vi.fn() } })) + +vi.mock('@sentry/node', () => ({ + init: (...args: unknown[]) => sentry.init(...args), + // Named in `integrations` by server/utils/monitoring.ts, so the mock has to + // satisfy it even though this file never inspects it. + httpIntegration: () => ({ name: 'Http' }), +})) + /** Every distinct capture of `pattern` in `source`, sorted. */ function matchAll(source: string, pattern: RegExp) { return [ @@ -83,6 +102,25 @@ function buildStage() { return dockerfile.slice(start, end) } +/** + * The final stage of the Dockerfile — the image that actually runs in + * production, as opposed to the `as build` stage that only produces it. + * + * This is where `COMMIT_SHA` has to be *redeclared*. `ARG` does not cross a + * `FROM`, so the build stage's declaration says nothing about what ends up in the + * running container, which is why the server had no release for two years while + * the client bundle looked perfectly correct. + */ +function runtimeStage() { + const dockerfile = read(DOCKERFILE) + const start = dockerfile.indexOf('\nFROM base\n') + expect( + start, + `could not find the final stage in ${DOCKERFILE}`, + ).toBeGreaterThan(-1) + return dockerfile.slice(start) +} + /** * Every build input `getSentryPlugin()` reads, as named in `vite.config.ts`. * @@ -230,6 +268,31 @@ describe('Sentry release management build inputs', () => { }) }) +describe('the release reaches the running server', () => { + it('redeclares COMMIT_SHA in the runtime stage, since ARG does not cross FROM', () => { + const stage = runtimeStage() + // Both lines: `ARG` alone makes it a build-time-only value, and `ENV` is + // what the running process actually reads. + expect(stage).toMatch(/^ARG COMMIT_SHA$/m) + expect(stage).toMatch(/^ENV COMMIT_SHA=\$COMMIT_SHA$/m) + }) + + it('tags the SDK with the release, so server events are attributable', async () => { + const commit = 'a'.repeat(40) + vi.stubEnv('COMMIT_SHA', commit) + vi.stubEnv('NODE_ENV', 'production') + vi.stubEnv('SENTRY_DSN', 'https://abc@o0.ingest.sentry.io/0') + + const { init } = await import( + /* @vite-ignore */ '../server/utils/monitoring.ts' + ) + init() + + const [options] = sentry.init.mock.calls[0] as [Record] + expect(options.release).toBe(commit) + }) +}) + describe('Sentry credentials at the build/runtime boundary', () => { // #16 removes the three build-only secrets from the Fly *runtime* // environment once the build is proven. That is a one-line, unreviewable