ref(node): Use new performance APIs in legacy http & undici (#11055)

Extraced this out from #11051, as it's actually not fully related.
getsentry · Mar 12, 2024 · af90bfa · af90bfa
1 parent 2ee0c18
commit af90bfa
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 41 deletions.
diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts
@@ -49,29 +49,29 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
   const dsc = getDynamicSamplingContextFromClient(spanToJSON(span).trace_id || '', client);
 
   // TODO (v8): Remove v7FrozenDsc as a Transaction will no longer have _frozenDynamicSamplingContext
-  const txn = getRootSpan(span) as TransactionWithV7FrozenDsc | undefined;
-  if (!txn) {
+  const rootSpan = getRootSpan(span);
+  if (!rootSpan) {
     return dsc;
   }
 
   // TODO (v8): Remove v7FrozenDsc as a Transaction will no longer have _frozenDynamicSamplingContext
   // For now we need to avoid breaking users who directly created a txn with a DSC, where this field is still set.
   // @see Transaction class constructor
-  const v7FrozenDsc = txn && txn._frozenDynamicSamplingContext;
+  const v7FrozenDsc = rootSpan && (rootSpan as TransactionWithV7FrozenDsc)._frozenDynamicSamplingContext;
   if (v7FrozenDsc) {
     return v7FrozenDsc;
   }
 
   // TODO (v8): Replace txn.metadata with txn.attributes[]
   // We can't do this yet because attributes aren't always set yet.
   // eslint-disable-next-line deprecation/deprecation
-  const { sampleRate: maybeSampleRate } = txn.metadata;
+  const { sampleRate: maybeSampleRate } = (rootSpan as TransactionWithV7FrozenDsc).metadata || {};
   if (maybeSampleRate != null) {
     dsc.sample_rate = `${maybeSampleRate}`;
   }
 
   // We don't want to have a transaction name in the DSC if the source is "url" because URLs might contain PII
-  const jsonSpan = spanToJSON(txn);
+  const jsonSpan = spanToJSON(rootSpan);
 
   const source = (jsonSpan.data || {})[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
 
@@ -80,7 +80,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
     dsc.transaction = jsonSpan.description;
   }
 
-  dsc.sampled = String(spanIsSampled(txn));
+  dsc.sampled = String(spanIsSampled(rootSpan));
 
   client.emit('createDsc', dsc);
 

diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -1,11 +1,11 @@
 /* eslint-disable max-lines */
 import type * as http from 'http';
 import type * as https from 'https';
-import type { Hub, SentrySpan } from '@sentry/core';
+import type { Hub } from '@sentry/core';
+import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core';
 import { defineIntegration, getIsolationScope, hasTracingEnabled } from '@sentry/core';
 import {
   addBreadcrumb,
-  getActiveSpan,
   getClient,
   getCurrentHub,
   getCurrentScope,
@@ -319,17 +319,18 @@ function _createWrappedRequestMethodFactory(
 
       const scope = getCurrentScope();
       const isolationScope = getIsolationScope();
-      const parentSpan = getActiveSpan() as SentrySpan;
 
-      const data = getRequestSpanData(requestUrl, requestOptions);
+      const attributes = getRequestSpanData(requestUrl, requestOptions);
 
       const requestSpan = shouldCreateSpan(rawRequestUrl)
-        ? // eslint-disable-next-line deprecation/deprecation
-          parentSpan?.startChild({
+        ? startInactiveSpan({
+            onlyIfParent: true,
             op: 'http.client',
-            origin: 'auto.http.node.http',
-            name: `${data['http.method']} ${data.url}`,
-            data,
+            name: `${attributes['http.method']} ${attributes.url}`,
+            attributes: {
+              ...attributes,
+              [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.node.http',
+            },
           })
         : undefined;
 
@@ -365,7 +366,7 @@ function _createWrappedRequestMethodFactory(
           // eslint-disable-next-line @typescript-eslint/no-this-alias
           const req = this;
           if (breadcrumbsEnabled) {
-            addRequestBreadcrumb('response', data, req, res);
+            addRequestBreadcrumb('response', attributes, req, res);
           }
           if (requestSpan) {
             if (res.statusCode) {
@@ -380,7 +381,7 @@ function _createWrappedRequestMethodFactory(
           const req = this;
 
           if (breadcrumbsEnabled) {
-            addRequestBreadcrumb('error', data, req);
+            addRequestBreadcrumb('error', attributes, req);
           }
           if (requestSpan) {
             setHttpStatus(requestSpan, 500);

diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts
@@ -1,9 +1,8 @@
-import type { SentrySpan } from '@sentry/core';
+import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core';
 import {
   SPAN_STATUS_ERROR,
   addBreadcrumb,
   defineIntegration,
-  getActiveSpan,
   getClient,
   getCurrentScope,
   getDynamicSamplingContextFromClient,
@@ -14,7 +13,14 @@ import {
   setHttpStatus,
   spanToTraceHeader,
 } from '@sentry/core';
-import type { EventProcessor, Integration, IntegrationFn, IntegrationFnResult, Span } from '@sentry/types';
+import type {
+  EventProcessor,
+  Integration,
+  IntegrationFn,
+  IntegrationFnResult,
+  Span,
+  SpanAttributes,
+} from '@sentry/types';
 import {
   LRUMap,
   dynamicSamplingContextToSentryBaggageHeader,
@@ -185,9 +191,8 @@ export class Undici implements Integration {
     const clientOptions = client.getOptions();
     const scope = getCurrentScope();
     const isolationScope = getIsolationScope();
-    const parentSpan = getActiveSpan() as SentrySpan;
 
-    const span = this._shouldCreateSpan(stringUrl) ? createRequestSpan(parentSpan, request, stringUrl) : undefined;
+    const span = this._shouldCreateSpan(stringUrl) ? createRequestSpan(request, stringUrl) : undefined;
     if (span) {
       request.__sentry_span__ = span;
     }
@@ -326,28 +331,24 @@ function setHeadersOnRequest(
   }
 }
 
-function createRequestSpan(
-  activeSpan: SentrySpan | undefined,
-  request: RequestWithSentry,
-  stringUrl: string,
-): Span | undefined {
+function createRequestSpan(request: RequestWithSentry, stringUrl: string): Span {
   const url = parseUrl(stringUrl);
 
   const method = request.method || 'GET';
-  const data: Record<string, unknown> = {
+  const attributes: SpanAttributes = {
     'http.method': method,
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.node.undici',
   };
   if (url.search) {
-    data['http.query'] = url.search;
+    attributes['http.query'] = url.search;
   }
   if (url.hash) {
-    data['http.fragment'] = url.hash;
+    attributes['http.fragment'] = url.hash;
   }
-  // eslint-disable-next-line deprecation/deprecation
-  return activeSpan?.startChild({
+  return startInactiveSpan({
+    onlyIfParent: true,
     op: 'http.client',
-    origin: 'auto.http.node.undici',
     name: `${method} ${getSanitizedUrlString(url)}`,
-    data,
+    attributes,
   });
 }
diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts
@@ -1,6 +1,6 @@
 import * as http from 'http';
 import * as https from 'https';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getSpanDescendants } from '@sentry/core';
+import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getSpanDescendants, startSpan } from '@sentry/core';
 import type { Hub } from '@sentry/core';
 import { getCurrentHub, getIsolationScope, setCurrentClient } from '@sentry/core';
 import { Transaction } from '@sentry/core';
@@ -55,7 +55,6 @@ describe('tracing', () => {
     });
 
     expect(transaction).toBeInstanceOf(Transaction);
-
     // eslint-disable-next-line deprecation/deprecation
     getCurrentScope().setSpan(transaction);
 
@@ -207,19 +206,20 @@ describe('tracing', () => {
 
     const { traceId } = getCurrentScope().getPropagationContext();
 
-    const request = http.get('http://dogs.are.great/');
+    // Needs an outer span, or else we skip it
+    const request = startSpan({ name: 'outer' }, () => http.get('http://dogs.are.great/'));
     const sentryTraceHeader = request.getHeader('sentry-trace') as string;
     const baggageHeader = request.getHeader('baggage') as string;
 
     const parts = sentryTraceHeader.split('-');
 
-    // Should not include sampling decision since we don't wanna influence the tracing decisions downstream
-    expect(parts.length).toEqual(2);
+    expect(parts.length).toEqual(3);
     expect(parts[0]).toEqual(traceId);
     expect(parts[1]).toEqual(expect.any(String));
+    expect(parts[2]).toEqual('1');
 
     expect(baggageHeader).toEqual(
-      `sentry-environment=production,sentry-release=1.0.0,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=${traceId}`,
+      `sentry-environment=production,sentry-release=1.0.0,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=${traceId},sentry-sample_rate=1,sentry-transaction=outer,sentry-sampled=true`,
     );
   });
 
@@ -237,7 +237,8 @@ describe('tracing', () => {
       },
     });
 
-    const request = http.get('http://dogs.are.great/');
+    // Needs an outer span, or else we skip it
+    const request = startSpan({ name: 'outer' }, () => http.get('http://dogs.are.great/'));
     const sentryTraceHeader = request.getHeader('sentry-trace') as string;
     const baggageHeader = request.getHeader('baggage') as string;