feat(remix): Propagate the server trace to Remix 3 page loads

dev-packages/e2e-tests/test-applications/remix-v3/tests/client.test.ts

@@ -15,6 +15,25 @@ test('sends a pageload span', async ({ page }) => {
   expect(span.attributes?.['sentry.segment.name.source']?.value).toBe('route');
 });
 
+test('continues the server trace on a page load', async ({ page }) => {
+  const serverSpanPromise = waitForStreamedSpan(
+    APP_NAME,
+    span =>
+      getSpanOp(span) === 'http.server' && span.is_segment === true && span.attributes?.['http.route']?.value === '/',
+  );
+  const pageloadPromise = waitForStreamedSpan(
+    APP_NAME,
+    span => getSpanOp(span) === 'pageload' && span.is_segment === true,
+  );
+
+  await page.goto('/');
+
+  const [serverSpan, pageload] = await Promise.all([serverSpanPromise, pageloadPromise]);
+  // The document's response carried the trace in `Server-Timing`, which the browser SDK picks up.
+  expect(pageload.trace_id).toBe(serverSpan.trace_id);
+  expect(pageload.parent_span_id).toBe(serverSpan.span_id);
+});
+
 test('parameterizes a page load on a route with params', async ({ page }) => {
   const spanPromise = waitForStreamedSpan(APP_NAME, span => getSpanOp(span) === 'pageload' && span.is_segment === true);
 

packages/remix/src/v3/server/middleware.ts

@@ -4,6 +4,7 @@ import {
   getIsolationScope,
   getRootSpan,
   getSpanStatusFromHttpCode,
+  getTraceData,
   INTERNAL_setSegmentNameSourceIfSegment,
   type Scope,
   updateSpanName,
@@ -44,9 +45,7 @@ export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike {
     }
 
     setResponseStatus(response);
-    if (route) {
-      addRouteHeader(response, context.request, route);
-    }
+    addServerTimingHeaders(response, context.request, route);
 
     return response;
   };
@@ -70,19 +69,96 @@ function applyRoute(isolationScope: Scope, route: string, method: string): void
 }
 
 /**
- * Tells the browser SDK which route served a document, so it can name its page load and navigation
- * spans after the pattern. The browser has no route table of its own. Only HTML responses carry it,
- * which is what document loads and the runtime's frame fetches ask for.
+ * What the browser SDK needs from the server on a document: the trace to continue, so a page load
+ * joins the request's trace, and the route that served it, so spans are named after the pattern.
+ * The browser reads both off the navigation timing entry. Only HTML responses carry them, which is
+ * what document loads and the runtime's frame fetches ask for.
  */
-function addRouteHeader(response: Response, request: Request, route: string): void {
+function addServerTimingHeaders(response: Response, request: Request, route: string | undefined): void {
   if (!request.headers.get('accept')?.includes('text/html')) {
     return;
   }
-  try {
-    response.headers.append('Server-Timing', formatRouteTiming(route));
-  } catch {
-    // Immutable headers, e.g. a response passed through from `fetch()`.
+
+  const entries: string[] = [];
+  // A shared cache would hand this request's trace to every later page load, so a cacheable response
+  // carries the route only. The route is the same for every request to it.
+  if (!isSharedCacheable(response)) {
+    const traceData = getTraceData();
+    if (traceData['sentry-trace']) {
+      entries.push(`sentry-trace;desc="${traceData['sentry-trace']}"`);
+    }
+    if (traceData.baggage) {
+      entries.push(`baggage;desc="${traceData.baggage}"`);
+    }
+  }
+  if (route) {
+    entries.push(formatRouteTiming(route));
+  }
+
+  for (const entry of entries) {
+    try {
+      response.headers.append('Server-Timing', entry);
+    } catch {
+      // Immutable headers, e.g. a response passed through from `fetch()`.
+      return;
+    }
+  }
+}
+
+// Cache-control fields a CDN reads instead of `Cache-Control`.
+const CDN_CACHE_CONTROL_HEADERS = [
+  'cdn-cache-control',
+  'cloudflare-cdn-cache-control',
+  'vercel-cdn-cache-control',
+  'surrogate-control',
+];
+
+/**
+ * Whether a cache in front of the app may store this response and serve it to other users.
+ *
+ * A CDN that reads a targeted field ignores `Cache-Control`; a cache without targeted support reads
+ * `Cache-Control`. Any of them storing the response is enough.
+ */
+function isSharedCacheable(response: Response): boolean {
+  for (const name of CDN_CACHE_CONTROL_HEADERS) {
+    const value = response.headers.get(name);
+    if (value !== null && sharedCachingVerdict(value.toLowerCase()) === true) {
+      return true;
+    }
+  }
+
+  const verdict = sharedCachingVerdict(response.headers.get('cache-control')?.toLowerCase() ?? '');
+  if (verdict !== undefined) {
+    return verdict;
+  }
+  // Without a lifetime in `Cache-Control`, a cache falls back to `Expires`.
+  const expires = response.headers.get('expires');
+  return expires !== null && Date.parse(expires) > Date.now();
+}
+
+/** Whether these directives let a shared cache reuse the response, or `undefined` when they say nothing. */
+function sharedCachingVerdict(directives: string): boolean | undefined {
+  if (/\b(?:no-store|private)\b/.test(directives)) {
+    return false;
+  }
+  // A stale copy is served even after a zero lifetime.
+  if (/\b(?:stale-while-revalidate|stale-if-error)\s*=\s*[1-9]/.test(directives)) {
+    return true;
+  }
+  // `s-maxage` overrides `max-age` for shared caches. A zero lifetime means the cache revalidates
+  // every time, so it never serves this response to anyone else.
+  const sharedMaxAge = directives.match(/\bs-maxage\s*=\s*(\d+)/)?.[1];
+  if (sharedMaxAge !== undefined) {
+    return Number(sharedMaxAge) > 0;
+  }
+  if (/\bpublic\b/.test(directives)) {
+    return true;
+  }
+  const maxAge = directives.match(/\bmax-age\s*=\s*(\d+)/)?.[1];
+  if (maxAge !== undefined) {
+    return Number(maxAge) > 0;
   }
+  return undefined;
 }
 
 function setResponseStatus(response: Response): void {

packages/remix/test/v3/middleware.test.ts

@@ -1,4 +1,13 @@
-import { describe, expect, it } from 'vitest';
+import type * as SentryCore from '@sentry/core';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+// Trace data as the SDK would produce it for the active request; empty when nothing is active.
+const getTraceData = vi.hoisted(() => vi.fn<() => { 'sentry-trace'?: string; baggage?: string }>(() => ({})));
+
+vi.mock('@sentry/core', async importOriginal => ({
+  ...(await importOriginal<typeof SentryCore>()),
+  getTraceData,
+}));
 
 import { sentryRemixMiddleware } from '../../src/v3/server/middleware';
 import type { MatcherLike, RequestContextLike } from '../../src/v3/types';
@@ -16,6 +25,146 @@ function contextFor(url: string, headers: Record<string, string> = {}): RequestC
 }
 
 describe('sentryRemixMiddleware', () => {
+  beforeEach(() => {
+    getTraceData.mockReturnValue({});
+  });
+
+  it('propagates the trace to HTML responses through Server-Timing, ahead of the route', async () => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1', baggage: 'sentry-trace_id=abc' });
+    const middleware = sentryRemixMiddleware(matcherFor('/users/:id'));
+
+    const response = await middleware(
+      contextFor('http://x/users/1', { accept: 'text/html' }),
+      async () => new Response(''),
+    );
+
+    expect(response.headers.get('server-timing')).toBe(
+      'sentry-trace;desc="abc-def-1", baggage;desc="sentry-trace_id=abc", sentry-route;desc="/users/:id"',
+    );
+  });
+
+  it('propagates the trace even when no route matched', async () => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1' });
+    const middleware = sentryRemixMiddleware(matcherFor(undefined));
+
+    const response = await middleware(
+      contextFor('http://x/nope', { accept: 'text/html' }),
+      async () => new Response(''),
+    );
+
+    expect(response.headers.get('server-timing')).toBe('sentry-trace;desc="abc-def-1"');
+  });
+
+  it.each([
+    ['public', 'public, max-age=60'],
+    ['s-maxage', 's-maxage=300'],
+    ['max-age', 'max-age=60'],
+  ])('leaves the trace off a response a shared cache may store (%s), keeping the route', async (_why, cacheControl) => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1', baggage: 'sentry-trace_id=abc' });
+    const middleware = sentryRemixMiddleware(matcherFor('/'));
+
+    const response = await middleware(
+      contextFor('http://x/', { accept: 'text/html' }),
+      async () => new Response('', { headers: { 'cache-control': cacheControl } }),
+    );
+
+    expect(response.headers.get('server-timing')).toBe('sentry-route;desc="/"');
+  });
+
+  it.each([
+    ['stale-while-revalidate after a zero lifetime', { 'cache-control': 'max-age=0, stale-while-revalidate=60' }],
+    ['stale-if-error after a zero shared lifetime', { 'cache-control': 's-maxage=0, stale-if-error=600' }],
+    [
+      'CDN-Cache-Control over a private Cache-Control',
+      { 'cache-control': 'private', 'cdn-cache-control': 'max-age=3600' },
+    ],
+    [
+      'Vercel-CDN-Cache-Control over no-store',
+      { 'cache-control': 'no-store', 'vercel-cdn-cache-control': 's-maxage=3600' },
+    ],
+    ['Cloudflare-CDN-Cache-Control', { 'cloudflare-cdn-cache-control': 'public, max-age=60' }],
+    ['Surrogate-Control', { 'surrogate-control': 'max-age=3600' }],
+  ])('leaves the trace off a response a CDN or stale-serving cache may reuse (%s)', async (_why, headers) => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1' });
+    const middleware = sentryRemixMiddleware(matcherFor('/'));
+
+    const response = await middleware(
+      contextFor('http://x/', { accept: 'text/html' }),
+      async () => new Response('', { headers }),
+    );
+
+    expect(response.headers.get('server-timing')).toBe('sentry-route;desc="/"');
+  });
+
+  it('propagates the trace when the CDN field forbids caching and nothing else allows it', async () => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1' });
+    const middleware = sentryRemixMiddleware(matcherFor('/'));
+
+    const response = await middleware(
+      contextFor('http://x/', { accept: 'text/html' }),
+      async () => new Response('', { headers: { 'cdn-cache-control': 'max-age=0' } }),
+    );
+
+    expect(response.headers.get('server-timing')).toContain('sentry-trace;desc="abc-def-1"');
+  });
+
+  it('leaves the trace off a response with a future Expires and no Cache-Control', async () => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1' });
+    const middleware = sentryRemixMiddleware(matcherFor('/'));
+
+    const response = await middleware(
+      contextFor('http://x/', { accept: 'text/html' }),
+      async () => new Response('', { headers: { expires: new Date(Date.now() + 60_000).toUTCString() } }),
+    );
+
+    expect(response.headers.get('server-timing')).toBe('sentry-route;desc="/"');
+  });
+
+  it('propagates the trace when max-age=0 overrides a future Expires', async () => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1' });
+    const middleware = sentryRemixMiddleware(matcherFor('/'));
+
+    const response = await middleware(
+      contextFor('http://x/', { accept: 'text/html' }),
+      async () =>
+        new Response('', {
+          headers: { 'cache-control': 'max-age=0', expires: new Date(Date.now() + 60_000).toUTCString() },
+        }),
+    );
+
+    expect(response.headers.get('server-timing')).toContain('sentry-trace;desc="abc-def-1"');
+  });
+
+  it('propagates the trace when Expires is in the past', async () => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1' });
+    const middleware = sentryRemixMiddleware(matcherFor('/'));
+
+    const response = await middleware(
+      contextFor('http://x/', { accept: 'text/html' }),
+      async () => new Response('', { headers: { expires: 'Thu, 01 Jan 1970 00:00:00 GMT' } }),
+    );
+
+    expect(response.headers.get('server-timing')).toContain('sentry-trace;desc="abc-def-1"');
+  });
+
+  it.each([
+    ['private', 'private, max-age=60'],
+    ['no-store', 'no-store'],
+    ['max-age=0', 'max-age=0, must-revalidate'],
+    ['s-maxage=0', 'max-age=3600, s-maxage=0'],
+    ['no cache header', undefined],
+  ])('propagates the trace on a response only this user can get back (%s)', async (_why, cacheControl) => {
+    getTraceData.mockReturnValue({ 'sentry-trace': 'abc-def-1' });
+    const middleware = sentryRemixMiddleware(matcherFor('/'));
+
+    const response = await middleware(
+      contextFor('http://x/', { accept: 'text/html' }),
+      async () => new Response('', { headers: cacheControl ? { 'cache-control': cacheControl } : {} }),
+    );
+
+    expect(response.headers.get('server-timing')).toContain('sentry-trace;desc="abc-def-1"');
+  });
+
   it('reports the route on HTML responses through Server-Timing', async () => {
     const middleware = sentryRemixMiddleware(matcherFor('/users/:id'));
 
@@ -60,7 +209,7 @@ describe('sentryRemixMiddleware', () => {
     expect(response.headers.has('server-timing')).toBe(false);
   });
 
-  it('adds nothing when no route matched', async () => {
+  it('adds nothing when no route matched and no trace is active', async () => {
     const middleware = sentryRemixMiddleware(matcherFor(undefined));
 
     const response = await middleware(