diff --git a/CHANGELOG_NEXT.md b/CHANGELOG_NEXT.md index 79bfe2fc693..0fef0f2d1be 100644 --- a/CHANGELOG_NEXT.md +++ b/CHANGELOG_NEXT.md @@ -13,6 +13,7 @@ * refactor(sdk-trace-base)!: replace `SpanAttributes` with `Attributes` [#5009](/~https://github.com/open-telemetry/opentelemetry-js/pull/5009) @david-luna * refactor(resources)!: replace `ResourceAttributes` with `Attributes` [#5016](/~https://github.com/open-telemetry/opentelemetry-js/pull/5016) @david-luna * feat(sdk-metrics)!: drop `View` and `Aggregation` in favor of `ViewOptions` and `AggregationOption` [#4931](/~https://github.com/open-telemetry/opentelemetry-js/pull/4931) @pichlermarc +* refactor(sdk-trace-base)!: remove `new Span` constructor in favor of `Tracer.startSpan` API [#5048](/~https://github.com/open-telemetry/opentelemetry-js/pull/5048) @david-luna ### :rocket: (Enhancement) diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts index f61df5f1ffb..cfd23d46dba 100644 --- a/packages/opentelemetry-sdk-trace-base/src/Span.ts +++ b/packages/opentelemetry-sdk-trace-base/src/Span.ts @@ -48,17 +48,23 @@ import { SEMATTRS_EXCEPTION_STACKTRACE, SEMATTRS_EXCEPTION_TYPE, } from '@opentelemetry/semantic-conventions'; -import { ExceptionEventName } from './enums'; import { ReadableSpan } from './export/ReadableSpan'; +import { ExceptionEventName } from './enums'; import { SpanProcessor } from './SpanProcessor'; import { TimedEvent } from './TimedEvent'; import { Tracer } from './Tracer'; import { SpanLimits } from './types'; +/** + * This type provides the properties of @link{ReadableSpan} at the same time + * of the Span API + */ +export type Span = APISpan & ReadableSpan; + /** * This class represents a span. */ -export class Span implements APISpan, ReadableSpan { +export class SpanImpl implements Span { // Below properties are included to implement ReadableSpan for export // purposes but are not intended to be written-to directly. private readonly _spanContext: SpanContext; diff --git a/packages/opentelemetry-sdk-trace-base/src/Tracer.ts b/packages/opentelemetry-sdk-trace-base/src/Tracer.ts index f943e6a11cc..35799a030de 100644 --- a/packages/opentelemetry-sdk-trace-base/src/Tracer.ts +++ b/packages/opentelemetry-sdk-trace-base/src/Tracer.ts @@ -22,7 +22,7 @@ import { } from '@opentelemetry/core'; import { IResource } from '@opentelemetry/resources'; import { BasicTracerProvider } from './BasicTracerProvider'; -import { Span } from './Span'; +import { SpanImpl } from './Span'; import { GeneralLimits, SpanLimits, TracerConfig } from './types'; import { mergeConfig } from './utility'; import { SpanProcessor } from './SpanProcessor'; @@ -138,7 +138,7 @@ export class Tracer implements api.Tracer { Object.assign(attributes, samplingResult.attributes) ); - const span = new Span( + const span = new SpanImpl( this, context, name, diff --git a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts index 667949ce92b..92fe52605b6 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts @@ -42,6 +42,7 @@ import { AlwaysOnSampler, AlwaysOffSampler, } from '../../src'; +import { SpanImpl } from '../../src/Span'; class DummyPropagator implements TextMapPropagator { inject(context: Context, carrier: any, setter: TextMapSetter): void { @@ -582,7 +583,7 @@ describe('BasicTracerProvider', () => { const tracer = new BasicTracerProvider().getTracer('default'); const span = tracer.startSpan('my-span'); assert.ok(span); - assert.ok(span instanceof Span); + assert.ok(span instanceof SpanImpl); }); it('should propagate resources', () => { @@ -597,7 +598,7 @@ describe('BasicTracerProvider', () => { const tracer = new BasicTracerProvider().getTracer('default'); const span = tracer.startSpan('my-span', {}); assert.ok(span); - assert.ok(span instanceof Span); + assert.ok(span instanceof SpanImpl); const context = span.spanContext(); assert.ok(context.traceId.match(/[a-f0-9]{32}/)); assert.ok(context.spanId.match(/[a-f0-9]{16}/)); @@ -638,7 +639,7 @@ describe('BasicTracerProvider', () => { traceState: state, }) ); - assert.ok(span instanceof Span); + assert.ok(span instanceof SpanImpl); const context = span.spanContext(); assert.strictEqual(context.traceId, 'd4cda95b652f4a1592b449d5929fda1b'); assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED); @@ -691,7 +692,7 @@ describe('BasicTracerProvider', () => { 'invalid-parent' as unknown as SpanContext ) ); - assert.ok(span instanceof Span); + assert.ok(span instanceof SpanImpl); assert.deepStrictEqual((span as Span).parentSpanId, undefined); }); @@ -706,7 +707,7 @@ describe('BasicTracerProvider', () => { traceFlags: TraceFlags.SAMPLED, }) ); - assert.ok(span instanceof Span); + assert.ok(span instanceof SpanImpl); const context = span.spanContext(); assert.ok(context.traceId.match(/[a-f0-9]{32}/)); assert.ok(context.spanId.match(/[a-f0-9]{16}/)); @@ -733,7 +734,7 @@ describe('BasicTracerProvider', () => { sampler: new AlwaysOnSampler(), }).getTracer('default'); const span = tracer.startSpan('my-span'); - assert.ok(span instanceof Span); + assert.ok(span instanceof SpanImpl); assert.strictEqual(span.spanContext().traceFlags, TraceFlags.SAMPLED); assert.strictEqual(span.isRecording(), true); }); diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts index a676fddbb27..a08bd0d306f 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts @@ -42,6 +42,7 @@ import { import * as assert from 'assert'; import * as sinon from 'sinon'; import { BasicTracerProvider, Span, SpanProcessor } from '../../src'; +import { SpanImpl } from '../../src/Span'; import { invalidAttributes, validAttributes } from './util'; const performanceTimeOrigin: HrTime = [1, 1]; @@ -76,19 +77,19 @@ describe('Span', () => { }; it('should create a Span instance', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, spanContext, SpanKind.SERVER ); - assert.ok(span instanceof Span); + assert.ok(span instanceof SpanImpl); span.end(); }); it('should have valid startTime', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -102,7 +103,7 @@ describe('Span', () => { }); it('should have valid endTime', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -123,7 +124,7 @@ describe('Span', () => { }); it('should have a duration', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -135,7 +136,7 @@ describe('Span', () => { }); it('should ensure duration is never negative even if provided with inconsistent times', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -149,7 +150,7 @@ describe('Span', () => { }); it('should have valid event.time', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -165,7 +166,7 @@ describe('Span', () => { it('should have an entered time for event', () => { const startTime = Date.now(); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -188,7 +189,7 @@ describe('Span', () => { describe('when 2nd param is "TimeInput" type', () => { it('should have an entered time for event - ', () => { const startTime = Date.now(); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -210,7 +211,7 @@ describe('Span', () => { }); it('should get the span context of span', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -228,7 +229,7 @@ describe('Span', () => { describe('isRecording', () => { it('should return true when span is not ended', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -239,7 +240,7 @@ describe('Span', () => { span.end(); }); it('should return false when span is ended', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -254,7 +255,7 @@ describe('Span', () => { describe('setAttribute', () => { describe('when default options set', () => { it('should set an attribute', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -273,7 +274,7 @@ describe('Span', () => { }); it('should be able to overwrite attributes', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -299,7 +300,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -331,7 +332,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -375,7 +376,7 @@ describe('Span', () => { }); it('should truncate value when attributes are passed to the constructor', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -399,7 +400,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -436,7 +437,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -464,7 +465,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -516,7 +517,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -556,7 +557,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -588,7 +589,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -624,7 +625,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -680,7 +681,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -726,7 +727,7 @@ describe('Span', () => { describe('setAttributes', () => { it('should be able to set multiple attributes', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -743,7 +744,7 @@ describe('Span', () => { describe('addEvent', () => { it('should add an event', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -756,7 +757,7 @@ describe('Span', () => { }); it('should sanitize attribute values', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -776,7 +777,7 @@ describe('Span', () => { }); it('should drop extra events', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -803,7 +804,7 @@ describe('Span', () => { }); it('should store the count of dropped events in droppedEventsCount', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -825,7 +826,7 @@ describe('Span', () => { }, }).getTracer('default'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -841,7 +842,7 @@ describe('Span', () => { }); it('should set an error status', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -860,7 +861,7 @@ describe('Span', () => { it('should drop non-string status message', function () { const warnStub = sinon.spy(diag, 'warn'); - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -883,7 +884,7 @@ describe('Span', () => { it('should return ReadableSpan', () => { const parentId = '5c1c63257de34c67'; - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, 'my-span', @@ -910,7 +911,7 @@ describe('Span', () => { }); it('should return ReadableSpan with attributes', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, 'my-span', @@ -936,7 +937,7 @@ describe('Span', () => { }); it('should return ReadableSpan with links', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, 'my-span', @@ -966,7 +967,7 @@ describe('Span', () => { }); it('should be possible to add a link after span creation', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, 'my-span', @@ -987,7 +988,7 @@ describe('Span', () => { }); it('should be possible to add multiple links after span creation', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, 'my-span', @@ -1018,7 +1019,7 @@ describe('Span', () => { }); it('should return ReadableSpan with events', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, 'my-span', @@ -1053,7 +1054,7 @@ describe('Span', () => { }); it('should return ReadableSpan with new status', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1077,7 +1078,7 @@ describe('Span', () => { }); it('should only end a span once', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1091,7 +1092,7 @@ describe('Span', () => { }); it('should update name', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1107,7 +1108,7 @@ describe('Span', () => { }); it('should have ended', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1211,7 +1212,7 @@ describe('Span', () => { invalidExceptions.forEach(key => { describe(`when exception is (${JSON.stringify(key)})`, () => { it('should NOT record an exception', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1231,7 +1232,7 @@ describe('Span', () => { error = 'boom'; }); it('should record an exception', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1264,7 +1265,7 @@ 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( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1294,7 +1295,7 @@ describe('Span', () => { describe('when time is provided', () => { it('should record an exception with provided time', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1312,7 +1313,7 @@ describe('Span', () => { describe('when exception code is numeric', () => { it('should record an exception with string value', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, @@ -1330,7 +1331,7 @@ describe('Span', () => { describe('when attributes are specified', () => { it('should store specified attributes', () => { - const span = new Span( + const span = new SpanImpl( tracer, ROOT_CONTEXT, name, diff --git a/packages/opentelemetry-sdk-trace-base/test/common/export/SimpleSpanProcessor.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/export/SimpleSpanProcessor.test.ts index d1576ae08ae..fb3f556ef7d 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/export/SimpleSpanProcessor.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/export/SimpleSpanProcessor.test.ts @@ -32,8 +32,8 @@ import { BasicTracerProvider, InMemorySpanExporter, SimpleSpanProcessor, - Span, } from '../../../src'; +import { SpanImpl } from '../../../src/Span'; import { TestStackContextManager } from './TestStackContextManager'; import { TestTracingSpanExporter } from './TestTracingSpanExporter'; import { Attributes } from '@opentelemetry/api'; @@ -64,7 +64,7 @@ describe('SimpleSpanProcessor', () => { spanId: '5e0c63257de34c92', traceFlags: TraceFlags.SAMPLED, }; - const span = new Span( + const span = new SpanImpl( provider.getTracer('default'), ROOT_CONTEXT, 'span-name', @@ -88,7 +88,7 @@ describe('SimpleSpanProcessor', () => { spanId: '5e0c63257de34c92', traceFlags: TraceFlags.NONE, }; - const span = new Span( + const span = new SpanImpl( provider.getTracer('default'), ROOT_CONTEXT, 'span-name', @@ -113,7 +113,7 @@ describe('SimpleSpanProcessor', () => { spanId: '5e0c63257de34c92', traceFlags: TraceFlags.SAMPLED, }; - const span = new Span( + const span = new SpanImpl( provider.getTracer('default'), ROOT_CONTEXT, 'span-name', @@ -175,7 +175,7 @@ describe('SimpleSpanProcessor', () => { spanId: '5e0c63257de34c92', traceFlags: TraceFlags.SAMPLED, }; - const span = new Span( + const span = new SpanImpl( providerWithAsyncResource.getTracer('default'), ROOT_CONTEXT, 'span-name', @@ -216,7 +216,7 @@ describe('SimpleSpanProcessor', () => { spanId: '5e0c63257de34c92', traceFlags: TraceFlags.SAMPLED, }; - const span = new Span( + const span = new SpanImpl( providerWithAsyncResource.getTracer('default'), ROOT_CONTEXT, 'span-name', @@ -275,7 +275,7 @@ describe('SimpleSpanProcessor', () => { spanId: '5e0c63257de34c92', traceFlags: TraceFlags.SAMPLED, }; - const span = new Span( + const span = new SpanImpl( provider.getTracer('default'), ROOT_CONTEXT, 'span-name', @@ -304,7 +304,7 @@ describe('SimpleSpanProcessor', () => { // spanId: '5e0c63257de34c92', // traceFlags: TraceFlags.SAMPLED, // }; - // const span = new Span( + // const span = new SpanImpl( // providerWithAsyncResource.getTracer('default'), // ROOT_CONTEXT, // 'span-name', diff --git a/packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts index a656f6f69b5..6b7cacdb489 100644 --- a/packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts +++ b/packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts @@ -108,7 +108,6 @@ describe('NodeTracerProvider', () => { sampler: new AlwaysOnSampler(), }); const span = provider.getTracer('default').startSpan('my-span'); - assert.ok(span instanceof Span); assert.strictEqual(span.spanContext().traceFlags, TraceFlags.SAMPLED); assert.strictEqual(span.isRecording(), true); }); @@ -127,7 +126,6 @@ describe('NodeTracerProvider', () => { traceFlags: TraceFlags.NONE, }) ); - assert.ok(sampledParent instanceof Span); assert.strictEqual( sampledParent.spanContext().traceFlags, TraceFlags.SAMPLED @@ -141,7 +139,6 @@ describe('NodeTracerProvider', () => { {}, trace.setSpan(ROOT_CONTEXT, sampledParent) ); - assert.ok(span instanceof Span); assert.strictEqual(span.spanContext().traceFlags, TraceFlags.SAMPLED); assert.strictEqual(span.isRecording(), true); }); diff --git a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts index fb65e63acbf..11952d31e03 100644 --- a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts +++ b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts @@ -260,7 +260,7 @@ describe('OpenTracing Shim', () => { const otSpan = (span as SpanShim).getSpan() as Span; - const adjustment = otSpan['_performanceOffset']; + const adjustment = (otSpan as any)['_performanceOffset']; assert.strictEqual(otSpan.links.length, 1); assert.deepStrictEqual( @@ -496,7 +496,7 @@ describe('OpenTracing Shim', () => { it('sets explicit end timestamp', () => { const now = performance.now(); span.finish(now); - const adjustment = otSpan['_performanceOffset']; + const adjustment = (otSpan as any)['_performanceOffset']; assert.deepStrictEqual( hrTimeToMilliseconds(otSpan.endTime), now + adjustment + performance.timeOrigin