fix(node): Pass inferred name & attributes to tracesSampler

dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/server.js

@@ -0,0 +1,40 @@
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+const Sentry = require('@sentry/node');
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  transport: loggingTransport,
+  tracesSampler: samplingContext => {
+    // The name we get here is inferred at span creation time
+    // At this point, we sadly do not have a http.route attribute yet,
+    // so we infer the name from the unparametrized route instead
+    return (
+      samplingContext.name === 'GET /test/123' &&
+      samplingContext.attributes['sentry.op'] === 'http.server' &&
+      samplingContext.attributes['http.method'] === 'GET'
+    );
+  },
+  debug: true,
+});
+
+// express must be required after Sentry is initialized
+const express = require('express');
+const cors = require('cors');
+const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
+
+const app = express();
+
+app.use(cors());
+
+app.get('/test/:id', (_req, res) => {
+  res.send('Success');
+});
+
+app.get('/test2', (_req, res) => {
+  res.send('Success');
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/test.ts

@@ -0,0 +1,24 @@
+import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
+
+describe('express tracesSampler', () => {
+  afterAll(() => {
+    cleanupChildProcesses();
+  });
+
+  describe('CJS', () => {
+    test('correctly samples & passes data to tracesSampler', done => {
+      const runner = createRunner(__dirname, 'server.js')
+        .expect({
+          transaction: {
+            transaction: 'GET /test/:id',
+          },
+        })
+        .start(done);
+
+      // This is not sampled
+      runner.makeRequest('get', '/test2?q=1');
+      // This is sampled
+      runner.makeRequest('get', '/test/123?q=1');
+    });
+  });
+});

packages/opentelemetry/src/sampler.ts

@@ -4,7 +4,12 @@ import { isSpanContextValid, trace } from '@opentelemetry/api';
 import { TraceState } from '@opentelemetry/core';
 import type { Sampler, SamplingResult } from '@opentelemetry/sdk-trace-base';
 import { SamplingDecision } from '@opentelemetry/sdk-trace-base';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, hasTracingEnabled, sampleSpan } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
+  hasTracingEnabled,
+  sampleSpan,
+} from '@sentry/core';
 import type { Client, SpanAttributes } from '@sentry/types';
 import { logger } from '@sentry/utils';
 import { SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, SENTRY_TRACE_STATE_URL } from './constants';
@@ -13,6 +18,7 @@ import { SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_URL } from '@opentelemetry/semantic
 import { DEBUG_BUILD } from './debug-build';
 import { getPropagationContextFromSpan } from './propagator';
 import { getSamplingDecision } from './utils/getSamplingDecision';
+import { inferSpanData } from './utils/parseSpanDescription';
 import { setIsSetup } from './utils/setupCheck';
 
 /**
@@ -56,12 +62,28 @@ export class SentrySampler implements Sampler {
 
     const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined;
 
+    // We want to pass the inferred name & attributes to the sampler method
+    const {
+      description: inferredSpanName,
+      data: inferredAttributes,
+      op,
+    } = inferSpanData(spanName, spanAttributes, spanKind);
+
+    const mergedAttributes = {
+      ...inferredAttributes,
+      ...spanAttributes,
+    };
+
+    if (op) {
+      mergedAttributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] = op;
+    }
+
     const mutableSamplingDecision = { decision: true };
     this._client.emit(
       'beforeSampling',
       {
-        spanAttributes: spanAttributes,
-        spanName: spanName,
+        spanAttributes: mergedAttributes,
+        spanName: inferredSpanName,
         parentSampled: parentSampled,
         parentContext: parentContext,
       },
@@ -72,10 +94,10 @@ export class SentrySampler implements Sampler {
     }
 
     const [sampled, sampleRate] = sampleSpan(options, {
-      name: spanName,
-      attributes: spanAttributes,
+      name: inferredSpanName,
+      attributes: mergedAttributes,
       transactionContext: {
-        name: spanName,
+        name: inferredSpanName,
         parentSampled,
       },
       parentSampled,

packages/opentelemetry/src/trace.ts

@@ -45,8 +45,6 @@ export function startSpan<T>(options: OpenTelemetrySpanContext, callback: (span:
     const spanOptions = getSpanOptions(options);
 
     return tracer.startActiveSpan(name, spanOptions, ctx, span => {
-      _applySentryAttributesToSpan(span, options);
-
       return handleCallbackErrors(
         () => callback(span),
         () => {
@@ -90,8 +88,6 @@ export function startSpanManual<T>(
     const spanOptions = getSpanOptions(options);
 
     return tracer.startActiveSpan(name, spanOptions, ctx, span => {
-      _applySentryAttributesToSpan(span, options);
-
       return handleCallbackErrors(
         () => callback(span, () => span.end()),
         () => {
@@ -131,8 +127,6 @@ export function startInactiveSpan(options: OpenTelemetrySpanContext): Span {
 
     const span = tracer.startSpan(name, spanOptions, ctx);
 
-    _applySentryAttributesToSpan(span, options);
-
     return span;
   });
 }
@@ -156,22 +150,19 @@ function getTracer(): Tracer {
   return (client && client.tracer) || trace.getTracer('@sentry/opentelemetry', SDK_VERSION);
 }
 
-function _applySentryAttributesToSpan(span: Span, options: OpenTelemetrySpanContext): void {
-  const { op } = options;
-
-  if (op) {
-    span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, op);
-  }
-}
-
 function getSpanOptions(options: OpenTelemetrySpanContext): SpanOptions {
-  const { startTime, attributes, kind } = options;
+  const { startTime, attributes, kind, op } = options;
 
   // OTEL expects timestamps in ms, not seconds
   const fixedStartTime = typeof startTime === 'number' ? ensureTimestampInMilliseconds(startTime) : startTime;
 
   return {
-    attributes,
+    attributes: op
+      ? {
+          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
+          ...attributes,
+        }
+      : attributes,
     kind,
     startTime: fixedStartTime,
   };

packages/opentelemetry/src/utils/parseSpanDescription.ts

@@ -11,7 +11,7 @@ import {
   SEMATTRS_MESSAGING_SYSTEM,
   SEMATTRS_RPC_SERVICE,
 } from '@opentelemetry/semantic-conventions';
-import type { TransactionSource } from '@sentry/types';
+import type { SpanAttributes, TransactionSource } from '@sentry/types';
 import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils';
 
 import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core';
@@ -27,14 +27,9 @@ interface SpanDescription {
 }
 
 /**
- * Extract better op/description from an otel span.
- *
- * Based on https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/7422ce2a06337f68a59b552b8c5a2ac125d6bae5/exporter/sentryexporter/sentry_exporter.go#L306
+ * Infer the op & description for a set of name, attributes and kind of a span.
  */
-export function parseSpanDescription(span: AbstractSpan): SpanDescription {
-  const attributes = spanHasAttributes(span) ? span.attributes : {};
-  const name = spanHasName(span) ? span.name : '<unknown>';
-
+export function inferSpanData(name: string, attributes: SpanAttributes, kind: SpanKind): SpanDescription {
   // This attribute is intentionally exported as a SEMATTR constant because it should stay intimite API
   if (attributes['sentry.skip_span_data_inference']) {
     return {
@@ -54,7 +49,7 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription {
   // conventions export an attribute key for it.
   const httpMethod = attributes['http.request.method'] || attributes[SEMATTRS_HTTP_METHOD];
   if (httpMethod) {
-    return descriptionForHttpMethod({ attributes, name, kind: getSpanKind(span) }, httpMethod);
+    return descriptionForHttpMethod({ attributes, name, kind }, httpMethod);
   }
 
   const dbSystem = attributes[SEMATTRS_DB_SYSTEM];
@@ -97,6 +92,19 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription {
   return { op: undefined, description: name, source: 'custom' };
 }
 
+/**
+ * Extract better op/description from an otel span.
+ *
+ * Based on https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/7422ce2a06337f68a59b552b8c5a2ac125d6bae5/exporter/sentryexporter/sentry_exporter.go#L306
+ */
+export function parseSpanDescription(span: AbstractSpan): SpanDescription {
+  const attributes = spanHasAttributes(span) ? span.attributes : {};
+  const name = spanHasName(span) ? span.name : '<unknown>';
+  const kind = getSpanKind(span);
+
+  return inferSpanData(name, attributes, kind);
+}
+
 function descriptionForDbSystem({ attributes, name }: { attributes: Attributes; name: string }): SpanDescription {
   // Use DB statement (Ex "SELECT * FROM table") if possible as description.
   const statement = attributes[SEMATTRS_DB_STATEMENT];

packages/opentelemetry/test/trace.test.ts

@@ -1320,9 +1320,7 @@ describe('trace (sampling)', () => {
     expect(tracesSampler).toHaveBeenLastCalledWith({
       parentSampled: undefined,
       name: 'outer',
-      attributes: {
-        [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
-      },
+      attributes: {},
       transactionContext: { name: 'outer', parentSampled: undefined },
     });
 
@@ -1357,16 +1355,25 @@ describe('trace (sampling)', () => {
 
     mockSdkInit({ tracesSampler });
 
-    startSpan({ name: 'outer' }, outerSpan => {
-      expect(outerSpan).toBeDefined();
-    });
+    startSpan(
+      {
+        name: 'outer',
+        op: 'test.op',
+        attributes: { attr1: 'yes', attr2: 1 },
+      },
+      outerSpan => {
+        expect(outerSpan).toBeDefined();
+      },
+    );
 
-    expect(tracesSampler).toBeCalledTimes(1);
+    expect(tracesSampler).toHaveBeenCalledTimes(1);
     expect(tracesSampler).toHaveBeenLastCalledWith({
       parentSampled: undefined,
       name: 'outer',
       attributes: {
-        [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
+        attr1: 'yes',
+        attr2: 1,
+        'sentry.op': 'test.op',
       },
       transactionContext: { name: 'outer', parentSampled: undefined },
     });