chore: fixing async resolvers (#300)

open-telemetry · Jan 8, 2021 · e3de6ba · e3de6ba
1 parent 31d7bee
commit e3de6ba
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 25 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts b/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import { safeExecuteInTheMiddle } from '@opentelemetry/instrumentation';
 import type * as graphqlTypes from 'graphql';
 import * as api from '@opentelemetry/api';
 import type { Maybe } from 'graphql/jsutils/Maybe';
@@ -358,8 +357,11 @@ export function wrapFieldResolver<TSource = any, TContext = any, TArgs = any>(
     }
 
     return tracer.withSpan(field.span, () => {
-      return safeExecuteInTheMiddle<
-        Maybe<graphqlTypes.GraphQLFieldResolver<TSource, TContext, TArgs>>
+      return safeExecuteInTheMiddleAsync<
+        | Maybe<graphqlTypes.GraphQLFieldResolver<TSource, TContext, TArgs>>
+        | Promise<
+            Maybe<graphqlTypes.GraphQLFieldResolver<TSource, TContext, TArgs>>
+          >
       >(
         () => {
           return fieldResolver.call(this, source, args, contextValue, info);
@@ -377,3 +379,32 @@ export function wrapFieldResolver<TSource = any, TContext = any, TArgs = any>(
 
   return wrappedFieldResolver;
 }
+
+/**
+ * Async version of safeExecuteInTheMiddle from instrumentation package
+ * can be removed once this will be added to instrumentation package
+ * @param execute
+ * @param onFinish
+ * @param preventThrowingError
+ */
+async function safeExecuteInTheMiddleAsync<T>(
+  execute: () => T,
+  onFinish: (e: Error | undefined, result: T | undefined) => void,
+  preventThrowingError?: boolean
+): Promise<T> {
+  let error: Error | undefined;
+  let result: T | undefined;
+  try {
+    result = await execute();
+  } catch (e) {
+    error = e;
+  } finally {
+    onFinish(error, result);
+    if (error && !preventThrowingError) {
+      // eslint-disable-next-line no-unsafe-finally
+      throw error;
+    }
+    // eslint-disable-next-line no-unsafe-finally
+    return result as T;
+  }
+}
diff --git a/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts b/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts
@@ -141,7 +141,7 @@ describe('graphql', () => {
       });
 
       it('should instrument execute', () => {
-        const executeSpan = spans[3];
+        const executeSpan = spans[2];
         const validateSpan = spans[1];
 
         assert.deepStrictEqual(
@@ -165,8 +165,8 @@ describe('graphql', () => {
       });
 
       it('should instrument resolvers', () => {
-        const executeSpan = spans[3];
-        const resolveParentSpan = spans[2];
+        const executeSpan = spans[2];
+        const resolveParentSpan = spans[3];
         const span1 = spans[4];
         const span2 = spans[5];
         const span3 = spans[6];
@@ -251,7 +251,7 @@ describe('graphql', () => {
       });
 
       it('should instrument execute', () => {
-        const executeSpan = spans[3];
+        const executeSpan = spans[2];
         const validateSpan = spans[1];
 
         assert.deepStrictEqual(
@@ -275,8 +275,8 @@ describe('graphql', () => {
       });
 
       it('should instrument resolvers', () => {
-        const executeSpan = spans[3];
-        const resolveParentSpan = spans[2];
+        const executeSpan = spans[2];
+        const resolveParentSpan = spans[3];
         const span1 = spans[4];
 
         assertResolveSpan(
@@ -345,7 +345,7 @@ describe('graphql', () => {
       });
 
       it('should instrument execute', () => {
-        const executeSpan = spans[3];
+        const executeSpan = spans[2];
         const validateSpan = spans[1];
 
         assert.deepStrictEqual(
@@ -373,8 +373,8 @@ describe('graphql', () => {
       });
 
       it('should instrument resolvers', () => {
-        const executeSpan = spans[3];
-        const resolveParentSpan = spans[2];
+        const executeSpan = spans[2];
+        const resolveParentSpan = spans[3];
         const span1 = spans[4];
 
         assertResolveSpan(
@@ -517,7 +517,7 @@ describe('graphql', () => {
       });
 
       it('should instrument execute', () => {
-        const executeSpan = spans[3];
+        const executeSpan = spans[2];
         const validateSpan = spans[1];
 
         assert.deepStrictEqual(
@@ -612,7 +612,7 @@ describe('graphql', () => {
       });
 
       it('should instrument execute', () => {
-        const executeSpan = spans[3];
+        const executeSpan = spans[2];
         const validateSpan = spans[1];
 
         assert.deepStrictEqual(
@@ -636,8 +636,8 @@ describe('graphql', () => {
       });
 
       it('should instrument resolvers', () => {
-        const executeSpan = spans[3];
-        const resolveParentSpan = spans[2];
+        const executeSpan = spans[2];
+        const resolveParentSpan = spans[3];
         const span1 = spans[4];
 
         assertResolveSpan(
@@ -709,7 +709,7 @@ describe('graphql', () => {
       });
 
       it('should instrument execute', () => {
-        const executeSpan = spans[3];
+        const executeSpan = spans[2];
         const validateSpan = spans[1];
 
         assert.deepStrictEqual(
@@ -736,8 +736,8 @@ describe('graphql', () => {
       });
 
       it('should instrument resolvers', () => {
-        const executeSpan = spans[3];
-        const resolveParentSpan = spans[2];
+        const executeSpan = spans[2];
+        const resolveParentSpan = spans[3];
         const span1 = spans[4];
 
         assertResolveSpan(
@@ -806,7 +806,7 @@ describe('graphql', () => {
       });
 
       it('should instrument execute', () => {
-        const executeSpan = spans[3];
+        const executeSpan = spans[2];
         const validateSpan = spans[1];
 
         assert.deepStrictEqual(
@@ -834,8 +834,8 @@ describe('graphql', () => {
       });
 
       it('should instrument resolvers', () => {
-        const executeSpan = spans[3];
-        const resolveParentSpan = spans[2];
+        const executeSpan = spans[2];
+        const resolveParentSpan = spans[3];
         const span1 = spans[4];
 
         assertResolveSpan(
@@ -909,7 +909,7 @@ describe('graphql', () => {
     });
 
     it('should instrument execute', () => {
-      const executeSpan = spans[3];
+      const executeSpan = spans[2];
       const validateSpan = spans[1];
 
       assert.deepStrictEqual(
@@ -936,8 +936,8 @@ describe('graphql', () => {
     });
 
     it('should instrument resolvers', () => {
-      const executeSpan = spans[3];
-      const resolveParentSpan = spans[2];
+      const executeSpan = spans[2];
+      const resolveParentSpan = spans[3];
       const span1 = spans[4];
 
       assertResolveSpan(