From d8907d8fe731067418b1857b1e48ab4f9e0946dd Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 30 Oct 2020 12:00:04 -0400 Subject: [PATCH] feat: span processor onstart recieves context (#1632) --- .../test/common/transform.test.ts | 30 ++- .../test/functionals/utils.test.ts | 8 +- .../src/BasicTracerProvider.ts | 2 +- .../src/MultiSpanProcessor.ts | 5 +- .../src/NoopSpanProcessor.ts | 3 +- packages/opentelemetry-tracing/src/Span.ts | 5 +- .../src/SpanProcessor.ts | 3 +- packages/opentelemetry-tracing/src/Tracer.ts | 1 + .../opentelemetry-tracing/test/Span.test.ts | 202 +++++++++++++++--- .../test/export/SimpleSpanProcessor.test.ts | 26 ++- 10 files changed, 233 insertions(+), 52 deletions(-) diff --git a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts index 04030944f9..4b97df3a85 100644 --- a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts @@ -14,24 +14,24 @@ * limitations under the License. */ -import * as assert from 'assert'; import * as api from '@opentelemetry/api'; -import { Span, BasicTracerProvider } from '@opentelemetry/tracing'; import { - NoopLogger, - hrTimeToMicroseconds, hrTimeDuration, + hrTimeToMicroseconds, + NoopLogger, VERSION, } from '@opentelemetry/core'; +import { Resource, TELEMETRY_SDK_RESOURCE } from '@opentelemetry/resources'; +import { BasicTracerProvider, Span } from '@opentelemetry/tracing'; +import * as assert from 'assert'; import { - toZipkinSpan, - _toZipkinTags, - _toZipkinAnnotations, statusCodeTagName, statusDescriptionTagName, + toZipkinSpan, + _toZipkinAnnotations, + _toZipkinTags, } from '../../src/transform'; import * as zipkinTypes from '../../src/types'; -import { Resource, TELEMETRY_SDK_RESOURCE } from '@opentelemetry/resources'; const logger = new NoopLogger(); const tracer = new BasicTracerProvider({ logger, @@ -57,6 +57,7 @@ describe('transform', () => { it('should convert an OpenTelemetry span to a Zipkin span', () => { const span = new Span( tracer, + api.ROOT_CONTEXT, 'my-span', spanContext, api.SpanKind.SERVER, @@ -107,6 +108,7 @@ describe('transform', () => { it("should skip parentSpanId if doesn't exist", () => { const span = new Span( tracer, + api.ROOT_CONTEXT, 'my-span', spanContext, api.SpanKind.SERVER @@ -152,7 +154,13 @@ describe('transform', () => { it(`should map OpenTelemetry SpanKind ${ api.SpanKind[item.ot] } to Zipkin ${item.zipkin}`, () => { - const span = new Span(tracer, 'my-span', spanContext, item.ot); + const span = new Span( + tracer, + api.ROOT_CONTEXT, + 'my-span', + spanContext, + item.ot + ); span.end(); const zipkinSpan = toZipkinSpan( @@ -190,6 +198,7 @@ describe('transform', () => { it('should convert OpenTelemetry attributes to Zipkin tags', () => { const span = new Span( tracer, + api.ROOT_CONTEXT, 'my-span', spanContext, api.SpanKind.SERVER, @@ -219,6 +228,7 @@ describe('transform', () => { it('should map OpenTelemetry Status.code to a Zipkin tag', () => { const span = new Span( tracer, + api.ROOT_CONTEXT, 'my-span', spanContext, api.SpanKind.SERVER, @@ -249,6 +259,7 @@ describe('transform', () => { it('should map OpenTelemetry Status.message to a Zipkin tag', () => { const span = new Span( tracer, + api.ROOT_CONTEXT, 'my-span', spanContext, api.SpanKind.SERVER, @@ -284,6 +295,7 @@ describe('transform', () => { it('should convert OpenTelemetry events to Zipkin annotations', () => { const span = new Span( tracer, + api.ROOT_CONTEXT, 'my-span', spanContext, api.SpanKind.SERVER, diff --git a/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts b/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts index e69d6b7252..dbb21c2502 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts @@ -13,7 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { CanonicalCode, SpanKind, TraceFlags } from '@opentelemetry/api'; +import { + CanonicalCode, + ROOT_CONTEXT, + SpanKind, + TraceFlags, +} from '@opentelemetry/api'; import { NoopLogger } from '@opentelemetry/core'; import { BasicTracerProvider, Span } from '@opentelemetry/tracing'; import { HttpAttribute } from '@opentelemetry/semantic-conventions'; @@ -256,6 +261,7 @@ describe('Utility', () => { for (const obj of [undefined, { statusCode: 400 }]) { const span = new Span( new BasicTracerProvider().getTracer('default'), + ROOT_CONTEXT, 'test', { spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED }, SpanKind.INTERNAL diff --git a/packages/opentelemetry-tracing/src/BasicTracerProvider.ts b/packages/opentelemetry-tracing/src/BasicTracerProvider.ts index aa49abd1a0..570e2ed66d 100644 --- a/packages/opentelemetry-tracing/src/BasicTracerProvider.ts +++ b/packages/opentelemetry-tracing/src/BasicTracerProvider.ts @@ -36,7 +36,7 @@ export class BasicTracerProvider implements api.TracerProvider { private readonly _registeredSpanProcessors: SpanProcessor[] = []; private readonly _tracers: Map = new Map(); - activeSpanProcessor = new NoopSpanProcessor(); + activeSpanProcessor: SpanProcessor = new NoopSpanProcessor(); readonly logger: api.Logger; readonly resource: Resource; diff --git a/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts b/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts index 7300e7e78a..f5eb8f772e 100644 --- a/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { Context } from '@opentelemetry/api'; import { globalErrorHandler } from '@opentelemetry/core'; import { ReadableSpan } from './export/ReadableSpan'; import { Span } from './Span'; @@ -46,9 +47,9 @@ export class MultiSpanProcessor implements SpanProcessor { }); } - onStart(span: Span): void { + onStart(span: Span, context: Context): void { for (const spanProcessor of this._spanProcessors) { - spanProcessor.onStart(span); + spanProcessor.onStart(span, context); } } diff --git a/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts b/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts index 6e7c446b7b..0d0e9c99e8 100644 --- a/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts @@ -14,13 +14,14 @@ * limitations under the License. */ +import { Context } from '@opentelemetry/api'; import { ReadableSpan } from './export/ReadableSpan'; import { Span } from './Span'; import { SpanProcessor } from './SpanProcessor'; /** No-op implementation of SpanProcessor */ export class NoopSpanProcessor implements SpanProcessor { - onStart(span: Span): void {} + onStart(span: Span, context: Context): void {} onEnd(span: ReadableSpan): void {} shutdown(): Promise { return Promise.resolve(); diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts index 8669f899dc..61d0869b6a 100644 --- a/packages/opentelemetry-tracing/src/Span.ts +++ b/packages/opentelemetry-tracing/src/Span.ts @@ -32,7 +32,7 @@ import { ReadableSpan } from './export/ReadableSpan'; import { Tracer } from './Tracer'; import { SpanProcessor } from './SpanProcessor'; import { TraceParams } from './types'; -import { AttributeValue } from '@opentelemetry/api'; +import { AttributeValue, Context } from '@opentelemetry/api'; /** * This class represents a span. @@ -63,6 +63,7 @@ export class Span implements api.Span, ReadableSpan { /** Constructs a new Span instance. */ constructor( parentTracer: Tracer, + context: Context, spanName: string, spanContext: api.SpanContext, kind: api.SpanKind, @@ -81,7 +82,7 @@ export class Span implements api.Span, ReadableSpan { this._logger = parentTracer.logger; this._traceParams = parentTracer.getActiveTraceParams(); this._spanProcessor = parentTracer.getActiveSpanProcessor(); - this._spanProcessor.onStart(this); + this._spanProcessor.onStart(this, context); } context(): api.SpanContext { diff --git a/packages/opentelemetry-tracing/src/SpanProcessor.ts b/packages/opentelemetry-tracing/src/SpanProcessor.ts index 3292236025..dc9b2232fd 100644 --- a/packages/opentelemetry-tracing/src/SpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/SpanProcessor.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { Context } from '@opentelemetry/api'; import { ReadableSpan } from './export/ReadableSpan'; import { Span } from './Span'; @@ -32,7 +33,7 @@ export interface SpanProcessor { * returns true. * @param span the Span that just started. */ - onStart(span: Span): void; + onStart(span: Span, parentContext: Context): void; /** * Called when a {@link ReadableSpan} is ended, if the `span.isRecording()` diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts index 7addec465d..0bdb41d2a7 100644 --- a/packages/opentelemetry-tracing/src/Tracer.ts +++ b/packages/opentelemetry-tracing/src/Tracer.ts @@ -109,6 +109,7 @@ export class Tracer implements api.Tracer { const span = new Span( this, + context, name, spanContext, spanKind, diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts index 79e404a966..91d67d8670 100644 --- a/packages/opentelemetry-tracing/test/Span.test.ts +++ b/packages/opentelemetry-tracing/test/Span.test.ts @@ -18,6 +18,7 @@ import { CanonicalCode, Exception, LinkContext, + ROOT_CONTEXT, SpanContext, SpanKind, TraceFlags, @@ -51,13 +52,25 @@ describe('Span', () => { }; it('should create a Span instance', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.SERVER + ); assert.ok(span instanceof Span); span.end(); }); it('should have valid startTime', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.SERVER + ); assert.ok( hrTimeToMilliseconds(span.startTime) > hrTimeToMilliseconds(performanceTimeOrigin) @@ -65,7 +78,13 @@ describe('Span', () => { }); it('should have valid endTime', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.SERVER + ); span.end(); assert.ok( hrTimeToNanoseconds(span.endTime) >= hrTimeToNanoseconds(span.startTime), @@ -80,13 +99,25 @@ describe('Span', () => { }); it('should have a duration', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.SERVER + ); span.end(); assert.ok(hrTimeToNanoseconds(span.duration) >= 0); }); it('should have valid event.time', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.SERVER + ); span.addEvent('my-event'); assert.ok( hrTimeToMilliseconds(span.events[0].time) > @@ -97,6 +128,7 @@ describe('Span', () => { it('should have an entered time for event', () => { const span = new Span( tracer, + ROOT_CONTEXT, name, spanContext, SpanKind.SERVER, @@ -118,6 +150,7 @@ describe('Span', () => { it('should have an entered time for event - ', () => { const span = new Span( tracer, + ROOT_CONTEXT, name, spanContext, SpanKind.SERVER, @@ -137,7 +170,13 @@ describe('Span', () => { }); it('should get the span context of span', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); const context = span.context(); assert.strictEqual(context.traceId, spanContext.traceId); assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED); @@ -148,13 +187,25 @@ describe('Span', () => { }); it('should return true when isRecording:true', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); assert.ok(span.isRecording()); span.end(); }); it('should set an attribute', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); span.setAttribute('string', 'string'); span.setAttribute('number', 0); @@ -179,7 +230,13 @@ describe('Span', () => { }); it('should overwrite attributes', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); span.setAttribute('overwrite', 'initial value'); span.setAttribute('overwrite', 'overwritten value'); @@ -190,7 +247,13 @@ describe('Span', () => { }); it('should set attributes', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); span.setAttributes({ string: 'string', @@ -216,7 +279,13 @@ describe('Span', () => { }); it('should set an event', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); span.addEvent('sent'); span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true }); span.end(); @@ -233,15 +302,26 @@ describe('Span', () => { spanId: '6e0c63257de34c92', }; const attributes = { attr1: 'value', attr2: 123, attr3: true }; - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT, '12345', [ - { context: linkContext }, - { context: linkContext, attributes }, - ]); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT, + '12345', + [{ context: linkContext }, { context: linkContext, attributes }] + ); span.end(); }); it('should drop extra links, attributes and events', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); for (let i = 0; i < 150; i++) { span.setAttribute('foo' + i, 'bar' + i); span.addEvent('sent' + i); @@ -256,7 +336,13 @@ describe('Span', () => { }); it('should set an error status', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); span.setStatus({ code: CanonicalCode.PERMISSION_DENIED, message: 'This is an error', @@ -268,6 +354,7 @@ describe('Span', () => { const parentId = '5c1c63257de34c67'; const span = new Span( tracer, + ROOT_CONTEXT, 'my-span', spanContext, SpanKind.INTERNAL, @@ -292,7 +379,13 @@ describe('Span', () => { }); it('should return ReadableSpan with attributes', () => { - const span = new Span(tracer, 'my-span', spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + 'my-span', + spanContext, + SpanKind.CLIENT + ); span.setAttribute('attr1', 'value1'); assert.deepStrictEqual(span.attributes, { attr1: 'value1' }); @@ -314,6 +407,7 @@ describe('Span', () => { it('should return ReadableSpan with links', () => { const span = new Span( tracer, + ROOT_CONTEXT, 'my-span', spanContext, SpanKind.CLIENT, @@ -341,7 +435,13 @@ describe('Span', () => { }); it('should return ReadableSpan with events', () => { - const span = new Span(tracer, 'my-span', spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + 'my-span', + spanContext, + SpanKind.CLIENT + ); span.addEvent('sent'); assert.strictEqual(span.events.length, 1); const [event] = span.events; @@ -370,7 +470,13 @@ describe('Span', () => { }); it('should return ReadableSpan with new status', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); span.setStatus({ code: CanonicalCode.PERMISSION_DENIED, message: 'This is an error', @@ -388,7 +494,13 @@ describe('Span', () => { }); it('should only end a span once', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.SERVER + ); const endTime = Date.now(); span.end(endTime); span.end(endTime + 10); @@ -396,7 +508,13 @@ describe('Span', () => { }); it('should update name', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.SERVER + ); span.updateName('foo-span'); span.end(); @@ -406,7 +524,13 @@ describe('Span', () => { }); it('should have ended', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.SERVER + ); assert.strictEqual(span.ended, false); span.end(); assert.strictEqual(span.ended, true); @@ -489,7 +613,13 @@ describe('Span', () => { invalidExceptions.forEach(key => { describe(`when exception is (${JSON.stringify(key)})`, () => { it('should NOT record an exception', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); assert.strictEqual(span.events.length, 0); span.recordException(key); assert.strictEqual(span.events.length, 0); @@ -503,7 +633,13 @@ describe('Span', () => { error = 'boom'; }); it('should record an exception', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); assert.strictEqual(span.events.length, 0); span.recordException(error); @@ -530,7 +666,13 @@ describe('Span', () => { describe(`when exception type is an object with ${errorObj.description}`, () => { const error: Exception = errorObj.obj; it('should record an exception', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); assert.strictEqual(span.events.length, 0); span.recordException(error); @@ -554,7 +696,13 @@ describe('Span', () => { describe('when time is provided', () => { it('should record an exception with provided time', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); assert.strictEqual(span.events.length, 0); span.recordException('boom', [0, 123]); const event = span.events[0]; diff --git a/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts index 1289343013..e647068d8b 100644 --- a/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts @@ -14,22 +14,28 @@ * limitations under the License. */ +import { + context, + ROOT_CONTEXT, + SpanContext, + SpanKind, + TraceFlags, +} from '@opentelemetry/api'; +import { + ExportResult, + loggingErrorHandler, + setGlobalErrorHandler, +} from '@opentelemetry/core'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { - Span, BasicTracerProvider, InMemorySpanExporter, SimpleSpanProcessor, + Span, } from '../../src'; -import { - ExportResult, - setGlobalErrorHandler, - loggingErrorHandler, -} from '@opentelemetry/core'; -import { SpanContext, SpanKind, TraceFlags, context } from '@opentelemetry/api'; -import { TestTracingSpanExporter } from './TestTracingSpanExporter'; import { TestStackContextManager } from './TestStackContextManager'; +import { TestTracingSpanExporter } from './TestTracingSpanExporter'; describe('SimpleSpanProcessor', () => { const provider = new BasicTracerProvider(); @@ -52,6 +58,7 @@ describe('SimpleSpanProcessor', () => { }; const span = new Span( provider.getTracer('default'), + ROOT_CONTEXT, 'span-name', spanContext, SpanKind.CLIENT @@ -75,6 +82,7 @@ describe('SimpleSpanProcessor', () => { }; const span = new Span( provider.getTracer('default'), + ROOT_CONTEXT, 'span-name', spanContext, SpanKind.CLIENT @@ -101,6 +109,7 @@ describe('SimpleSpanProcessor', () => { }; const span = new Span( provider.getTracer('default'), + ROOT_CONTEXT, 'span-name', spanContext, SpanKind.CLIENT @@ -176,6 +185,7 @@ describe('SimpleSpanProcessor', () => { }; const span = new Span( provider.getTracer('default'), + ROOT_CONTEXT, 'span-name', spanContext, SpanKind.CLIENT