feat(core): Add updateSpanName helper function

Lms24 · Lms24 · commit 91ee3ade008c · 2024-11-14T12:14:20.000+01:00
diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts
@@ -20,7 +20,11 @@ import { getMainCarrier } from '../carrier';
 import { getCurrentScope } from '../currentScopes';
 import { getMetricSummaryJsonForSpan, updateMetricSummaryOnSpan } from '../metrics/metric-summary';
 import type { MetricType } from '../metrics/types';
-import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+} from '../semanticAttributes';
 import type { SentrySpan } from '../tracing/sentrySpan';
 import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
 import { _getSpanForScope } from './spanOnScope';
@@ -283,3 +287,24 @@ export function updateMetricSummaryOnActiveSpan(
     updateMetricSummaryOnSpan(span, metricType, sanitizedName, value, unit, tags, bucketKey);
   }
 }
+
+/**
+ * Updates the name of the given span and ensures that the span name is not
+ * overwritten by the Sentry SDK.
+ *
+ * Use this function instead of `span.updateName()` if you want to make sure that
+ * your name is kept. For some spans, for example root `http.server` spans the
+ * Sentry SDK would otherwise overwrite the span name with a high-quality name
+ * it infers when the span ends.
+ *
+ * Use this function in server code or when your span is started on the server
+ * and on the client (browser). If you only update a span name on the client,
+ * you can also use `span.updateName()` the SDK does not overwrite the name.
+ *
+ * @param span - The span to update the name of.
+ * @param name - The name to set on the span.
+ */
+export function updateSpanName(span: Span, name: string): void {
+  span.updateName(name);
+  span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom');
+}
diff --git a/packages/core/test/lib/utils/spanUtils.test.ts b/packages/core/test/lib/utils/spanUtils.test.ts
@@ -3,6 +3,7 @@ import { TRACEPARENT_REGEXP, timestampInSeconds } from '@sentry/utils';
 import {
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   SPAN_STATUS_ERROR,
   SPAN_STATUS_OK,
   SPAN_STATUS_UNSET,
@@ -13,7 +14,13 @@ import {
   startSpan,
 } from '../../../src';
 import type { OpenTelemetrySdkTraceBaseSpan } from '../../../src/utils/spanUtils';
-import { getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../../../src/utils/spanUtils';
+import {
+  getRootSpan,
+  spanIsSampled,
+  spanTimeInputToSeconds,
+  spanToJSON,
+  updateSpanName,
+} from '../../../src/utils/spanUtils';
 import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
 
 describe('spanToTraceHeader', () => {
@@ -245,3 +252,13 @@ describe('getRootSpan', () => {
     });
   });
 });
+
+describe('updateSpanName', () => {
+  it('updates the span name and source', () => {
+    const span = new SentrySpan({ name: 'old-name', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } });
+    updateSpanName(span, 'new-name');
+    const spanJSON = spanToJSON(span);
+    expect(spanJSON.description).toBe('new-name');
+    expect(spanJSON.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
+  });
+});
diff --git a/packages/opentelemetry/src/utils/parseSpanDescription.ts b/packages/opentelemetry/src/utils/parseSpanDescription.ts
@@ -16,7 +16,11 @@ import {
 import type { SpanAttributes, TransactionSource } from '@sentry/types';
 import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils';
 
-import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+} from '@sentry/core';
 import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '../semanticAttributes';
 import type { AbstractSpan } from '../types';
 import { getSpanKind } from './getSpanKind';
@@ -32,12 +36,12 @@ interface SpanDescription {
 /**
  * Infer the op & description for a set of name, attributes and kind of a span.
  */
-export function inferSpanData(name: string, attributes: SpanAttributes, kind: SpanKind): SpanDescription {
+export function inferSpanData(originalName: string, attributes: SpanAttributes, kind: SpanKind): SpanDescription {
   // if http.method exists, this is an http request span
   // eslint-disable-next-line deprecation/deprecation
   const httpMethod = attributes[ATTR_HTTP_REQUEST_METHOD] || attributes[SEMATTRS_HTTP_METHOD];
   if (httpMethod) {
-    return descriptionForHttpMethod({ attributes, name, kind }, httpMethod);
+    return descriptionForHttpMethod({ attributes, name: originalName, kind }, httpMethod);
   }
 
   // eslint-disable-next-line deprecation/deprecation
@@ -49,17 +53,19 @@ export function inferSpanData(name: string, attributes: SpanAttributes, kind: Sp
   // If db.type exists then this is a database call span
   // If the Redis DB is used as a cache, the span description should not be changed
   if (dbSystem && !opIsCache) {
-    return descriptionForDbSystem({ attributes, name });
+    return descriptionForDbSystem({ attributes, name: originalName });
   }
 
+  const customSourceOrRoute = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom' ? 'custom' : 'route';
+
   // If rpc.service exists then this is a rpc call span.
   // eslint-disable-next-line deprecation/deprecation
   const rpcService = attributes[SEMATTRS_RPC_SERVICE];
   if (rpcService) {
     return {
       op: 'rpc',
-      description: name,
-      source: 'route',
+      description: originalName,
+      source: customSourceOrRoute,
     };
   }
 
@@ -69,24 +75,28 @@ export function inferSpanData(name: string, attributes: SpanAttributes, kind: Sp
   if (messagingSystem) {
     return {
       op: 'message',
-      description: name,
-      source: 'route',
+      description: originalName,
+      source: customSourceOrRoute,
     };
   }
 
   // If faas.trigger exists then this is a function as a service span.
   // eslint-disable-next-line deprecation/deprecation
   const faasTrigger = attributes[SEMATTRS_FAAS_TRIGGER];
   if (faasTrigger) {
-    return { op: faasTrigger.toString(), description: name, source: 'route' };
+    return { op: faasTrigger.toString(), description: originalName, source: customSourceOrRoute };
   }
 
-  return { op: undefined, description: name, source: 'custom' };
+  return { op: undefined, description: originalName, source: 'custom' };
 }
 
 /**
  * Extract better op/description from an otel span.
  *
+ * Does not overwrite the span name if the source is already set to custom to ensure
+ * that user-updated span names are preserved. In this case, we only adjust the op but
+ * leave span description and source unchanged.
+ *
  * Based on https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/7422ce2a06337f68a59b552b8c5a2ac125d6bae5/exporter/sentryexporter/sentry_exporter.go#L306
  */
 export function parseSpanDescription(span: AbstractSpan): SpanDescription {
@@ -98,6 +108,11 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription {
 }
 
 function descriptionForDbSystem({ attributes, name }: { attributes: Attributes; name: string }): SpanDescription {
+  // if we already set the source to custom, we don't overwrite the span description but just adjust the op
+  if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom') {
+    return { op: 'db', description: name, source: 'custom' };
+  }
+
   // Use DB statement (Ex "SELECT * FROM table") if possible as description.
   // eslint-disable-next-line deprecation/deprecation
   const statement = attributes[SEMATTRS_DB_STATEMENT];
@@ -170,7 +185,10 @@ export function descriptionForHttpMethod(
   const origin = attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] || 'manual';
   const isManualSpan = !`${origin}`.startsWith('auto');
 
-  const useInferredDescription = isClientOrServerKind || !isManualSpan;
+  // If users (or in very rare occasions we) set the source to custom, we don't overwrite it
+  const alreadyHasCustomSource = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom';
+
+  const useInferredDescription = !alreadyHasCustomSource && (isClientOrServerKind || !isManualSpan);
 
   return {
     op: opParts.join('.'),
diff --git a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts
@@ -16,6 +16,7 @@ import {
 } from '@opentelemetry/semantic-conventions';
 
 import { descriptionForHttpMethod, getSanitizedUrl, parseSpanDescription } from '../../src/utils/parseSpanDescription';
+import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
 
 describe('parseSpanDescription', () => {
   it.each([
@@ -81,6 +82,21 @@ describe('parseSpanDescription', () => {
         source: 'task',
       },
     ],
+    [
+      'works with db system and custom source',
+      {
+        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
+        [SEMATTRS_DB_SYSTEM]: 'mysql',
+        [SEMATTRS_DB_STATEMENT]: 'SELECT * from users',
+      },
+      'test name',
+      SpanKind.CLIENT,
+      {
+        description: 'test name',
+        op: 'db',
+        source: 'custom',
+      },
+    ],
     [
       'works with db system without statement',
       {
@@ -107,6 +123,20 @@ describe('parseSpanDescription', () => {
         source: 'route',
       },
     ],
+    [
+      'works with rpc service and custom source',
+      {
+        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
+        [SEMATTRS_RPC_SERVICE]: 'rpc-test-service',
+      },
+      'test name',
+      undefined,
+      {
+        description: 'test name',
+        op: 'rpc',
+        source: 'custom',
+      },
+    ],
     [
       'works with messaging system',
       {
@@ -120,6 +150,20 @@ describe('parseSpanDescription', () => {
         source: 'route',
       },
     ],
+    [
+      'works with messaging system and custom source',
+      {
+        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
+        [SEMATTRS_MESSAGING_SYSTEM]: 'test-messaging-system',
+      },
+      'test name',
+      undefined,
+      {
+        description: 'test name',
+        op: 'message',
+        source: 'custom',
+      },
+    ],
     [
       'works with faas trigger',
       {
@@ -133,6 +177,20 @@ describe('parseSpanDescription', () => {
         source: 'route',
       },
     ],
+    [
+      'works with faas trigger and custom source',
+      {
+        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
+        [SEMATTRS_FAAS_TRIGGER]: 'test-faas-trigger',
+      },
+      'test name',
+      undefined,
+      {
+        description: 'test name',
+        op: 'test-faas-trigger',
+        source: 'custom',
+      },
+    ],
   ])('%s', (_, attributes, name, kind, expected) => {
     const actual = parseSpanDescription({ attributes, kind, name } as unknown as Span);
     expect(actual).toEqual(expected);
@@ -172,6 +230,26 @@ describe('descriptionForHttpMethod', () => {
         source: 'url',
       },
     ],
+    [
+      'works with prefetch request',
+      'GET',
+      {
+        [SEMATTRS_HTTP_METHOD]: 'GET',
+        [SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path',
+        [SEMATTRS_HTTP_TARGET]: '/my-path',
+        'sentry.http.prefetch': true,
+      },
+      'test name',
+      SpanKind.CLIENT,
+      {
+        op: 'http.client.prefetch',
+        description: 'GET https://www.example.com/my-path',
+        data: {
+          url: 'https://www.example.com/my-path',
+        },
+        source: 'url',
+      },
+    ],
     [
       'works with basic server POST',
       'POST',
@@ -230,6 +308,27 @@ describe('descriptionForHttpMethod', () => {
         source: 'custom',
       },
     ],
+    [
+      "doesn't overwrite name with source custom",
+      'GET',
+      {
+        [SEMATTRS_HTTP_METHOD]: 'GET',
+        [SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path/123',
+        [SEMATTRS_HTTP_TARGET]: '/my-path/123',
+        [ATTR_HTTP_ROUTE]: '/my-path/:id',
+        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
+      },
+      'test name',
+      SpanKind.CLIENT,
+      {
+        op: 'http.client',
+        description: 'test name',
+        data: {
+          url: 'https://www.example.com/my-path/123',
+        },
+        source: 'custom',
+      },
+    ],
   ])('%s', (_, httpMethod, attributes, name, kind, expected) => {
     const actual = descriptionForHttpMethod({ attributes, kind, name }, httpMethod);
     expect(actual).toEqual(expected);
