chore: requires user to pass context to propagation APIs (open-teleme…

…try#1734) * chore: requires user to pass context to propagation APIs At least for extract() using the current active context is often a bad choice because usually a plugin wants to use an extracted span instead the current active. Change propagation APIs to require context as first argument instead of using active context on default. This ensures that plugin developers have to be explicit instead of relying on a potential wrong default. * (chore) fix lint * add test * Adapt http instrumentation * fix: update integration testserver
dyladan · Feb 18, 2021 · 0342ec6 · 0342ec6
1 parent 966a1c1
commit 0342ec6
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 9 deletions.
diff --git a/api/src/api/propagation.ts b/api/src/api/propagation.ts
@@ -23,16 +23,13 @@ import {
   TextMapPropagator,
   TextMapSetter,
 } from '../context/propagation/TextMapPropagator';
-import { ContextAPI } from './context';
 import {
   API_BACKWARDS_COMPATIBILITY_VERSION,
   GLOBAL_PROPAGATION_API_KEY,
   makeGetter,
   _global,
 } from './global-utils';
 
-const contextApi = ContextAPI.getInstance();
-
 /**
  * Singleton object which represents the entry point to the OpenTelemetry Propagation API
  */
@@ -72,29 +69,29 @@ export class PropagationAPI {
   /**
    * Inject context into a carrier to be propagated inter-process
    *
+   * @param context Context carrying tracing data to inject
    * @param carrier carrier to inject context into
    * @param setter Function used to set values on the carrier
-   * @param context Context carrying tracing data to inject. Defaults to the currently active context.
    */
   public inject<Carrier>(
+    context: Context,
     carrier: Carrier,
-    setter: TextMapSetter<Carrier> = defaultTextMapSetter,
-    context: Context = contextApi.active()
+    setter: TextMapSetter<Carrier> = defaultTextMapSetter
   ): void {
     return this._getGlobalPropagator().inject(context, carrier, setter);
   }
 
   /**
    * Extract context from a carrier
    *
+   * @param context Context which the newly created context will inherit from
    * @param carrier Carrier to extract context from
    * @param getter Function used to extract keys from a carrier
-   * @param context Context which the newly created context will inherit from. Defaults to the currently active context.
    */
   public extract<Carrier>(
+    context: Context,
     carrier: Carrier,
-    getter: TextMapGetter<Carrier> = defaultTextMapGetter,
-    context: Context = contextApi.active()
+    getter: TextMapGetter<Carrier> = defaultTextMapGetter
   ): Context {
     return this._getGlobalPropagator().extract(context, carrier, getter);
   }

diff --git a/api/test/api/api.test.ts b/api/test/api/api.test.ts
@@ -26,6 +26,13 @@ import api, {
   trace,
   propagation,
   metrics,
+  TextMapPropagator,
+  Context,
+  TextMapSetter,
+  TextMapGetter,
+  ROOT_CONTEXT,
+  defaultTextMapSetter,
+  defaultTextMapGetter,
 } from '../../src';
 
 describe('API', () => {
@@ -84,5 +91,81 @@ describe('API', () => {
         return new TestTracer();
       }
     }
+
+    describe('should use the global propagation', () => {
+      const testKey = Symbol('kTestKey');
+
+      interface Carrier {
+        context?: Context;
+        setter?: TextMapSetter;
+      }
+
+      class TestTextMapPropagation implements TextMapPropagator<Carrier> {
+        inject(
+          context: Context,
+          carrier: Carrier,
+          setter: TextMapSetter
+        ): void {
+          carrier.context = context;
+          carrier.setter = setter;
+        }
+
+        extract(
+          context: Context,
+          carrier: Carrier,
+          getter: TextMapGetter
+        ): Context {
+          return context.setValue(testKey, {
+            context,
+            carrier,
+            getter,
+          });
+        }
+
+        fields(): string[] {
+          return ['TestField'];
+        }
+      }
+
+      it('inject', () => {
+        api.propagation.setGlobalPropagator(new TestTextMapPropagation());
+
+        const context = ROOT_CONTEXT.setValue(testKey, 15);
+        const carrier: Carrier = {};
+        api.propagation.inject(context, carrier);
+        assert.strictEqual(carrier.context, context);
+        assert.strictEqual(carrier.setter, defaultTextMapSetter);
+
+        const setter: TextMapSetter = {
+          set: () => {},
+        };
+        api.propagation.inject(context, carrier, setter);
+        assert.strictEqual(carrier.context, context);
+        assert.strictEqual(carrier.setter, setter);
+      });
+
+      it('extract', () => {
+        api.propagation.setGlobalPropagator(new TestTextMapPropagation());
+
+        const carrier: Carrier = {};
+        let context = api.propagation.extract(ROOT_CONTEXT, carrier);
+        let data: any = context.getValue(testKey);
+        assert.ok(data != null);
+        assert.strictEqual(data.context, ROOT_CONTEXT);
+        assert.strictEqual(data.carrier, carrier);
+        assert.strictEqual(data.getter, defaultTextMapGetter);
+
+        const getter: TextMapGetter = {
+          keys: () => [],
+          get: () => undefined,
+        };
+        context = api.propagation.extract(ROOT_CONTEXT, carrier, getter);
+        data = context.getValue(testKey);
+        assert.ok(data != null);
+        assert.strictEqual(data.context, ROOT_CONTEXT);
+        assert.strictEqual(data.carrier, carrier);
+        assert.strictEqual(data.getter, getter);
+      });
+    });
   });
 });