feat(opentelemetry): Align span options with core span options (#10761)

mydea · AbhiPrasad · commit 3589664fc807 · 2024-02-21T13:33:46.000-05:00
This aligns the options for `startSpan` in otel with the core ones. Only
the `kind` is there in addition for now (we may want to remove this,
let's see).

Especially, this also adds the ability to pass a `scope` there and pick
up the correct context &amp; span to continue, as well as adding tests for
the options.
diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts
@@ -291,7 +291,7 @@ describe('startSpan', () => {
     expect(spanToJSON(_span!).timestamp).toBeDefined();
   });
 
-  it('allows to pass a `startTime` yyy', () => {
+  it('allows to pass a `startTime`', () => {
     const start = startSpan({ name: 'outer', startTime: [1234, 0] }, span => {
       return spanToJSON(span!).start_timestamp;
     });
diff --git a/packages/opentelemetry/src/contextManager.ts b/packages/opentelemetry/src/contextManager.ts
@@ -8,7 +8,7 @@ import {
   SENTRY_FORK_SET_SCOPE_CONTEXT_KEY,
 } from './constants';
 import { getCurrentHub } from './custom/getCurrentHub';
-import { getScopesFromContext, setHubOnContext, setScopesOnContext } from './utils/contextData';
+import { getScopesFromContext, setContextOnScope, setHubOnContext, setScopesOnContext } from './utils/contextData';
 
 /**
  * Wrap an OpenTelemetry ContextManager in a way that ensures the context is kept in sync with the Sentry Hub.
@@ -70,6 +70,8 @@ export function wrapContextManagerClass<ContextManagerInstance extends ContextMa
         .deleteValue(SENTRY_FORK_SET_SCOPE_CONTEXT_KEY)
         .deleteValue(SENTRY_FORK_SET_ISOLATION_SCOPE_CONTEXT_KEY);
 
+      setContextOnScope(newCurrentScope, ctx3);
+
       return super.with(ctx3, fn, thisArg, ...args);
     }
   }
diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts
@@ -1,4 +1,4 @@
-import type { Span, Tracer } from '@opentelemetry/api';
+import type { Context, Span, SpanOptions, Tracer } from '@opentelemetry/api';
 import { context } from '@opentelemetry/api';
 import { SpanStatusCode, trace } from '@opentelemetry/api';
 import { suppressTracing } from '@opentelemetry/core';
@@ -7,6 +7,7 @@ import type { Client, Scope } from '@sentry/types';
 
 import { InternalSentrySemanticAttributes } from './semanticAttributes';
 import type { OpenTelemetryClient, OpenTelemetrySpanContext } from './types';
+import { getContextFromScope } from './utils/contextData';
 import { setSpanMetadata } from './utils/spanData';
 
 /**
@@ -18,17 +19,19 @@ import { setSpanMetadata } from './utils/spanData';
  *
  * Note that you'll always get a span passed to the callback, it may just be a NonRecordingSpan if the span is not sampled.
  */
-export function startSpan<T>(spanContext: OpenTelemetrySpanContext, callback: (span: Span) => T): T {
+export function startSpan<T>(options: OpenTelemetrySpanContext, callback: (span: Span) => T): T {
   const tracer = getTracer();
 
-  const { name } = spanContext;
+  const { name } = options;
 
-  const activeCtx = context.active();
-  const shouldSkipSpan = spanContext.onlyIfParent && !trace.getSpan(activeCtx);
+  const activeCtx = getContext(options.scope);
+  const shouldSkipSpan = options.onlyIfParent && !trace.getSpan(activeCtx);
   const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
 
+  const spanContext = getSpanContext(options);
+
   return tracer.startActiveSpan(name, spanContext, ctx, span => {
-    _applySentryAttributesToSpan(span, spanContext);
+    _applySentryAttributesToSpan(span, options);
 
     return handleCallbackErrors(
       () => callback(span),
@@ -49,17 +52,19 @@ export function startSpan<T>(spanContext: OpenTelemetrySpanContext, callback: (s
  *
  * Note that you'll always get a span passed to the callback, it may just be a NonRecordingSpan if the span is not sampled.
  */
-export function startSpanManual<T>(spanContext: OpenTelemetrySpanContext, callback: (span: Span) => T): T {
+export function startSpanManual<T>(options: OpenTelemetrySpanContext, callback: (span: Span) => T): T {
   const tracer = getTracer();
 
-  const { name } = spanContext;
+  const { name } = options;
 
-  const activeCtx = context.active();
-  const shouldSkipSpan = spanContext.onlyIfParent && !trace.getSpan(activeCtx);
+  const activeCtx = getContext(options.scope);
+  const shouldSkipSpan = options.onlyIfParent && !trace.getSpan(activeCtx);
   const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
 
+  const spanContext = getSpanContext(options);
+
   return tracer.startActiveSpan(name, spanContext, ctx, span => {
-    _applySentryAttributesToSpan(span, spanContext);
+    _applySentryAttributesToSpan(span, options);
 
     return handleCallbackErrors(
       () => callback(span),
@@ -85,18 +90,20 @@ export const startActiveSpan = startSpan;
  * or you didn't set `tracesSampleRate` or `tracesSampler`, this function will not generate spans
  * and the `span` returned from the callback will be undefined.
  */
-export function startInactiveSpan(spanContext: OpenTelemetrySpanContext): Span {
+export function startInactiveSpan(options: OpenTelemetrySpanContext): Span {
   const tracer = getTracer();
 
-  const { name } = spanContext;
+  const { name } = options;
 
-  const activeCtx = context.active();
-  const shouldSkipSpan = spanContext.onlyIfParent && !trace.getSpan(activeCtx);
+  const activeCtx = getContext(options.scope);
+  const shouldSkipSpan = options.onlyIfParent && !trace.getSpan(activeCtx);
   const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx;
 
+  const spanContext = getSpanContext(options);
+
   const span = tracer.startSpan(name, spanContext, ctx);
 
-  _applySentryAttributesToSpan(span, spanContext);
+  _applySentryAttributesToSpan(span, options);
 
   return span;
 }
@@ -120,8 +127,9 @@ function getTracer(): Tracer {
   return (client && client.tracer) || trace.getTracer('@sentry/opentelemetry', SDK_VERSION);
 }
 
-function _applySentryAttributesToSpan(span: Span, spanContext: OpenTelemetrySpanContext): void {
-  const { origin, op, source, metadata } = spanContext;
+function _applySentryAttributesToSpan(span: Span, options: OpenTelemetrySpanContext): void {
+  // eslint-disable-next-line deprecation/deprecation
+  const { origin, op, source, metadata } = options;
 
   if (origin) {
     span.setAttribute(InternalSentrySemanticAttributes.ORIGIN, origin);
@@ -139,3 +147,32 @@ function _applySentryAttributesToSpan(span: Span, spanContext: OpenTelemetrySpan
     setSpanMetadata(span, metadata);
   }
 }
+
+function getSpanContext(options: OpenTelemetrySpanContext): SpanOptions {
+  const { startTime, attributes, kind } = options;
+
+  // OTEL expects timestamps in ms, not seconds
+  const fixedStartTime = typeof startTime === 'number' ? ensureTimestampInMilliseconds(startTime) : startTime;
+
+  return {
+    attributes,
+    kind,
+    startTime: fixedStartTime,
+  };
+}
+
+function ensureTimestampInMilliseconds(timestamp: number): number {
+  const isMs = timestamp < 9999999999;
+  return isMs ? timestamp * 1000 : timestamp;
+}
+
+function getContext(scope?: Scope): Context {
+  if (scope) {
+    const ctx = getContextFromScope(scope);
+    if (ctx) {
+      return ctx;
+    }
+  }
+
+  return context.active();
+}
diff --git a/packages/opentelemetry/src/types.ts b/packages/opentelemetry/src/types.ts
@@ -1,25 +1,15 @@
-import type { Attributes, Span as WriteableSpan, SpanKind, TimeInput, Tracer } from '@opentelemetry/api';
+import type { Span as WriteableSpan, SpanKind, Tracer } from '@opentelemetry/api';
 import type { BasicTracerProvider, ReadableSpan, Span } from '@opentelemetry/sdk-trace-base';
-import type { Scope, SpanOrigin, TransactionMetadata, TransactionSource } from '@sentry/types';
+import type { Scope, StartSpanOptions } from '@sentry/types';
 
 export interface OpenTelemetryClient {
   tracer: Tracer;
   traceProvider: BasicTracerProvider | undefined;
 }
 
-export interface OpenTelemetrySpanContext {
-  name: string;
-  op?: string;
-  metadata?: Partial<TransactionMetadata>;
-  origin?: SpanOrigin;
-  source?: TransactionSource;
-  scope?: Scope;
-  onlyIfParent?: boolean;
-
-  // Base SpanOptions we support
-  attributes?: Attributes;
+export interface OpenTelemetrySpanContext extends StartSpanOptions {
+  // Additional otel-only option, for now...?
   kind?: SpanKind;
-  startTime?: TimeInput;
 }
 
 /**
diff --git a/packages/opentelemetry/src/utils/contextData.ts b/packages/opentelemetry/src/utils/contextData.ts
@@ -55,12 +55,17 @@ export function getScopesFromContext(context: Context): CurrentScopes | undefine
  * This will return a forked context with the Propagation Context set.
  */
 export function setScopesOnContext(context: Context, scopes: CurrentScopes): Context {
-  // So we can look up the context from the scope later
-  SCOPE_CONTEXT_MAP.set(scopes.scope, context);
-
   return context.setValue(SENTRY_SCOPES_CONTEXT_KEY, scopes);
 }
 
+/**
+ * Set the context on the scope so we can later look it up.
+ * We need this to get the context from the scope in the `trace` functions.
+ */
+export function setContextOnScope(scope: Scope, context: Context): void {
+  SCOPE_CONTEXT_MAP.set(scope, context);
+}
+
 /**
  * Get the context related to a scope.
  * TODO v8: Use this for the `trace` functions.
diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts
