Skip to content

Commit 7fa4ff7

Browse files
authored
chore: remove references to NOOP singletons (#2229)
1 parent d82ad8a commit 7fa4ff7

File tree

5 files changed

+41
-39
lines changed

5 files changed

+41
-39
lines changed

packages/opentelemetry-instrumentation-http/src/http.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,17 @@
1414
* limitations under the License.
1515
*/
1616
import {
17-
SpanStatusCode,
1817
context,
18+
diag,
19+
INVALID_SPAN_CONTEXT,
1920
propagation,
21+
ROOT_CONTEXT,
2022
Span,
2123
SpanKind,
2224
SpanOptions,
2325
SpanStatus,
24-
ROOT_CONTEXT,
25-
NOOP_TRACER,
26-
diag, trace,
26+
SpanStatusCode,
27+
trace,
2728
} from '@opentelemetry/api';
2829
import { suppressTracing } from '@opentelemetry/core';
2930
import type * as http from 'http';
@@ -599,9 +600,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
599600
const currentSpan = trace.getSpan(ctx);
600601

601602
if (requireParent === true && currentSpan === undefined) {
602-
// TODO: Refactor this when a solution is found in
603-
// https://github.com/open-telemetry/opentelemetry-specification/issues/530
604-
span = NOOP_TRACER.startSpan(name, options, ctx);
603+
span = trace.wrapSpanContext(INVALID_SPAN_CONTEXT);
605604
} else if (requireParent === true && currentSpan?.spanContext().isRemote) {
606605
span = currentSpan;
607606
} else {

packages/opentelemetry-instrumentation-http/test/functionals/http-disable.test.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
import { NoopTracerProvider, NOOP_TRACER } from '@opentelemetry/api';
1716
import * as assert from 'assert';
1817
import { HttpInstrumentation } from '../../src/http';
1918
import { AddressInfo } from 'net';
@@ -27,14 +26,24 @@ instrumentation.enable();
2726
instrumentation.disable();
2827

2928
import * as http from 'http';
29+
import { trace, TracerProvider, INVALID_SPAN_CONTEXT } from '@opentelemetry/api';
30+
3031

3132
describe('HttpInstrumentation', () => {
3233
let server: http.Server;
3334
let serverPort = 0;
3435

3536
describe('disable()', () => {
36-
const provider = new NoopTracerProvider();
37+
let provider: TracerProvider;
38+
let startSpanStub: sinon.SinonStub;
39+
3740
before(() => {
41+
provider = {
42+
getTracer: () => {
43+
startSpanStub = sinon.stub().returns(trace.wrapSpanContext(INVALID_SPAN_CONTEXT));
44+
return { startSpan: startSpanStub } as any;
45+
}
46+
};
3847
nock.cleanAll();
3948
nock.enableNetConnect();
4049
instrumentation.enable();
@@ -51,10 +60,6 @@ describe('HttpInstrumentation', () => {
5160
});
5261
});
5362

54-
beforeEach(() => {
55-
sinon.spy(NOOP_TRACER, 'startSpan');
56-
});
57-
5863
afterEach(() => {
5964
sinon.restore();
6065
});
@@ -71,11 +76,8 @@ describe('HttpInstrumentation', () => {
7176

7277
const options = { host: 'localhost', path: testPath, port: serverPort };
7378

74-
await httpRequest.get(options).then(result => {
75-
assert.strictEqual(
76-
(NOOP_TRACER.startSpan as sinon.SinonSpy).called,
77-
false
78-
);
79+
await httpRequest.get(options).then(() => {
80+
sinon.assert.notCalled(startSpanStub);
7981
});
8082
});
8183
});

packages/opentelemetry-instrumentation-http/test/functionals/https-disable.test.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { NoopTracerProvider, NOOP_TRACER } from '@opentelemetry/api';
1817
import * as assert from 'assert';
1918
import * as fs from 'fs';
2019
import type { AddressInfo } from 'net';
@@ -29,14 +28,23 @@ instrumentation.disable();
2928

3029
import * as https from 'https';
3130
import { httpsRequest } from '../utils/httpsRequest';
31+
import { INVALID_SPAN_CONTEXT, trace, TracerProvider } from '@opentelemetry/api';
3232

3333
describe('HttpsInstrumentation', () => {
3434
let server: https.Server;
3535
let serverPort = 0;
3636

3737
describe('disable()', () => {
38-
const provider = new NoopTracerProvider();
38+
let provider: TracerProvider;
39+
let startSpanStub: sinon.SinonStub;
40+
3941
before(() => {
42+
provider = {
43+
getTracer: () => {
44+
startSpanStub = sinon.stub().returns(trace.wrapSpanContext(INVALID_SPAN_CONTEXT));
45+
return { startSpan: startSpanStub } as any;
46+
}
47+
};
4048
nock.cleanAll();
4149
nock.enableNetConnect();
4250

@@ -60,10 +68,6 @@ describe('HttpsInstrumentation', () => {
6068
});
6169
});
6270

63-
beforeEach(() => {
64-
sinon.spy(NOOP_TRACER, 'startSpan');
65-
});
66-
6771
afterEach(() => {
6872
sinon.restore();
6973
});
@@ -78,12 +82,8 @@ describe('HttpsInstrumentation', () => {
7882

7983
const options = { host: 'localhost', path: testPath, port: serverPort };
8084

81-
await httpsRequest.get(options).then(result => {
82-
assert.strictEqual(
83-
(NOOP_TRACER.startSpan as sinon.SinonSpy).called,
84-
false
85-
);
86-
85+
await httpsRequest.get(options).then(() => {
86+
sinon.assert.notCalled(startSpanStub);
8787
assert.strictEqual(isWrapped(https.Server.prototype.emit), false);
8888
});
8989
});

packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,17 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { NOOP_TRACER_PROVIDER } from '@opentelemetry/api';
17+
import { Tracer, TracerProvider } from '@opentelemetry/api';
1818
import { NOOP_METER_PROVIDER } from '@opentelemetry/api-metrics';
1919
import * as assert from 'assert';
2020
import * as sinon from 'sinon';
2121
import { InstrumentationBase, registerInstrumentations } from '../../src';
2222

23+
class DummyTracerProvider implements TracerProvider {
24+
getTracer(name: string, version?: string): Tracer {
25+
throw new Error("not implemented");
26+
}
27+
}
2328
class FooInstrumentation extends InstrumentationBase {
2429
init() {
2530
return [];
@@ -45,7 +50,7 @@ describe('autoLoader', () => {
4550
let enableSpy: sinon.SinonSpy;
4651
let setTracerProviderSpy: sinon.SinonSpy;
4752
let setsetMeterProvider: sinon.SinonSpy;
48-
const tracerProvider = NOOP_TRACER_PROVIDER;
53+
const tracerProvider = new DummyTracerProvider();
4954
const meterProvider = NOOP_METER_PROVIDER;
5055
beforeEach(() => {
5156
instrumentation = new FooInstrumentation('foo', '1', {});

packages/opentelemetry-tracing/src/Tracer.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export class Tracer implements api.Tracer {
105105
): api.Span {
106106
if (isTracingSuppressed(context)) {
107107
api.diag.debug('Instrumentation suppressed, returning Noop Span');
108-
return api.NOOP_TRACER.startSpan(name, options, context);
108+
return api.trace.wrapSpanContext(api.INVALID_SPAN_CONTEXT);
109109
}
110110

111111
const parentContext = getParent(options, context);
@@ -144,12 +144,8 @@ export class Tracer implements api.Tracer {
144144
: api.TraceFlags.NONE;
145145
const spanContext = { traceId, spanId, traceFlags, traceState };
146146
if (samplingResult.decision === api.SamplingDecision.NOT_RECORD) {
147-
api.diag.debug('Recording is off, starting no recording span');
148-
return api.NOOP_TRACER.startSpan(
149-
name,
150-
options,
151-
api.trace.setSpanContext(context, spanContext)
152-
);
147+
api.diag.debug('Recording is off, propagating context in a non-recording span');
148+
return api.trace.wrapSpanContext(spanContext);
153149
}
154150

155151
const span = new Span(

0 commit comments

Comments
 (0)