chore: remove references to NOOP singletons (#2229)

open-telemetry · May 27, 2021 · 7fa4ff7 · 7fa4ff7
1 parent d82ad8a
commit 7fa4ff7
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 39 deletions.
diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -14,16 +14,17 @@
  * limitations under the License.
  */
 import {
-  SpanStatusCode,
   context,
+  diag,
+  INVALID_SPAN_CONTEXT,
   propagation,
+  ROOT_CONTEXT,
   Span,
   SpanKind,
   SpanOptions,
   SpanStatus,
-  ROOT_CONTEXT,
-  NOOP_TRACER,
-  diag, trace,
+  SpanStatusCode,
+  trace,
 } from '@opentelemetry/api';
 import { suppressTracing } from '@opentelemetry/core';
 import type * as http from 'http';
@@ -599,9 +600,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
     const currentSpan = trace.getSpan(ctx);
 
     if (requireParent === true && currentSpan === undefined) {
-      // TODO: Refactor this when a solution is found in
-      // https://github.com/open-telemetry/opentelemetry-specification/issues/530
-      span = NOOP_TRACER.startSpan(name, options, ctx);
+      span = trace.wrapSpanContext(INVALID_SPAN_CONTEXT);
     } else if (requireParent === true && currentSpan?.spanContext().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
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { NoopTracerProvider, NOOP_TRACER } from '@opentelemetry/api';
 import * as assert from 'assert';
 import { HttpInstrumentation } from '../../src/http';
 import { AddressInfo } from 'net';
@@ -27,14 +26,24 @@ instrumentation.enable();
 instrumentation.disable();
 
 import * as http from 'http';
+import { trace, TracerProvider, INVALID_SPAN_CONTEXT } from '@opentelemetry/api';
+
 
 describe('HttpInstrumentation', () => {
   let server: http.Server;
   let serverPort = 0;
 
   describe('disable()', () => {
-    const provider = new NoopTracerProvider();
+    let provider: TracerProvider;
+    let startSpanStub: sinon.SinonStub;
+
     before(() => {
+      provider = {
+        getTracer: () => {
+          startSpanStub = sinon.stub().returns(trace.wrapSpanContext(INVALID_SPAN_CONTEXT));
+          return { startSpan: startSpanStub } as any;
+        }
+      };
       nock.cleanAll();
       nock.enableNetConnect();
       instrumentation.enable();
@@ -51,10 +60,6 @@ describe('HttpInstrumentation', () => {
       });
     });
 
-    beforeEach(() => {
-      sinon.spy(NOOP_TRACER, 'startSpan');
-    });
-
     afterEach(() => {
       sinon.restore();
     });
@@ -71,11 +76,8 @@ describe('HttpInstrumentation', () => {
 
         const options = { host: 'localhost', path: testPath, port: serverPort };
 
-        await httpRequest.get(options).then(result => {
-          assert.strictEqual(
-            (NOOP_TRACER.startSpan as sinon.SinonSpy).called,
-            false
-          );
+        await httpRequest.get(options).then(() => {
+          sinon.assert.notCalled(startSpanStub);
         });
       });
     });

diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/https-disable.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/https-disable.test.ts
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import { NoopTracerProvider, NOOP_TRACER } from '@opentelemetry/api';
 import * as assert from 'assert';
 import * as fs from 'fs';
 import type { AddressInfo } from 'net';
@@ -29,14 +28,23 @@ instrumentation.disable();
 
 import * as https from 'https';
 import { httpsRequest } from '../utils/httpsRequest';
+import { INVALID_SPAN_CONTEXT, trace, TracerProvider } from '@opentelemetry/api';
 
 describe('HttpsInstrumentation', () => {
   let server: https.Server;
   let serverPort = 0;
 
   describe('disable()', () => {
-    const provider = new NoopTracerProvider();
+    let provider: TracerProvider;
+    let startSpanStub: sinon.SinonStub;
+
     before(() => {
+      provider = {
+        getTracer: () => {
+          startSpanStub = sinon.stub().returns(trace.wrapSpanContext(INVALID_SPAN_CONTEXT));
+          return { startSpan: startSpanStub } as any;
+        }
+      };
       nock.cleanAll();
       nock.enableNetConnect();
 
@@ -60,10 +68,6 @@ describe('HttpsInstrumentation', () => {
       });
     });
 
-    beforeEach(() => {
-      sinon.spy(NOOP_TRACER, 'startSpan');
-    });
-
     afterEach(() => {
       sinon.restore();
     });
@@ -78,12 +82,8 @@ describe('HttpsInstrumentation', () => {
 
         const options = { host: 'localhost', path: testPath, port: serverPort };
 
-        await httpsRequest.get(options).then(result => {
-          assert.strictEqual(
-            (NOOP_TRACER.startSpan as sinon.SinonSpy).called,
-            false
-          );
-
+        await httpsRequest.get(options).then(() => {
+          sinon.assert.notCalled(startSpanStub);
           assert.strictEqual(isWrapped(https.Server.prototype.emit), false);
         });
       });

diff --git a/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts b/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts
@@ -14,12 +14,17 @@
  * limitations under the License.
  */
 
-import { NOOP_TRACER_PROVIDER } from '@opentelemetry/api';
+import { Tracer, TracerProvider } from '@opentelemetry/api';
 import { NOOP_METER_PROVIDER } from '@opentelemetry/api-metrics';
 import * as assert from 'assert';
 import * as sinon from 'sinon';
 import { InstrumentationBase, registerInstrumentations } from '../../src';
 
+class DummyTracerProvider implements TracerProvider {
+  getTracer(name: string, version?: string): Tracer {
+    throw new Error("not implemented");
+  }
+}
 class FooInstrumentation extends InstrumentationBase {
   init() {
     return [];
@@ -45,7 +50,7 @@ describe('autoLoader', () => {
       let enableSpy: sinon.SinonSpy;
       let setTracerProviderSpy: sinon.SinonSpy;
       let setsetMeterProvider: sinon.SinonSpy;
-      const tracerProvider = NOOP_TRACER_PROVIDER;
+      const tracerProvider = new DummyTracerProvider();
       const meterProvider = NOOP_METER_PROVIDER;
       beforeEach(() => {
         instrumentation = new FooInstrumentation('foo', '1', {});

diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts
@@ -105,7 +105,7 @@ export class Tracer implements api.Tracer {
   ): api.Span {
     if (isTracingSuppressed(context)) {
       api.diag.debug('Instrumentation suppressed, returning Noop Span');
-      return api.NOOP_TRACER.startSpan(name, options, context);
+      return api.trace.wrapSpanContext(api.INVALID_SPAN_CONTEXT);
     }
 
     const parentContext = getParent(options, context);
@@ -144,12 +144,8 @@ export class Tracer implements api.Tracer {
         : api.TraceFlags.NONE;
     const spanContext = { traceId, spanId, traceFlags, traceState };
     if (samplingResult.decision === api.SamplingDecision.NOT_RECORD) {
-      api.diag.debug('Recording is off, starting no recording span');
-      return api.NOOP_TRACER.startSpan(
-        name,
-        options,
-        api.trace.setSpanContext(context, spanContext)
-      );
+      api.diag.debug('Recording is off, propagating context in a non-recording span');
+      return api.trace.wrapSpanContext(spanContext);
     }
 
     const span = new Span(