From 3ffd996fa20289d9fb7980639db4303c3b27414e Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Thu, 4 Feb 2021 10:20:39 +0100 Subject: [PATCH 1/3] chore: create NoopSpan instead reusing NOOP_SPAN Always create a fresh NoopSpan instead of returning the same NOOP_SPAN several times. Users may attach some data to a span or use it as a key in a map. Avoid issues/leaks in such cases by always returning a new NoopSpan. --- .../opentelemetry-api/src/trace/NoopSpan.ts | 2 -- .../opentelemetry-api/src/trace/NoopTracer.ts | 6 +++--- .../noop-implementations/noop-tracer.test.ts | 18 ++++++++---------- .../proxy-implementations/proxy-tracer.test.ts | 17 +++++++---------- packages/opentelemetry-tracing/src/Tracer.ts | 2 +- .../opentelemetry-tracing/test/Tracer.test.ts | 3 +-- 6 files changed, 20 insertions(+), 28 deletions(-) diff --git a/packages/opentelemetry-api/src/trace/NoopSpan.ts b/packages/opentelemetry-api/src/trace/NoopSpan.ts index 0c175ed44c..67d4e88aaa 100644 --- a/packages/opentelemetry-api/src/trace/NoopSpan.ts +++ b/packages/opentelemetry-api/src/trace/NoopSpan.ts @@ -73,5 +73,3 @@ export class NoopSpan implements Span { // By default does nothing recordException(_exception: Exception, _time?: TimeInput): void {} } - -export const NOOP_SPAN = new NoopSpan(); diff --git a/packages/opentelemetry-api/src/trace/NoopTracer.ts b/packages/opentelemetry-api/src/trace/NoopTracer.ts index bb25bcbe1d..683a7e5ab2 100644 --- a/packages/opentelemetry-api/src/trace/NoopTracer.ts +++ b/packages/opentelemetry-api/src/trace/NoopTracer.ts @@ -16,7 +16,7 @@ import { Span, SpanOptions, Tracer, SpanContext } from '..'; import { Context } from '@opentelemetry/context-base'; -import { NoopSpan, NOOP_SPAN } from './NoopSpan'; +import { NoopSpan } from './NoopSpan'; import { isSpanContextValid } from './spancontext-utils'; import { getSpanContext } from '../context/context'; @@ -28,7 +28,7 @@ export class NoopTracer implements Tracer { startSpan(name: string, options?: SpanOptions, context?: Context): Span { const root = Boolean(options?.root); if (root) { - return NOOP_SPAN; + return new NoopSpan(); } const parentFromContext = context && getSpanContext(context); @@ -39,7 +39,7 @@ export class NoopTracer implements Tracer { ) { return new NoopSpan(parentFromContext); } else { - return NOOP_SPAN; + return new NoopSpan(); } } } diff --git a/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts b/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts index 3730968323..1fcaa78db9 100644 --- a/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts +++ b/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts @@ -17,28 +17,26 @@ import * as assert from 'assert'; import { NoopTracer, - NOOP_SPAN, SpanContext, SpanKind, TraceFlags, context, setSpanContext, + NoopSpan, } from '../../src'; describe('NoopTracer', () => { it('should not crash', () => { const tracer = new NoopTracer(); - assert.deepStrictEqual(tracer.startSpan('span-name'), NOOP_SPAN); - assert.deepStrictEqual( - tracer.startSpan('span-name1', { kind: SpanKind.CLIENT }), - NOOP_SPAN + assert.ok(tracer.startSpan('span-name') instanceof NoopSpan); + assert.ok( + tracer.startSpan('span-name1', { kind: SpanKind.CLIENT }) instanceof + NoopSpan ); - assert.deepStrictEqual( - tracer.startSpan('span-name2', { - kind: SpanKind.CLIENT, - }), - NOOP_SPAN + assert.ok( + tracer.startSpan('span-name2', { kind: SpanKind.CLIENT }) instanceof + NoopSpan ); }); diff --git a/packages/opentelemetry-api/test/proxy-implementations/proxy-tracer.test.ts b/packages/opentelemetry-api/test/proxy-implementations/proxy-tracer.test.ts index 9170f3e44a..182ffdae67 100644 --- a/packages/opentelemetry-api/test/proxy-implementations/proxy-tracer.test.ts +++ b/packages/opentelemetry-api/test/proxy-implementations/proxy-tracer.test.ts @@ -18,7 +18,6 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { NoopSpan, - NOOP_SPAN, ProxyTracerProvider, SpanKind, TracerProvider, @@ -52,16 +51,14 @@ describe('ProxyTracer', () => { it('startSpan should return Noop Spans', () => { const tracer = provider.getTracer('test'); - assert.deepStrictEqual(tracer.startSpan('span-name'), NOOP_SPAN); - assert.deepStrictEqual( - tracer.startSpan('span-name1', { kind: SpanKind.CLIENT }), - NOOP_SPAN + assert.ok(tracer.startSpan('span-name') instanceof NoopSpan); + assert.ok( + tracer.startSpan('span-name1', { kind: SpanKind.CLIENT }) instanceof + NoopSpan ); - assert.deepStrictEqual( - tracer.startSpan('span-name2', { - kind: SpanKind.CLIENT, - }), - NOOP_SPAN + assert.ok( + tracer.startSpan('span-name2', { kind: SpanKind.CLIENT }) instanceof + NoopSpan ); }); }); diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts index fefdb00dfd..38754c4956 100644 --- a/packages/opentelemetry-tracing/src/Tracer.ts +++ b/packages/opentelemetry-tracing/src/Tracer.ts @@ -68,7 +68,7 @@ export class Tracer implements api.Tracer { ): api.Span { if (api.isInstrumentationSuppressed(context)) { this.logger.debug('Instrumentation suppressed, returning Noop Span'); - return api.NOOP_SPAN; + return new api.NoopSpan(); } const parentContext = getParent(options, context); diff --git a/packages/opentelemetry-tracing/test/Tracer.test.ts b/packages/opentelemetry-tracing/test/Tracer.test.ts index 288a823742..0f97ec2781 100644 --- a/packages/opentelemetry-tracing/test/Tracer.test.ts +++ b/packages/opentelemetry-tracing/test/Tracer.test.ts @@ -19,7 +19,6 @@ import { NoopSpan, Sampler, SamplingDecision, - NOOP_SPAN, TraceFlags, ROOT_CONTEXT, suppressInstrumentation, @@ -130,7 +129,7 @@ describe('Tracer', () => { const span = tracer.startSpan('span3', undefined, context); - assert.equal(span, NOOP_SPAN); + assert.ok(span instanceof NoopSpan); span.end(); done(); From 2b46339b8c6addaedede733a00e81a3d4f560b1e Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Fri, 5 Feb 2021 13:48:08 +0100 Subject: [PATCH 2/3] chore: use NOOP_TRACER.startSpan() Remove NoopSpan from API and use NOOP_TRACER.startSpan() to create them. Remove NoRecordingSpan and replace creation of them by NOOP_TRACER.startSpan(). --- .../opentelemetry-api/src/context/context.ts | 3 +- packages/opentelemetry-api/src/index.ts | 1 - .../opentelemetry-api/test/api/api.test.ts | 2 +- .../noop-implementations/noop-span.test.ts | 2 +- .../noop-implementations/noop-tracer.test.ts | 2 +- .../proxy-tracer.test.ts | 2 +- packages/opentelemetry-core/src/index.ts | 1 - .../src/trace/NoRecordingSpan.ts | 39 ------------------- .../test/trace/NoRecordingSpan.test.ts | 33 ---------------- .../src/http.ts | 13 ++----- .../test/functionals/http-disable.test.ts | 2 +- .../test/functionals/https-disable.test.ts | 2 +- .../test/NodeTracerProvider.test.ts | 7 +--- .../opentelemetry-plugin-http/src/http.ts | 15 ++----- .../test/functionals/http-disable.test.ts | 2 +- packages/opentelemetry-tracing/src/Tracer.ts | 9 +++-- .../test/BasicTracerProvider.test.ts | 5 +-- .../opentelemetry-tracing/test/Tracer.test.ts | 7 ++-- 18 files changed, 28 insertions(+), 119 deletions(-) delete mode 100644 packages/opentelemetry-core/src/trace/NoRecordingSpan.ts delete mode 100644 packages/opentelemetry-core/test/trace/NoRecordingSpan.test.ts diff --git a/packages/opentelemetry-api/src/context/context.ts b/packages/opentelemetry-api/src/context/context.ts index 4dc2b4c47c..86e7aabec3 100644 --- a/packages/opentelemetry-api/src/context/context.ts +++ b/packages/opentelemetry-api/src/context/context.ts @@ -15,7 +15,8 @@ */ import { Context, createContextKey } from '@opentelemetry/context-base'; -import { Baggage, NoopSpan, Span, SpanContext } from '../'; +import { Baggage, Span, SpanContext } from '../'; +import { NoopSpan } from '../trace/NoopSpan'; /** * span key diff --git a/packages/opentelemetry-api/src/index.ts b/packages/opentelemetry-api/src/index.ts index f1dcf8211a..d12fbbcd56 100644 --- a/packages/opentelemetry-api/src/index.ts +++ b/packages/opentelemetry-api/src/index.ts @@ -27,7 +27,6 @@ export * from './trace/Event'; export * from './trace/link_context'; export * from './trace/link'; export * from './trace/NoopLogger'; -export * from './trace/NoopSpan'; export * from './trace/NoopTracer'; export * from './trace/NoopTracerProvider'; export * from './trace/ProxyTracer'; diff --git a/packages/opentelemetry-api/test/api/api.test.ts b/packages/opentelemetry-api/test/api/api.test.ts index bb15fb02e9..1c31903727 100644 --- a/packages/opentelemetry-api/test/api/api.test.ts +++ b/packages/opentelemetry-api/test/api/api.test.ts @@ -17,7 +17,6 @@ import * as assert from 'assert'; import api, { TraceFlags, - NoopSpan, NoopTracerProvider, NoopTracer, SpanOptions, @@ -33,6 +32,7 @@ import api, { defaultTextMapSetter, defaultTextMapGetter, } from '../../src'; +import { NoopSpan } from '../../src/trace/NoopSpan'; describe('API', () => { it('should expose a tracer provider via getTracerProvider', () => { diff --git a/packages/opentelemetry-api/test/noop-implementations/noop-span.test.ts b/packages/opentelemetry-api/test/noop-implementations/noop-span.test.ts index f05beae0b7..d2974fca7e 100644 --- a/packages/opentelemetry-api/test/noop-implementations/noop-span.test.ts +++ b/packages/opentelemetry-api/test/noop-implementations/noop-span.test.ts @@ -19,9 +19,9 @@ import { SpanStatusCode, INVALID_SPANID, INVALID_TRACEID, - NoopSpan, TraceFlags, } from '../../src'; +import { NoopSpan } from '../../src/trace/NoopSpan'; describe('NoopSpan', () => { it('do not crash', () => { diff --git a/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts b/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts index 1fcaa78db9..661ff34dbc 100644 --- a/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts +++ b/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts @@ -22,8 +22,8 @@ import { TraceFlags, context, setSpanContext, - NoopSpan, } from '../../src'; +import { NoopSpan } from '../../src/trace/NoopSpan'; describe('NoopTracer', () => { it('should not crash', () => { diff --git a/packages/opentelemetry-api/test/proxy-implementations/proxy-tracer.test.ts b/packages/opentelemetry-api/test/proxy-implementations/proxy-tracer.test.ts index 182ffdae67..c6e2756ca5 100644 --- a/packages/opentelemetry-api/test/proxy-implementations/proxy-tracer.test.ts +++ b/packages/opentelemetry-api/test/proxy-implementations/proxy-tracer.test.ts @@ -17,7 +17,6 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { - NoopSpan, ProxyTracerProvider, SpanKind, TracerProvider, @@ -28,6 +27,7 @@ import { ROOT_CONTEXT, SpanOptions, } from '../../src'; +import { NoopSpan } from '../../src/trace/NoopSpan'; describe('ProxyTracer', () => { let provider: ProxyTracerProvider; diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index a463bbc871..0a57beb228 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -27,7 +27,6 @@ export * from './context/propagation/HttpTraceContext'; export * from './context/propagation/types'; export * from './baggage/propagation/HttpBaggage'; export * from './platform'; -export * from './trace/NoRecordingSpan'; export * from './trace/Plugin'; export * from './trace/sampler/AlwaysOffSampler'; export * from './trace/sampler/AlwaysOnSampler'; diff --git a/packages/opentelemetry-core/src/trace/NoRecordingSpan.ts b/packages/opentelemetry-core/src/trace/NoRecordingSpan.ts deleted file mode 100644 index 589ea5c1cf..0000000000 --- a/packages/opentelemetry-core/src/trace/NoRecordingSpan.ts +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - SpanContext, - NoopSpan, - INVALID_SPAN_CONTEXT, -} from '@opentelemetry/api'; - -/** - * The NoRecordingSpan extends the {@link NoopSpan}, making all operations no-op - * except context propagation. - */ -export class NoRecordingSpan extends NoopSpan { - private readonly _context: SpanContext; - - constructor(spanContext: SpanContext) { - super(spanContext); - this._context = spanContext || INVALID_SPAN_CONTEXT; - } - - // Returns a SpanContext. - context(): SpanContext { - return this._context; - } -} diff --git a/packages/opentelemetry-core/test/trace/NoRecordingSpan.test.ts b/packages/opentelemetry-core/test/trace/NoRecordingSpan.test.ts deleted file mode 100644 index 87bc315c9e..0000000000 --- a/packages/opentelemetry-core/test/trace/NoRecordingSpan.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import * as assert from 'assert'; -import { NoRecordingSpan } from '../../src/trace/NoRecordingSpan'; -import { TraceFlags } from '@opentelemetry/api'; - -describe('NoRecordingSpan', () => { - it('propagates span contexts', () => { - const spanContext = { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.NONE, - }; - - const span = new NoRecordingSpan(spanContext); - assert.strictEqual(span.context(), spanContext); - span.end(); - }); -}); diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts index 22a6fd796b..ca61b947e9 100644 --- a/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/packages/opentelemetry-instrumentation-http/src/http.ts @@ -22,13 +22,11 @@ import { SpanOptions, SpanStatus, setSpan, - SpanContext, - TraceFlags, ROOT_CONTEXT, getSpan, suppressInstrumentation, + NOOP_TRACER, } from '@opentelemetry/api'; -import { NoRecordingSpan } from '@opentelemetry/core'; import type * as http from 'http'; import type * as https from 'https'; import { Socket } from 'net'; @@ -61,11 +59,6 @@ export class HttpInstrumentation extends InstrumentationBase { /** keep track on spans not ended */ private readonly _spanNotEnded: WeakSet = new WeakSet(); private readonly _version = process.versions.node; - private readonly _emptySpanContext: SpanContext = { - traceId: '', - spanId: '', - traceFlags: TraceFlags.NONE, - }; constructor(config: HttpInstrumentationConfig & InstrumentationConfig = {}) { super( @@ -580,7 +573,7 @@ export class HttpInstrumentation extends InstrumentationBase { private _startHttpSpan(name: string, options: SpanOptions) { /* - * If a parent is required but not present, we use a `NoRecordingSpan` to still + * If a parent is required but not present, we use a `NoopSpan` to still * propagate context without recording it. */ const requireParent = @@ -594,7 +587,7 @@ export class HttpInstrumentation extends InstrumentationBase { if (requireParent === true && currentSpan === undefined) { // TODO: Refactor this when a solution is found in // https://github.com/open-telemetry/opentelemetry-specification/issues/530 - span = new NoRecordingSpan(this._emptySpanContext); + span = NOOP_TRACER.startSpan(name, options); } else if (requireParent === true && currentSpan?.context().isRemote) { span = currentSpan; } else { diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/http-disable.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/http-disable.test.ts index 1fff7ea5a0..722922b26f 100644 --- a/packages/opentelemetry-instrumentation-http/test/functionals/http-disable.test.ts +++ b/packages/opentelemetry-instrumentation-http/test/functionals/http-disable.test.ts @@ -57,7 +57,7 @@ describe('HttpInstrumentation', () => { }); beforeEach(() => { - NOOP_TRACER.startSpan = sinon.spy(); + sinon.spy(NOOP_TRACER, 'startSpan'); }); afterEach(() => { diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/https-disable.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/https-disable.test.ts index 795b304de6..254c6eda18 100644 --- a/packages/opentelemetry-instrumentation-http/test/functionals/https-disable.test.ts +++ b/packages/opentelemetry-instrumentation-http/test/functionals/https-disable.test.ts @@ -66,7 +66,7 @@ describe('HttpsInstrumentation', () => { }); beforeEach(() => { - NOOP_TRACER.startSpan = sinon.spy(); + sinon.spy(NOOP_TRACER, 'startSpan'); }); afterEach(() => { diff --git a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts index 851091b0ac..25f13ac1d8 100644 --- a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts +++ b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts @@ -22,11 +22,7 @@ import { setSpanContext, getSpan, } from '@opentelemetry/api'; -import { - AlwaysOnSampler, - AlwaysOffSampler, - NoRecordingSpan, -} from '@opentelemetry/core'; +import { AlwaysOnSampler, AlwaysOffSampler } from '@opentelemetry/core'; import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; import { Span } from '@opentelemetry/tracing'; import { Resource, TELEMETRY_SDK_RESOURCE } from '@opentelemetry/resources'; @@ -144,7 +140,6 @@ describe('NodeTracerProvider', () => { logger: new NoopLogger(), }); const span = provider.getTracer('default').startSpan('my-span'); - assert.ok(span instanceof NoRecordingSpan); assert.strictEqual(span.context().traceFlags, TraceFlags.NONE); assert.strictEqual(span.isRecording(), false); }); diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 676a2a3209..1efeb33dc1 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -21,14 +21,13 @@ import { SpanKind, SpanOptions, SpanStatus, - SpanContext, - TraceFlags, setSpan, ROOT_CONTEXT, getSpan, suppressInstrumentation, + NOOP_TRACER, } from '@opentelemetry/api'; -import { BasePlugin, NoRecordingSpan } from '@opentelemetry/core'; +import { BasePlugin } from '@opentelemetry/core'; import type { ClientRequest, IncomingMessage, @@ -60,12 +59,6 @@ export class HttpPlugin extends BasePlugin { /** keep track on spans not ended */ private readonly _spanNotEnded: WeakSet; - private readonly _emptySpanContext: SpanContext = { - traceId: '', - spanId: '', - traceFlags: TraceFlags.NONE, - }; - constructor(readonly moduleName: string, readonly version: string) { super(`@opentelemetry/plugin-${moduleName}`, VERSION); // For now component is equal to moduleName but it can change in the future. @@ -439,7 +432,7 @@ export class HttpPlugin extends BasePlugin { private _startHttpSpan(name: string, options: SpanOptions) { /* - * If a parent is required but not present, we use a `NoRecordingSpan` to still + * If a parent is required but not present, we use a `NoopSpan` to still * propagate context without recording it. */ const requireParent = @@ -453,7 +446,7 @@ export class HttpPlugin extends BasePlugin { if (requireParent === true && currentSpan === undefined) { // TODO: Refactor this when a solution is found in // https://github.com/open-telemetry/opentelemetry-specification/issues/530 - span = new NoRecordingSpan(plugin._emptySpanContext); + span = NOOP_TRACER.startSpan(name, options); } else if (requireParent === true && currentSpan?.context().isRemote) { span = currentSpan; } else { diff --git a/packages/opentelemetry-plugin-http/test/functionals/http-disable.test.ts b/packages/opentelemetry-plugin-http/test/functionals/http-disable.test.ts index 9ed917b509..55b057dbc6 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/http-disable.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/http-disable.test.ts @@ -51,7 +51,7 @@ describe('HttpPlugin', () => { }); beforeEach(() => { - NOOP_TRACER.startSpan = sinon.spy(); + sinon.spy(NOOP_TRACER, 'startSpan'); }); afterEach(() => { diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts index 38754c4956..d35162fcc2 100644 --- a/packages/opentelemetry-tracing/src/Tracer.ts +++ b/packages/opentelemetry-tracing/src/Tracer.ts @@ -18,7 +18,6 @@ import * as api from '@opentelemetry/api'; import { ConsoleLogger, InstrumentationLibrary, - NoRecordingSpan, IdGenerator, RandomIdGenerator, sanitizeAttributes, @@ -68,7 +67,7 @@ export class Tracer implements api.Tracer { ): api.Span { if (api.isInstrumentationSuppressed(context)) { this.logger.debug('Instrumentation suppressed, returning Noop Span'); - return new api.NoopSpan(); + return api.NOOP_TRACER.startSpan(name, options, context); } const parentContext = getParent(options, context); @@ -104,7 +103,11 @@ export class Tracer implements api.Tracer { const spanContext = { traceId, spanId, traceFlags, traceState }; if (samplingResult.decision === api.SamplingDecision.NOT_RECORD) { this.logger.debug('Recording is off, starting no recording span'); - return new NoRecordingSpan(spanContext); + return api.NOOP_TRACER.startSpan( + name, + options, + api.setSpanContext(context, spanContext) + ); } const span = new Span( diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts index 91e3332008..b688f11ced 100644 --- a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts @@ -27,7 +27,6 @@ import { import { AlwaysOnSampler, AlwaysOffSampler, - NoRecordingSpan, TraceState, } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; @@ -258,13 +257,13 @@ describe('BasicTracerProvider', () => { assert.deepStrictEqual(context.traceState, undefined); }); - it('should return a no recording span when never sampling', () => { + it('should return a non recording span when never sampling', () => { const tracer = new BasicTracerProvider({ sampler: new AlwaysOffSampler(), logger: new NoopLogger(), }).getTracer('default'); const span = tracer.startSpan('my-span'); - assert.ok(span instanceof NoRecordingSpan); + assert.ok(!span.isRecording()); const context = span.context(); assert.ok(context.traceId.match(/[a-f0-9]{32}/)); assert.ok(context.spanId.match(/[a-f0-9]{16}/)); diff --git a/packages/opentelemetry-tracing/test/Tracer.test.ts b/packages/opentelemetry-tracing/test/Tracer.test.ts index 0f97ec2781..9f90c387d8 100644 --- a/packages/opentelemetry-tracing/test/Tracer.test.ts +++ b/packages/opentelemetry-tracing/test/Tracer.test.ts @@ -16,7 +16,6 @@ import * as assert from 'assert'; import { - NoopSpan, Sampler, SamplingDecision, TraceFlags, @@ -78,7 +77,7 @@ describe('Tracer', () => { tracerProvider ); const span = tracer.startSpan('span1'); - assert.ok(span instanceof NoopSpan); + assert.ok(!span.isRecording()); span.end(); }); @@ -89,7 +88,7 @@ describe('Tracer', () => { tracerProvider ); const span = tracer.startSpan('span2'); - assert.ok(!(span instanceof NoopSpan)); + assert.ok(span.isRecording()); span.end(); }); @@ -129,7 +128,7 @@ describe('Tracer', () => { const span = tracer.startSpan('span3', undefined, context); - assert.ok(span instanceof NoopSpan); + assert.ok(!span.isRecording()); span.end(); done(); From 877b780b085783740ca2afcf791b864427bd9384 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Mon, 8 Feb 2021 18:19:03 +0100 Subject: [PATCH 3/3] chore: fix merge errors --- packages/opentelemetry-node/test/NodeTracerProvider.test.ts | 1 - packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts index bed62eec42..2ebf4b7574 100644 --- a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts +++ b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts @@ -21,7 +21,6 @@ import { setSpan, setSpanContext, getSpan, - NoopSpan, } from '@opentelemetry/api'; import { AlwaysOnSampler, AlwaysOffSampler } from '@opentelemetry/core'; import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts index 865286a421..b688f11ced 100644 --- a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts @@ -20,7 +20,6 @@ import { TraceFlags, ROOT_CONTEXT, NoopLogger, - NoopSpan, setSpan, setSpanContext, getSpan,