chore: address PR comments

Author: vmarchaud

packages/opentelemetry-api/src/api/context.ts

@@ -54,7 +54,7 @@ export class ContextAPI {
   ): ContextManager {
     if (_global[GLOBAL_CONTEXT_MANAGER_API_KEY]) {
       // global context manager has already been set
-      return this._getContextManager();
+      return this.getContextManager();
     }
 
     _global[GLOBAL_CONTEXT_MANAGER_API_KEY] = makeGetter(
@@ -70,7 +70,7 @@ export class ContextAPI {
    * Get the currently active context
    */
   public active(): Context {
-    return this._getContextManager().active();
+    return this.getContextManager().active();
   }
 
   /**
@@ -83,7 +83,7 @@ export class ContextAPI {
     context: Context,
     fn: T
   ): ReturnType<T> {
-    return this._getContextManager().with(context, fn);
+    return this.getContextManager().with(context, fn);
   }
 
   /**
@@ -93,10 +93,10 @@ export class ContextAPI {
    * @param context context to bind to the event emitter or function. Defaults to the currently active context
    */
   public bind<T>(target: T, context: Context = this.active()): T {
-    return this._getContextManager().bind(target, context);
+    return this.getContextManager().bind(target, context);
   }
 
-  private _getContextManager(): ContextManager {
+  public getContextManager(): ContextManager {
     return (
       _global[GLOBAL_CONTEXT_MANAGER_API_KEY]?.(
         API_BACKWARDS_COMPATIBILITY_VERSION
@@ -106,7 +106,7 @@ export class ContextAPI {
 
   /** Disable and remove the global context manager */
   public disable() {
-    this._getContextManager().disable();
+    this.getContextManager().disable();
     delete _global[GLOBAL_CONTEXT_MANAGER_API_KEY];
   }
 }

packages/opentelemetry-api/test/api/global.test.ts

@@ -34,8 +34,8 @@ describe('Global Utils', () => {
   assert.notEqual(api1, api2);
   // that return separate noop instances to start
   assert.notStrictEqual(
-    api1.context['_getContextManager'](),
-    api2.context['_getContextManager']()
+    api1.context['getContextManager'](),
+    api2.context['getContextManager']()
   );
 
   beforeEach(() => {
@@ -46,33 +46,33 @@ describe('Global Utils', () => {
   });
 
   it('should change the global context manager', () => {
-    const original = api1.context['_getContextManager']();
+    const original = api1.context['getContextManager']();
     const newContextManager = new NoopContextManager();
     api1.context.setGlobalContextManager(newContextManager);
-    assert.notStrictEqual(api1.context['_getContextManager'](), original);
-    assert.strictEqual(api1.context['_getContextManager'](), newContextManager);
+    assert.notStrictEqual(api1.context['getContextManager'](), original);
+    assert.strictEqual(api1.context['getContextManager'](), newContextManager);
   });
 
   it('should load an instance from one which was set in the other', () => {
     api1.context.setGlobalContextManager(new NoopContextManager());
     assert.strictEqual(
-      api1.context['_getContextManager'](),
-      api2.context['_getContextManager']()
+      api1.context['getContextManager'](),
+      api2.context['getContextManager']()
     );
   });
 
   it('should disable both if one is disabled', () => {
-    const original = api1.context['_getContextManager']();
+    const original = api1.context['getContextManager']();
 
     api1.context.setGlobalContextManager(new NoopContextManager());
 
-    assert.notStrictEqual(original, api1.context['_getContextManager']());
+    assert.notStrictEqual(original, api1.context['getContextManager']());
     api2.context.disable();
-    assert.strictEqual(original, api1.context['_getContextManager']());
+    assert.strictEqual(original, api1.context['getContextManager']());
   });
 
   it('should return the module NoOp implementation if the version is a mismatch', () => {
-    const original = api1.context['_getContextManager']();
+    const original = api1.context['getContextManager']();
     api1.context.setGlobalContextManager(new NoopContextManager());
     const afterSet = _global[GLOBAL_CONTEXT_MANAGER_API_KEY]!(-1);
 

packages/opentelemetry-node/README.md

@@ -92,6 +92,14 @@ provider.register()
 ## Examples
 See how to automatically instrument [http](https://github.com/open-telemetry/opentelemetry-js/tree/master/examples/http) and [gRPC](https://github.com/open-telemetry/opentelemetry-js/tree/master/examples/grpc) using node-sdk.
 
+## Experimental API methods
+
+#### withSpanAsync
+
+The `NodeTracer` expose a method called `withSpanAsync` that allows you to set a current span in the context of a `async` function. This previously was a problem with `withSpan` because its synchronous behavior, read about that in [this issue](https://github.com/open-telemetry/opentelemetry-js/issues/752).
+This method have two drawback:
+ - strict requirement of the `AsyncHooksScopeManager`, you **cannot** use it without.
+ - the underlying implementation inside `AsyncHooksScopeManager` is experimental, so is this method, please open an issue if you have a problem with it.
 
 ## Useful links
 - For more information on OpenTelemetry, visit: <https://opentelemetry.io/>

packages/opentelemetry-node/src/NodeTracer.ts

@@ -54,18 +54,17 @@ export class NodeTracer extends Tracer {
     T extends Promise<any>,
     U extends (...args: unknown[]) => T
   >(span: api.Span, fn: U): Promise<T> {
-    // @ts-ignore
-    const contextManager = api.context._getContextManager();
+    const contextManager = api.context.getContextManager();
     if (contextManager instanceof AsyncHooksContextManager) {
       return contextManager.withAsync(
         setActiveSpan(api.context.active(), span),
         fn
       );
     } else {
       this.logger.warn(
-        `Using withAsync without AsyncHookContextManager doesn't work, please refer to`
+        `Using withAsync without AsyncHookContextManager doesn't work, please refer to https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-node#withspanasync`
       );
-      return fn();
+      return await fn();
     }
   }
 }