fix(sdk-trace-base): processor onStart called with a span having empt…

…y attributes (#4277) Co-authored-by: artahmetaj <artahmetaj@yahoo.com>
open-telemetry · Nov 15, 2023 · 10f6c46 · 10f6c46
1 parent b0c0ace
commit 10f6c46
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 ### :bug: (Bug Fix)
 
+* fix(sdk-trace-base): processor onStart called with a span having empty attributes
+
 ## 1.18.1
 
 ### :bug: (Bug Fix)

diff --git a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts
@@ -236,6 +236,40 @@ describe('transform', () => {
         version: '1',
       });
     });
+    it('should map OpenTelemetry constructor attributes to a Zipkin tag', () => {
+      const span = new Span(
+        tracer,
+        api.ROOT_CONTEXT,
+        'my-span',
+        spanContext,
+        api.SpanKind.SERVER,
+        parentId,
+        [],
+        undefined,
+        undefined,
+        {
+          key1: 'value1',
+          key2: 'value2',
+        }
+      );
+      const tags: zipkinTypes.Tags = _toZipkinTags(
+        span,
+        defaultStatusCodeTagName,
+        defaultStatusErrorTagName
+      );
+
+      assert.deepStrictEqual(tags, {
+        key1: 'value1',
+        key2: 'value2',
+        [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
+        'telemetry.sdk.language': language,
+        'telemetry.sdk.name': 'opentelemetry',
+        'telemetry.sdk.version': VERSION,
+        cost: '112.12',
+        service: 'ui',
+        version: '1',
+      });
+    });
     it('should map OpenTelemetry SpanStatus.code to a Zipkin tag', () => {
       const span = new Span(
         tracer,

diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts
@@ -100,7 +100,8 @@ export class Span implements APISpan, ReadableSpan {
     parentSpanId?: string,
     links: Link[] = [],
     startTime?: TimeInput,
-    _deprecatedClock?: unknown // keeping this argument even though it is unused to ensure backwards compatibility
+    _deprecatedClock?: unknown, // keeping this argument even though it is unused to ensure backwards compatibility
+    attributes?: SpanAttributes
   ) {
     this.name = spanName;
     this._spanContext = spanContext;
@@ -119,6 +120,11 @@ export class Span implements APISpan, ReadableSpan {
     this.resource = parentTracer.resource;
     this.instrumentationLibrary = parentTracer.instrumentationLibrary;
     this._spanLimits = parentTracer.getSpanLimits();
+
+    if (attributes != null) {
+      this.setAttributes(attributes);
+    }
+
     this._spanProcessor = parentTracer.getActiveSpanProcessor();
     this._spanProcessor.onStart(this, context);
     this._attributeValueLengthLimit =

diff --git a/packages/opentelemetry-sdk-trace-base/src/Tracer.ts b/packages/opentelemetry-sdk-trace-base/src/Tracer.ts
@@ -132,6 +132,12 @@ export class Tracer implements api.Tracer {
       return nonRecordingSpan;
     }
 
+    // Set initial span attributes. The attributes object may have been mutated
+    // by the sampler, so we sanitize the merged attributes before setting them.
+    const initAttributes = sanitizeAttributes(
+      Object.assign(attributes, samplingResult.attributes)
+    );
+
     const span = new Span(
       this,
       context,
@@ -140,14 +146,10 @@ export class Tracer implements api.Tracer {
       spanKind,
       parentSpanId,
       links,
-      options.startTime
-    );
-    // Set initial span attributes. The attributes object may have been mutated
-    // by the sampler, so we sanitize the merged attributes before setting them.
-    const initAttributes = sanitizeAttributes(
-      Object.assign(attributes, samplingResult.attributes)
+      options.startTime,
+      undefined,
+      initAttributes
     );
-    span.setAttributes(initAttributes);
     return span;
   }
 

diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
@@ -1054,6 +1054,27 @@ describe('Span', () => {
       assert.ok(started);
     });
 
+    it('should include attributes in onStart', () => {
+      let attributes;
+      const processor: SpanProcessor = {
+        onStart: span => {
+          attributes = { ...span.attributes };
+        },
+        forceFlush: () => Promise.resolve(),
+        onEnd() {},
+        shutdown: () => Promise.resolve(),
+      };
+
+      const provider = new BasicTracerProvider();
+
+      provider.addSpanProcessor(processor);
+
+      provider
+        .getTracer('default')
+        .startSpan('test', { attributes: { foo: 'bar' } });
+      assert.deepStrictEqual(attributes, { foo: 'bar' });
+    });
+
     it('should call onEnd synchronously when span is ended', () => {
       let ended = false;
       const processor: SpanProcessor = {
@@ -1222,5 +1243,23 @@ describe('Span', () => {
         });
       });
     });
+
+    describe('when attributes are specified', () => {
+      it('should store specified attributes', () => {
+        const span = new Span(
+          tracer,
+          ROOT_CONTEXT,
+          name,
+          spanContext,
+          SpanKind.CLIENT,
+          undefined,
+          undefined,
+          undefined,
+          undefined,
+          { foo: 'bar' }
+        );
+        assert.deepStrictEqual(span.attributes, { foo: 'bar' });
+      });
+    });
   });
 });