feat(node-experimental): Use new Propagator for OTEL Spans (#9214)

This fixes the propagator for node-experimental.

The "old" propagator from opentelemetry-node relies on having a
transaction for the active span etc, which we don't have anymore. So the
propagator never attached the correct stuff etc.

So now there is a new propagator for node-experimental:

* Instead of keeping DSC & TraceparentData on the OTEL Context, we just
keep the PropagationContext (which includes the DSC...)
* Add an E2E test to make sure we attach the outgoing header correctly

Author: mydea

packages/e2e-tests/test-applications/node-experimental-fastify-app/src/app.js

@@ -3,6 +3,7 @@ require('./tracing');
 const Sentry = require('@sentry/node-experimental');
 const { fastify } = require('fastify');
 const fastifyPlugin = require('fastify-plugin');
+const http = require('http');
 
 const FastifySentry = fastifyPlugin(async (fastify, options) => {
   fastify.decorateRequest('_sentryContext', null);
@@ -25,14 +26,24 @@ app.get('/test-param/:param', function (req, res) {
   res.send({ paramWas: req.params.param });
 });
 
+app.get('/test-inbound-headers', function (req, res) {
+  const headers = req.headers;
+
+  res.send({ headers });
+});
+
+app.get('/test-outgoing-http', async function (req, res) {
+  const data = await makeHttpRequest('http://localhost:3030/test-inbound-headers');
+
+  res.send(data);
+});
+
 app.get('/test-transaction', async function (req, res) {
   Sentry.startSpan({ name: 'test-span' }, () => {
     Sentry.startSpan({ name: 'child-span' }, () => {});
   });
 
-  res.send({
-    transactionIds: global.transactionIds || [],
-  });
+  res.send({});
 });
 
 app.get('/test-error', async function (req, res) {
@@ -45,16 +56,20 @@ app.get('/test-error', async function (req, res) {
 
 app.listen({ port: port });
 
-Sentry.addGlobalEventProcessor(event => {
-  global.transactionIds = global.transactionIds || [];
-
-  if (event.type === 'transaction') {
-    const eventId = event.event_id;
-
-    if (eventId) {
-      global.transactionIds.push(eventId);
-    }
-  }
-
-  return event;
-});
+function makeHttpRequest(url) {
+  return new Promise(resolve => {
+    const data = [];
+
+    http
+      .request(url, httpRes => {
+        httpRes.on('data', chunk => {
+          data.push(chunk);
+        });
+        httpRes.on('end', () => {
+          const json = JSON.parse(Buffer.concat(data).toString());
+          resolve(json);
+        });
+      })
+      .end();
+  });
+}

packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts

@@ -0,0 +1,100 @@
+import { test, expect } from '@playwright/test';
+import { Span } from '@sentry/types';
+import axios from 'axios';
+import { waitForTransaction } from '../event-proxy-server';
+
+const authToken = process.env.E2E_TEST_AUTH_TOKEN;
+const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG;
+const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT;
+const EVENT_POLLING_TIMEOUT = 30_000;
+
+test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
+  const inboundTransactionPromise = waitForTransaction('node-experimental-fastify-app', transactionEvent => {
+    return (
+      transactionEvent?.contexts?.trace?.op === 'http.server' &&
+      transactionEvent?.transaction === 'GET /test-inbound-headers'
+    );
+  });
+
+  const outboundTransactionPromise = waitForTransaction('node-experimental-fastify-app', transactionEvent => {
+    return (
+      transactionEvent?.contexts?.trace?.op === 'http.server' &&
+      transactionEvent?.transaction === 'GET /test-outgoing-http'
+    );
+  });
+
+  const { data } = await axios.get(`${baseURL}/test-outgoing-http`);
+
+  const inboundTransaction = await inboundTransactionPromise;
+  const outboundTransaction = await outboundTransactionPromise;
+
+  const traceId = outboundTransaction?.contexts?.trace?.trace_id;
+  const outgoingHttpSpan = outboundTransaction?.spans?.find(span => span.op === 'http.client') as
+    | ReturnType<Span['toJSON']>
+    | undefined;
+
+  expect(outgoingHttpSpan).toBeDefined();
+
+  const outgoingHttpSpanId = outgoingHttpSpan?.span_id;
+
+  expect(traceId).toEqual(expect.any(String));
+
+  // data is passed through from the inbound request, to verify we have the correct headers set
+  const inboundHeaderSentryTrace = data.headers?.['sentry-trace'];
+  const inboundHeaderBaggage = data.headers?.['baggage'];
+
+  expect(inboundHeaderSentryTrace).toEqual(`${traceId}-${outgoingHttpSpanId}-1`);
+  expect(inboundHeaderBaggage).toBeDefined();
+
+  const baggage = (inboundHeaderBaggage || '').split(',');
+  expect(baggage).toEqual(
+    expect.arrayContaining([
+      'sentry-environment=qa',
+      `sentry-trace_id=${traceId}`,
+      expect.stringMatching(/sentry-public_key=/),
+    ]),
+  );
+
+  expect(outboundTransaction).toEqual(
+    expect.objectContaining({
+      contexts: expect.objectContaining({
+        trace: {
+          data: {
+            url: 'http://localhost:3030/test-outgoing-http',
+            'otel.kind': 'SERVER',
+            'http.response.status_code': 200,
+          },
+          op: 'http.server',
+          span_id: expect.any(String),
+          status: 'ok',
+          tags: {
+            'http.status_code': 200,
+          },
+          trace_id: traceId,
+        },
+      }),
+    }),
+  );
+
+  expect(inboundTransaction).toEqual(
+    expect.objectContaining({
+      contexts: expect.objectContaining({
+        trace: {
+          data: {
+            url: 'http://localhost:3030/test-inbound-headers',
+            'otel.kind': 'SERVER',
+            'http.response.status_code': 200,
+          },
+          op: 'http.server',
+          parent_span_id: outgoingHttpSpanId,
+          span_id: expect.any(String),
+          status: 'ok',
+          tags: {
+            'http.status_code': 200,
+          },
+          trace_id: traceId,
+        },
+      }),
+    }),
+  );
+});

packages/node-experimental/src/constants.ts

@@ -14,3 +14,8 @@ export const OTEL_ATTR_BREADCRUMB_EVENT_ID = 'sentry.breadcrumb.event_id';
 export const OTEL_ATTR_BREADCRUMB_CATEGORY = 'sentry.breadcrumb.category';
 export const OTEL_ATTR_BREADCRUMB_DATA = 'sentry.breadcrumb.data';
 export const OTEL_ATTR_SENTRY_SAMPLE_RATE = 'sentry.sample_rate';
+
+export const SENTRY_TRACE_HEADER = 'sentry-trace';
+export const SENTRY_BAGGAGE_HEADER = 'baggage';
+
+export const SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY = createContextKey('SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY');

packages/node-experimental/src/opentelemetry/propagator.ts

@@ -0,0 +1,131 @@
+import type { Baggage, Context, SpanContext, TextMapGetter, TextMapSetter } from '@opentelemetry/api';
+import { propagation, trace, TraceFlags } from '@opentelemetry/api';
+import { isTracingSuppressed, W3CBaggagePropagator } from '@opentelemetry/core';
+import { getDynamicSamplingContextFromClient } from '@sentry/core';
+import type { DynamicSamplingContext, PropagationContext } from '@sentry/types';
+import { generateSentryTraceHeader, SENTRY_BAGGAGE_KEY_PREFIX, tracingContextFromHeaders } from '@sentry/utils';
+
+import { getCurrentHub } from '../sdk/hub';
+import { SENTRY_BAGGAGE_HEADER, SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY, SENTRY_TRACE_HEADER } from './../constants';
+import { getSpanScope } from './spanData';
+
+/**
+ * Injects and extracts `sentry-trace` and `baggage` headers from carriers.
+ */
+export class SentryPropagator extends W3CBaggagePropagator {
+  /**
+   * @inheritDoc
+   */
+  public inject(context: Context, carrier: unknown, setter: TextMapSetter): void {
+    if (isTracingSuppressed(context)) {
+      return;
+    }
+
+    let baggage = propagation.getBaggage(context) || propagation.createBaggage({});
+
+    const propagationContext = context.getValue(SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY) as
+      | PropagationContext
+      | undefined;
+
+    const { spanId, traceId, sampled } = getSentryTraceData(context, propagationContext);
+
+    const dynamicSamplingContext = propagationContext ? getDsc(context, propagationContext, traceId) : undefined;
+
+    if (dynamicSamplingContext) {
+      baggage = Object.entries(dynamicSamplingContext).reduce<Baggage>((b, [dscKey, dscValue]) => {
+        if (dscValue) {
+          return b.setEntry(`${SENTRY_BAGGAGE_KEY_PREFIX}${dscKey}`, { value: dscValue });
+        }
+        return b;
+      }, baggage);
+    }
+
+    setter.set(carrier, SENTRY_TRACE_HEADER, generateSentryTraceHeader(traceId, spanId, sampled));
+
+    super.inject(propagation.setBaggage(context, baggage), carrier, setter);
+  }
+
+  /**
+   * @inheritDoc
+   */
+  public extract(context: Context, carrier: unknown, getter: TextMapGetter): Context {
+    const maybeSentryTraceHeader: string | string[] | undefined = getter.get(carrier, SENTRY_TRACE_HEADER);
+    const maybeBaggageHeader = getter.get(carrier, SENTRY_BAGGAGE_HEADER);
+
+    const sentryTraceHeader = maybeSentryTraceHeader
+      ? Array.isArray(maybeSentryTraceHeader)
+        ? maybeSentryTraceHeader[0]
+        : maybeSentryTraceHeader
+      : undefined;
+
+    const { propagationContext } = tracingContextFromHeaders(sentryTraceHeader, maybeBaggageHeader);
+
+    // Add propagation context to context
+    const contextWithPropagationContext = context.setValue(SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY, propagationContext);
+
+    const spanContext: SpanContext = {
+      traceId: propagationContext.traceId,
+      spanId: propagationContext.parentSpanId || '',
+      isRemote: true,
+      traceFlags: propagationContext.sampled === true ? TraceFlags.SAMPLED : TraceFlags.NONE,
+    };
+
+    // Add remote parent span context
+    return trace.setSpanContext(contextWithPropagationContext, spanContext);
+  }
+
+  /**
+   * @inheritDoc
+   */
+  public fields(): string[] {
+    return [SENTRY_TRACE_HEADER, SENTRY_BAGGAGE_HEADER];
+  }
+}
+
+function getDsc(
+  context: Context,
+  propagationContext: PropagationContext,
+  traceId: string | undefined,
+): DynamicSamplingContext | undefined {
+  // If we have a DSC on the propagation context, we just use it
+  if (propagationContext.dsc) {
+    return propagationContext.dsc;
+  }
+
+  // Else, we try to generate a new one
+  const client = getCurrentHub().getClient();
+  const activeSpan = trace.getSpan(context);
+  const scope = activeSpan ? getSpanScope(activeSpan) : undefined;
+
+  if (client) {
+    return getDynamicSamplingContextFromClient(traceId || propagationContext.traceId, client, scope);
+  }
+
+  return undefined;
+}
+
+function getSentryTraceData(
+  context: Context,
+  propagationContext: PropagationContext | undefined,
+): {
+  spanId: string | undefined;
+  traceId: string | undefined;
+  sampled: boolean | undefined;
+} {
+  const span = trace.getSpan(context);
+  const spanContext = span && span.spanContext();
+
+  const traceId = spanContext ? spanContext.traceId : propagationContext?.traceId;
+
+  // We have a few scenarios here:
+  // If we have an active span, and it is _not_ remote, we just use the span's ID
+  // If we have an active span that is remote, we do not want to use the spanId, as we don't want to attach it to the parent span
+  // If `isRemote === true`, the span is bascially virtual
+  // If we don't have a local active span, we use the generated spanId from the propagationContext
+  const spanId = spanContext && !spanContext.isRemote ? spanContext.spanId : propagationContext?.spanId;
+
+  // eslint-disable-next-line no-bitwise
+  const sampled = spanContext ? Boolean(spanContext.traceFlags & TraceFlags.SAMPLED) : propagationContext?.sampled;
+
+  return { traceId, spanId, sampled };
+}

packages/node-experimental/src/opentelemetry/sampler.ts

@@ -4,11 +4,14 @@ import { isSpanContextValid, trace, TraceFlags } from '@opentelemetry/api';
 import type { Sampler, SamplingResult } from '@opentelemetry/sdk-trace-base';
 import { SamplingDecision } from '@opentelemetry/sdk-trace-base';
 import { hasTracingEnabled } from '@sentry/core';
-import { _INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY } from '@sentry/opentelemetry-node';
-import type { Client, ClientOptions, SamplingContext, TraceparentData } from '@sentry/types';
+import type { Client, ClientOptions, PropagationContext, SamplingContext } from '@sentry/types';
 import { isNaN, logger } from '@sentry/utils';
 
-import { OTEL_ATTR_PARENT_SAMPLED, OTEL_ATTR_SENTRY_SAMPLE_RATE } from '../constants';
+import {
+  OTEL_ATTR_PARENT_SAMPLED,
+  OTEL_ATTR_SENTRY_SAMPLE_RATE,
+  SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY,
+} from '../constants';
 
 /**
  * A custom OTEL sampler that uses Sentry sampling rates to make it's decision
@@ -177,14 +180,14 @@ function isValidSampleRate(rate: unknown): boolean {
   return true;
 }
 
-function getTraceParentData(parentContext: Context): TraceparentData | undefined {
-  return parentContext.getValue(_INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY) as TraceparentData | undefined;
+function getPropagationContext(parentContext: Context): PropagationContext | undefined {
+  return parentContext.getValue(SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY) as PropagationContext | undefined;
 }
 
 function getParentRemoteSampled(spanContext: SpanContext, context: Context): boolean | undefined {
   const traceId = spanContext.traceId;
-  const traceparentData = getTraceParentData(context);
+  const traceparentData = getPropagationContext(context);
 
   // Only inherit sample rate if `traceId` is the same
-  return traceparentData && traceId === traceparentData.traceId ? traceparentData.parentSampled : undefined;
+  return traceparentData && traceId === traceparentData.traceId ? traceparentData.sampled : undefined;
 }

packages/node-experimental/src/sdk/initOtel.ts

@@ -3,9 +3,9 @@ import { Resource } from '@opentelemetry/resources';
 import { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
 import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
 import { SDK_VERSION } from '@sentry/core';
-import { SentryPropagator } from '@sentry/opentelemetry-node';
 import { logger } from '@sentry/utils';
 
+import { SentryPropagator } from '../opentelemetry/propagator';
 import { SentrySampler } from '../opentelemetry/sampler';
 import { SentrySpanProcessor } from '../opentelemetry/spanProcessor';
 import type { NodeExperimentalClient } from '../types';
@@ -15,7 +15,6 @@ import { getCurrentHub } from './hub';
 
 /**
  * Initialize OpenTelemetry for Node.
- * We use the @sentry/opentelemetry-node package to communicate with OpenTelemetry.
  */
 export function initOtel(): void {
   const client = getCurrentHub().getClient<NodeExperimentalClient>();