fix incorrect getTraceData usage as well as fix a bug in propagator

Author: Lms24

dev-packages/e2e-tests/test-applications/remix-server-timing/app/entry.client.tsx

@@ -14,13 +14,13 @@ declare global {
   }
 }
 
-import { RemixBrowser, useLocation, useMatches } from '@remix-run/react';
-import * as Sentry from '@sentry/remix';
-import { StrictMode, startTransition, useEffect } from 'react';
-import { hydrateRoot } from 'react-dom/client';
+import { RemixBrowser, useLocation, useMatches } from "@remix-run/react";
+import * as Sentry from "@sentry/remix";
+import { StrictMode, startTransition, useEffect } from "react";
+import { hydrateRoot } from "react-dom/client";
 
 Sentry.init({
-  environment: 'qa', // dynamic sampling bias to keep transactions
+  environment: "qa", // dynamic sampling bias to keep transactions
   dsn: window.ENV.SENTRY_DSN,
   integrations: [
     Sentry.browserTracingIntegration({
@@ -31,7 +31,7 @@ Sentry.init({
   ],
   // Performance Monitoring
   tracesSampleRate: 1.0, // Capture 100% of the transactions
-  tunnel: 'http://localhost:3031/', // proxy server
+  tunnel: "http://localhost:3031/", // proxy server
 });
 
 startTransition(() => {

dev-packages/e2e-tests/test-applications/remix-server-timing/tests/server-timing-trace-propagation.test.ts

@@ -12,6 +12,10 @@ test('propagates trace context from server-timing header to client pageload', as
     return transactionEvent.contexts?.trace?.op === 'pageload' && transactionEvent.tags?.['sentry_test'] === testTag;
   });
 
+  const httpServerTransactionPromise = waitForTransaction('remix-server-timing', transactionEvent => {
+    return transactionEvent.contexts?.trace?.op === 'http.server';
+  });
+
   await page.goto(`/?tag=${testTag}`);
 
   const response = await responsePromise;
@@ -30,25 +34,46 @@ test('propagates trace context from server-timing header to client pageload', as
   expect(headerSampled).toBe('1');
 
   const pageloadTransaction = await pageLoadTransactionPromise;
+  const httpServerTransaction = await httpServerTransactionPromise;
 
   expect(pageloadTransaction).toBeDefined();
   expect(pageloadTransaction.transaction).toBe('/');
 
+  expect(httpServerTransaction.transaction).toMatch(/^GET http:\/\/localhost:\d+\/$/);
+
   expect(pageloadTransaction.contexts?.trace?.trace_id).toEqual(headerTraceId);
   expect(pageloadTransaction.contexts?.trace?.parent_span_id).toEqual(headerSpanId);
+
+  expect(httpServerTransaction.contexts?.trace?.trace_id).toEqual(headerTraceId);
+  expect(httpServerTransaction.contexts?.trace?.span_id).toEqual(headerSpanId);
 });
 
 test('includes server-timing header on redirect responses', async ({ page }) => {
   const redirectResponsePromise = page.waitForResponse(response => response.url().includes('/redirect-test'));
+  const redirectedPageloadResponsePromise = page.waitForResponse(response =>
+    response.url().includes('/user/redirected'),
+  );
+
+  const pageLoadTransactionPromise = waitForTransaction('remix-server-timing', transactionEvent => {
+    return transactionEvent.contexts?.trace?.op === 'pageload';
+  });
 
   await page.goto('/redirect-test');
 
   const redirectResponse = await redirectResponsePromise;
-  const serverTimingHeader = redirectResponse.headers()['server-timing'];
+  const redirectServerTimingHeader = redirectResponse.headers()['server-timing'];
 
-  expect(serverTimingHeader).toBeDefined();
-  expect(serverTimingHeader).toContain('sentry-trace');
+  expect(redirectServerTimingHeader).toBeDefined();
+  expect(redirectServerTimingHeader).toContain('sentry-trace');
+  expect(redirectServerTimingHeader).toContain('baggage');
 
+  const redirectSentryTraceMatch = redirectServerTimingHeader?.match(/sentry-trace;desc="([^"]+)"/);
+  expect(redirectSentryTraceMatch).toBeTruthy();
+  expect(redirectSentryTraceMatch![1]).toMatch(/[a-f0-9]{32}-[a-f0-9]{16}-1/);
+
+  const redirectedPageloadResponse = await redirectedPageloadResponsePromise;
+
+  const serverTimingHeader = redirectedPageloadResponse.headers()['server-timing'];
   const sentryTraceMatch = serverTimingHeader?.match(/sentry-trace;desc="([^"]+)"/);
   expect(sentryTraceMatch).toBeTruthy();
   const [traceId, spanId] = sentryTraceMatch![1].split('-');
@@ -57,6 +82,11 @@ test('includes server-timing header on redirect responses', async ({ page }) =>
 
   await page.waitForURL(/\/user\/redirected/);
   await expect(page.locator('h1')).toContainText('User redirected');
+
+  const pageLoadTransaction = await pageLoadTransactionPromise;
+  expect(pageLoadTransaction.transaction).toBe('/user/:id');
+  expect(pageLoadTransaction.contexts?.trace?.trace_id).toEqual(traceId);
+  expect(pageLoadTransaction.contexts?.trace?.parent_span_id).toEqual(spanId);
 });
 
 test('excludes server-timing header from client-side navigation data fetches', async ({ page }) => {

packages/opentelemetry/src/utils/getTraceData.ts

@@ -24,7 +24,7 @@ export function getTraceData({
   if (span) {
     const { scope } = getCapturedScopesOnSpan(span);
     // fall back to current context if for whatever reason we can't find the one of the span
-    ctx = (scope && getContextFromScope(scope)) || api.trace.setSpan(api.context.active(), span);
+    ctx = api.trace.setSpan(api.context.active(), span) || (scope && getContextFromScope(scope));
   }
 
   const { traceId, spanId, sampled, dynamicSamplingContext } = getInjectionData(ctx, { scope, client });

packages/remix/src/server/serverTimingTracePropagation.ts

@@ -1,13 +1,5 @@
 import type { Span } from '@sentry/core';
-import {
-  debug,
-  getActiveSpan,
-  getRootSpan,
-  getTraceData,
-  isNodeEnv,
-  spanToBaggageHeader,
-  spanToTraceHeader,
-} from '@sentry/core';
+import { debug, getTraceData, isNodeEnv } from '@sentry/core';
 import { DEBUG_BUILD } from '../utils/debug-build';
 import { isCloudflareEnv } from '../utils/utils';
 
@@ -18,25 +10,9 @@ export function generateSentryServerTimingHeader(span?: Span): string | null {
   }
 
   try {
-    let resolvedSpan = span;
-    if (!resolvedSpan) {
-      const activeSpan = getActiveSpan();
-      if (activeSpan) {
-        resolvedSpan = getRootSpan(activeSpan);
-      }
-    }
-
-    let sentryTrace: string | undefined;
-    let baggage: string | undefined;
-
-    if (resolvedSpan) {
-      sentryTrace = spanToTraceHeader(resolvedSpan);
-      baggage = spanToBaggageHeader(resolvedSpan);
-    } else {
-      const traceData = getTraceData();
-      sentryTrace = traceData['sentry-trace'];
-      baggage = traceData.baggage;
-    }
+    const traceData = getTraceData({ span });
+    const sentryTrace = traceData['sentry-trace'];
+    const baggage = traceData.baggage;
 
     if (!sentryTrace) {
       return null;