Skip to content

Commit

Permalink
fix(sdk-trace-base): processor onStart called with a span having empt…
Browse files Browse the repository at this point in the history
…y attributes (#4277)

Co-authored-by: artahmetaj <artahmetaj@yahoo.com>
  • Loading branch information
satazor and ArtAhmetaj authored Nov 15, 2023
1 parent b0c0ace commit 10f6c46
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 7 additions & 1 deletion packages/opentelemetry-sdk-trace-base/src/Span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 =
Expand Down
16 changes: 9 additions & 7 deletions packages/opentelemetry-sdk-trace-base/src/Tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}

Expand Down
39 changes: 39 additions & 0 deletions packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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' });
});
});
});
});

0 comments on commit 10f6c46

Please sign in to comment.