chore: change withSpanAsync return to void and add tsdoc

open-telemetry · May 17, 2020 · 07e7843 · 07e7843
1 parent 128cdad
commit 07e7843
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 34 deletions.
diff --git a/packages/opentelemetry-api/test/api/global.test.ts b/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);
 

diff --git a/packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts b/packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts
@@ -67,10 +67,7 @@ export class AsyncHooksContextManager implements ContextManager {
     return ref === undefined ? Context.ROOT_CONTEXT : ref.get();
   }
 
-  with<T extends (...args: unknown[]) => ReturnType<T>>(
-    context: Context,
-    fn: T
-  ): ReturnType<T> {
+  with<T extends () => ReturnType<T>>(context: Context, fn: T): ReturnType<T> {
     const uid = asyncHooks.executionAsyncId();
     let ref = this._contextRefs.get(uid);
     let oldContext: Context | undefined = undefined;
@@ -94,10 +91,10 @@ export class AsyncHooksContextManager implements ContextManager {
     }
   }
 
-  async withAsync<T extends Promise<any>, U extends (...args: unknown[]) => T>(
+  async withAsync<T extends any, U extends () => Promise<T>>(
     context: Context,
     fn: U
-  ): Promise<T> {
+  ): Promise<void> {
     const uid = asyncHooks.executionAsyncId();
     let ref = this._contextRefs.get(uid);
     let oldContext: Context | undefined = undefined;
@@ -109,7 +106,7 @@ export class AsyncHooksContextManager implements ContextManager {
       ref.set(context);
     }
     try {
-      return await fn();
+      await fn();
     } catch (err) {
       throw err;
     } finally {

diff --git a/packages/opentelemetry-node/src/NodeTracer.ts b/packages/opentelemetry-node/src/NodeTracer.ts
@@ -16,39 +16,34 @@
 
 import * as api from '@opentelemetry/api';
 import { setActiveSpan } from '@opentelemetry/core';
-import {
-  BasicTracerProvider,
-  Tracer,
-  TracerConfig,
-} from '@opentelemetry/tracing';
+import { Tracer } from '@opentelemetry/tracing';
 import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
 
 /**
  * This class represents a nodejs-specific tracer.
  */
 export class NodeTracer extends Tracer {
   /**
-   * Constructs a new NodeTracer instance.
+   * Execute the provided function with the given span in the current context.
+   *
+   * **NOTE**: This function is experimental, refer to to
+   *  https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-node#withspanasync
    */
-  constructor(config: TracerConfig, _tracerProvider: BasicTracerProvider) {
-    super(config, _tracerProvider);
-  }
-
-  async withSpanAsync<
-    T extends Promise<any>,
-    U extends (...args: unknown[]) => T
-  >(span: api.Span, fn: U): Promise<T> {
+  async withSpanAsync<T extends any, U extends () => Promise<T>>(
+    span: api.Span,
+    fn: U
+  ): Promise<void> {
     const contextManager = api.context.getContextManager();
     if (contextManager instanceof AsyncHooksContextManager) {
-      return contextManager.withAsync(
+      await contextManager.withAsync(
         setActiveSpan(api.context.active(), span),
         fn
       );
     } else {
       this.logger.warn(
         `Using withAsync without AsyncHookContextManager doesn't work, please refer to https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-node#withspanasync`
       );
-      return await fn();
+      await fn();
     }
   }
 }