ref(node): Align semantic attribute handling

mydea · mydea · commit 22b3256ac412 · 2024-02-28T09:44:59.000Z
Also stop clearing out sentry attributes, we don't actually want that (except for the parent sampled stuff for now).
diff --git a/packages/node-experimental/test/integration/transactions.test.ts b/packages/node-experimental/test/integration/transactions.test.ts
@@ -70,6 +70,9 @@ describe('Integration | Transactions', () => {
           otel: {
             attributes: {
               'test.outer': 'test value',
+              'sentry.op': 'test op',
+              'sentry.origin': 'auto.test',
+              'sentry.source': 'task',
             },
             resource: {
               'service.name': 'node-experimental',
@@ -241,6 +244,9 @@ describe('Integration | Transactions', () => {
           otel: expect.objectContaining({
             attributes: {
               'test.outer': 'test value',
+              'sentry.op': 'test op',
+              'sentry.origin': 'auto.test',
+              'sentry.source': 'task',
             },
           }),
           trace: {
@@ -283,6 +289,7 @@ describe('Integration | Transactions', () => {
           otel: expect.objectContaining({
             attributes: {
               'test.outer': 'test value b',
+              'sentry.op': 'test op b',
             },
           }),
           trace: {
@@ -513,7 +520,11 @@ describe('Integration | Transactions', () => {
       expect.objectContaining({
         contexts: expect.objectContaining({
           otel: expect.objectContaining({
-            attributes: {},
+            attributes: {
+              'sentry.op': 'test op',
+              'sentry.origin': 'auto.test',
+              'sentry.source': 'task',
+            },
           }),
           trace: {
             data: {
diff --git a/packages/opentelemetry/src/semanticAttributes.ts b/packages/opentelemetry/src/semanticAttributes.ts
@@ -3,13 +3,5 @@
  * No guarantees apply to these attributes, and the may change/disappear at any time.
  */
 export const InternalSentrySemanticAttributes = {
-  ORIGIN: 'sentry.origin',
-  OP: 'sentry.op',
-  SOURCE: 'sentry.source',
   PARENT_SAMPLED: 'sentry.parentSampled',
-  BREADCRUMB_TYPE: 'sentry.breadcrumb.type',
-  BREADCRUMB_LEVEL: 'sentry.breadcrumb.level',
-  BREADCRUMB_EVENT_ID: 'sentry.breadcrumb.event_id',
-  BREADCRUMB_CATEGORY: 'sentry.breadcrumb.category',
-  BREADCRUMB_DATA: 'sentry.breadcrumb.data',
 } as const;
diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts
@@ -4,8 +4,13 @@ import { ExportResultCode } from '@opentelemetry/core';
 import type { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import type { Transaction } from '@sentry/core';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, getCurrentHub } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+  getCurrentHub,
+} from '@sentry/core';
 import type { Scope, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types';
 import { addNonEnumerableProperty, dropUndefinedKeys, logger } from '@sentry/utils';
 import { startTransaction } from './custom/transaction';
@@ -132,9 +137,9 @@ function shouldCleanupSpan(span: ReadableSpan, maxStartTimeOffsetSeconds: number
 function parseSpan(span: ReadableSpan): { op?: string; origin?: SpanOrigin; source?: TransactionSource } {
   const attributes = span.attributes;
 
-  const origin = attributes[InternalSentrySemanticAttributes.ORIGIN] as SpanOrigin | undefined;
-  const op = attributes[InternalSentrySemanticAttributes.OP] as string | undefined;
-  const source = attributes[InternalSentrySemanticAttributes.SOURCE] as TransactionSource | undefined;
+  const origin = attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined;
+  const op = attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] as string | undefined;
+  const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] as TransactionSource | undefined;
 
   return { origin, op, source };
 }
@@ -278,11 +283,6 @@ function removeSentryAttributes(data: Record<string, unknown>): Record<string, u
 
   /* eslint-disable @typescript-eslint/no-dynamic-delete */
   delete cleanedData[InternalSentrySemanticAttributes.PARENT_SAMPLED];
-  delete cleanedData[InternalSentrySemanticAttributes.ORIGIN];
-  delete cleanedData[InternalSentrySemanticAttributes.OP];
-  delete cleanedData[InternalSentrySemanticAttributes.SOURCE];
-  // We want to avoid having this on each span (as that is set by the Sampler)
-  // We only want this on the transaction, where we manually add it to `attributes`
   delete cleanedData[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE];
   /* eslint-enable @typescript-eslint/no-dynamic-delete */
 
diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts
@@ -3,12 +3,19 @@ import { TraceFlags } from '@opentelemetry/api';
 import { context } from '@opentelemetry/api';
 import { SpanStatusCode, trace } from '@opentelemetry/api';
 import { TraceState, suppressTracing } from '@opentelemetry/core';
-import { SDK_VERSION, getClient, getCurrentScope, handleCallbackErrors } from '@sentry/core';
+import {
+  SDK_VERSION,
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+  getClient,
+  getCurrentScope,
+  handleCallbackErrors,
+} from '@sentry/core';
 import type { Client, Scope } from '@sentry/types';
 import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
 import { SENTRY_TRACE_STATE_DSC } from './constants';
 
-import { InternalSentrySemanticAttributes } from './semanticAttributes';
 import type { OpenTelemetryClient, OpenTelemetrySpanContext } from './types';
 import { getContextFromScope } from './utils/contextData';
 import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
@@ -137,15 +144,15 @@ function _applySentryAttributesToSpan(span: Span, options: OpenTelemetrySpanCont
   const { origin, op, source, metadata } = options;
 
   if (origin) {
-    span.setAttribute(InternalSentrySemanticAttributes.ORIGIN, origin);
+    span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, origin);
   }
 
   if (op) {
-    span.setAttribute(InternalSentrySemanticAttributes.OP, op);
+    span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, op);
   }
 
   if (source) {
-    span.setAttribute(InternalSentrySemanticAttributes.SOURCE, source);
+    span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
   }
 
   if (metadata) {
diff --git a/packages/opentelemetry/src/utils/addOriginToSpan.ts b/packages/opentelemetry/src/utils/addOriginToSpan.ts
@@ -1,9 +1,8 @@
 import type { Span } from '@opentelemetry/api';
+import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
 import type { SpanOrigin } from '@sentry/types';
 
-import { InternalSentrySemanticAttributes } from '../semanticAttributes';
-
 /** Adds an origin to an OTEL Span. */
 export function addOriginToSpan(span: Span, origin: SpanOrigin): void {
-  span.setAttribute(InternalSentrySemanticAttributes.ORIGIN, origin);
+  span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, origin);
 }
diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts
@@ -77,6 +77,9 @@ describe('Integration | Transactions', () => {
           otel: {
             attributes: {
               'test.outer': 'test value',
+              'sentry.op': 'test op',
+              'sentry.origin': 'auto.test',
+              'sentry.source': 'task',
             },
             resource: {
               'service.name': 'opentelemetry-test',
@@ -245,6 +248,9 @@ describe('Integration | Transactions', () => {
           otel: expect.objectContaining({
             attributes: {
               'test.outer': 'test value',
+              'sentry.op': 'test op',
+              'sentry.origin': 'auto.test',
+              'sentry.source': 'task',
             },
           }),
           trace: {
@@ -287,6 +293,7 @@ describe('Integration | Transactions', () => {
           otel: expect.objectContaining({
             attributes: {
               'test.outer': 'test value b',
+              'sentry.op': 'test op b',
             },
           }),
           trace: {
@@ -364,7 +371,11 @@ describe('Integration | Transactions', () => {
       expect.objectContaining({
         contexts: expect.objectContaining({
           otel: expect.objectContaining({
-            attributes: {},
+            attributes: {
+              'sentry.op': 'test op',
+              'sentry.origin': 'auto.test',
+              'sentry.source': 'task',
+            },
           }),
           trace: {
             data: {
diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts
@@ -3,10 +3,16 @@ import { SpanKind } from '@opentelemetry/api';
 import { TraceFlags, context, trace } from '@opentelemetry/api';
 import type { ReadableSpan } from '@opentelemetry/sdk-trace-base';
 import { Span as SpanClass } from '@opentelemetry/sdk-trace-base';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, getClient, getCurrentScope, spanToJSON } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+  getClient,
+  getCurrentScope,
+} from '@sentry/core';
 import type { Event, PropagationContext, Scope } from '@sentry/types';
 
-import { InternalSentrySemanticAttributes } from '../src/semanticAttributes';
 import { startInactiveSpan, startSpan, startSpanManual } from '../src/trace';
 import type { AbstractSpan } from '../src/types';
 import { setPropagationContextOnContext } from '../src/utils/contextData';
@@ -220,15 +226,15 @@ describe('trace', () => {
           origin: 'auto.test.origin',
           metadata: { requestPath: 'test-path' },
           attributes: {
-            [InternalSentrySemanticAttributes.SOURCE]: 'task',
+            [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task',
           },
         },
         span => {
           expect(span).toBeDefined();
           expect(getSpanAttributes(span)).toEqual({
-            [InternalSentrySemanticAttributes.SOURCE]: 'task',
-            [InternalSentrySemanticAttributes.ORIGIN]: 'auto.test.origin',
-            [InternalSentrySemanticAttributes.OP]: 'my-op',
+            [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task',
+            [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin',
+            [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op',
             [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
           });
 
@@ -481,17 +487,17 @@ describe('trace', () => {
         op: 'my-op',
         origin: 'auto.test.origin',
         attributes: {
-          [InternalSentrySemanticAttributes.SOURCE]: 'task',
+          [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task',
         },
         metadata: { requestPath: 'test-path' },
       });
 
       expect(span2).toBeDefined();
       expect(getSpanAttributes(span2)).toEqual({
         [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
-        [InternalSentrySemanticAttributes.SOURCE]: 'task',
-        [InternalSentrySemanticAttributes.ORIGIN]: 'auto.test.origin',
-        [InternalSentrySemanticAttributes.OP]: 'my-op',
+        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task',
+        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.test.origin',
+        [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'my-op',
       });
 
       expect(getSpanMetadata(span2)).toEqual({ requestPath: 'test-path' });
