refactor: update error stack collection logic

packages/collector/test/tracing/misc/stack_trace/test.js

@@ -89,6 +89,10 @@ const mochaSuiteFn = supportedVersion(process.versions.node) ? describe : descri
         await expressProxyControls.stop();
       });
 
+      beforeEach(async () => {
+        await agentControls.clearReceivedTraceData();
+      });
+
       beforeEach(async () => {
         await agentControls.waitUntilAppIsCompletelyInitialized(expressControls.getPid());
       });
@@ -107,7 +111,7 @@ const mochaSuiteFn = supportedVersion(process.versions.node) ? describe : descri
           .then(() =>
             testUtils.retry(() =>
               agentControls.getSpans().then(spans => {
-                expect(spans.length).to.equal(6);
+                expect(spans.length).to.equal(3);
                 testUtils.expectAtLeastOneMatching(spans, [
                   span => expect(span.n).to.equal('node.http.server'),
                   span => expect(span.stack).to.have.lengthOf(0)
@@ -124,17 +128,16 @@ const mochaSuiteFn = supportedVersion(process.versions.node) ? describe : descri
             responseStatus: 201
           })
           .then(() =>
-            testUtils.retry(() =>
-              agentControls.getSpans().then(spans => {
-                expect(spans.length).to.equal(9);
-                testUtils.expectAtLeastOneMatching(spans, [
-                  span => expect(span.n).to.equal('node.http.client'),
-                  span => expect(span.k).to.equal(constants.EXIT),
-                  span => expect(span.stack[2].m).to.equal('fetch'),
-                  span => expect(span.stack[2].c).to.contains('node-fetch')
-                ]);
-              })
-            )
+            agentControls.getSpans().then(spans => {
+              expect(spans.length).to.equal(3);
+              testUtils.expectAtLeastOneMatching(spans, [
+                span => expect(span.n).to.equal('node.http.client'),
+                span => expect(span.k).to.equal(constants.EXIT),
+                // since we remove internal code, only customer code is present in stack trace from now on
+                span => expect(span.stack[1].m).to.equal('fetch'),
+                span => expect(span.stack[1].c).to.contains('node-fetch')
+              ]);
+            })
           ));
     });
   });

packages/core/src/tracing/instrumentation/databases/mysql.js

@@ -168,7 +168,7 @@ function instrumentedAccessFunction(
       kind: constants.EXIT
     });
     span.b = { s: 1 };
-    span.stack = tracingUtil.getStackTrace(instrumentedAccessFunction);
+
     span.data.mysql = {
       stmt: tracingUtil.shortenDatabaseStatement(
         typeof statementOrOpts === 'string' ? statementOrOpts : statementOrOpts.sql
@@ -184,16 +184,12 @@ function instrumentedAccessFunction(
 
       resultPromise
         .then(result => {
-          span.d = Date.now() - span.ts;
-          span.transmit();
+          tracingUtil.completeSpan({ span, referenceFunction: instrumentedAccessFunction });
           return result;
         })
         .catch(error => {
-          span.ec = 1;
           span.data.mysql.error = tracingUtil.getErrorDetails(error);
-
-          span.d = Date.now() - span.ts;
-          span.transmit();
+          tracingUtil.completeSpan({ span, referenceFunction: instrumentedAccessFunction, error });
           return error;
         });
       return resultPromise;
@@ -204,12 +200,10 @@ function instrumentedAccessFunction(
 
     function onResult(error) {
       if (error) {
-        span.ec = 1;
         span.data.mysql.error = tracingUtil.getErrorDetails(error);
       }
 
-      span.d = Date.now() - span.ts;
-      span.transmit();
+      tracingUtil.completeSpan({ span, referenceFunction: instrumentedAccessFunction, error });
 
       if (originalCallback) {
         return originalCallback.apply(this, arguments);

packages/core/src/tracing/instrumentation/databases/redis.js

@@ -319,7 +319,6 @@ function instrumentCommand(original, command, address, cbStyle) {
         kind: constants.EXIT,
         spanData
       });
-      span.stack = tracingUtil.getStackTrace(instrumentCommand);
 
       let userProvidedCallback;
 
@@ -361,14 +360,11 @@ function instrumentCommand(original, command, address, cbStyle) {
       }
 
       function onResult(error) {
-        span.d = Date.now() - span.ts;
-
         if (error) {
-          span.ec = 1;
           span.data.redis.error = tracingUtil.getErrorDetails(error);
         }
 
-        span.transmit();
+        tracingUtil.completeSpan({ span, referenceFunction: instrumentCommand, error });
 
         if (typeof userProvidedCallback === 'function') {
           return userProvidedCallback.apply(this, arguments);

packages/core/src/tracing/instrumentation/protocols/graphql.js

@@ -142,7 +142,7 @@ function traceQueryOrMutation(
         kind: constants.ENTRY
       });
     }
-    span.stack = tracingUtil.getStackTrace(stackTraceRef);
+
     span.data.graphql = {
       operationType: operationDefinition.operation,
       operationName: operationDefinition.name ? operationDefinition.name.value : operationName,
@@ -151,7 +151,7 @@ function traceQueryOrMutation(
     };
     addFieldsAndArguments(span, operationDefinition);
 
-    return runOriginalAndFinish(originalFunction, originalThis, originalArgs, span);
+    return runOriginalAndFinish(originalFunction, originalThis, originalArgs, span, stackTraceRef);
   });
 }
 
@@ -187,15 +187,14 @@ function traceSubscriptionUpdate(
         parentSpanId: parentSpan.s
       });
       span.ts = Date.now();
-      span.stack = tracingUtil.getStackTrace(stackTraceRef);
       span.data.graphql = {
         operationType: subscriptionUpdate,
         operationName: operationDefinition.name ? operationDefinition.name.value : operationName,
         fields: {},
         args: {}
       };
       addFieldsAndArguments(span, operationDefinition);
-      return runOriginalAndFinish(originalFunction, originalThis, originalArgs, span);
+      return runOriginalAndFinish(originalFunction, originalThis, originalArgs, span, stackTraceRef);
     });
   } else {
     return originalFunction.apply(originalThis, originalArgs);
@@ -250,13 +249,13 @@ function traverseSelections(definition, selectionPostProcessor) {
   return selectionPostProcessor(candidates);
 }
 
-function runOriginalAndFinish(originalFunction, originalThis, originalArgs, span) {
+function runOriginalAndFinish(originalFunction, originalThis, originalArgs, span, stackTraceRef) {
   let result;
   try {
     result = originalFunction.apply(originalThis, originalArgs);
   } catch (e) {
     // A synchronous exception happened when resolving the GraphQL query, finish immediately.
-    finishWithException(span, e);
+    finishWithException(span, e, stackTraceRef);
     return result;
   }
 
@@ -266,43 +265,55 @@ function runOriginalAndFinish(originalFunction, originalThis, originalArgs, span
   if (result && typeof result.then === 'function') {
     return result.then(
       promiseResult => {
-        finishSpan(span, promiseResult);
+        finishSpan(span, promiseResult, stackTraceRef);
         return promiseResult;
       },
       err => {
-        finishWithException(span, err);
+        finishWithException(span, err, stackTraceRef);
         throw err;
       }
     );
   } else {
     // The GraphQL operation returned a value instead of a promise - that means, the resolver finished synchronously. We
     // can finish the span immediately.
-    finishSpan(span, result);
+    finishSpan(span, result, stackTraceRef);
     return result;
   }
 }
 
-function finishSpan(span, result) {
-  span.ec = result.errors && result.errors.length >= 1 ? 1 : 0;
-  span.d = Date.now() - span.ts;
+function finishSpan(span, result, stackTraceRef) {
+  const hasErrors = result.errors && result.errors.length >= 1;
+
   if (Array.isArray(result.errors)) {
     span.data.graphql.errors = result.errors
       .map(singleError => (typeof singleError.message === 'string' ? singleError.message : null))
       .filter(msg => !!msg)
       .join(', ');
   }
-  if (!span.postponeTransmit && !span.postponeTransmitApolloGateway) {
-    span.transmit();
-  }
+
+  // call completeSpan with conditional transmission based on postpone flags
+  const shouldSkipTransmit = span.postponeTransmit || span.postponeTransmitApolloGateway;
+  const error = hasErrors ? new Error(span.data.graphql.errors || 'GraphQL errors') : null;
+
+  tracingUtil.completeSpan({
+    span,
+    referenceFunction: stackTraceRef || finishSpan,
+    error,
+    skipTransmit: shouldSkipTransmit
+  });
 }
 
-function finishWithException(span, err) {
-  span.ec = 1;
-  span.d = Date.now() - span.ts;
+function finishWithException(span, err, stackTraceRef) {
   span.data.graphql.errors = err.message;
-  if (!span.postponeTransmit) {
-    span.transmit();
-  }
+
+  const shouldSkipTransmit = span.postponeTransmit;
+
+  tracingUtil.completeSpan({
+    span,
+    referenceFunction: stackTraceRef || finishWithException,
+    error: err,
+    skipTransmit: shouldSkipTransmit
+  });
 }
 
 function instrumentApolloGatewayExecuteQueryPlan(apolloGatewayExecuteQueryPlanModule) {

packages/core/src/tracing/instrumentation/protocols/http2Client.js

@@ -120,7 +120,7 @@ function instrumentClientHttp2Session(clientHttp2Session) {
       const pathWithoutQuery = sanitizeUrl(path);
       const params = splitAndFilter(path);
 
-      span.stack = tracingUtil.getStackTrace(request);
+      const spanStackRefFn = originalRequest;
 
       span.data.http = {
         method,
@@ -138,13 +138,12 @@ function instrumentClientHttp2Session(clientHttp2Session) {
       });
 
       stream.on('end', () => {
-        span.d = Date.now() - span.ts;
         span.ec = status >= 500 ? 1 : 0;
         span.data.http.status = status;
         if (capturedHeaders) {
           span.data.http.header = capturedHeaders;
         }
-        span.transmit();
+        tracingUtil.completeSpan({ span, referenceFunction: spanStackRefFn, error: null });
       });
 
       return stream;

packages/core/src/tracing/instrumentation/protocols/httpClient.js

@@ -225,8 +225,12 @@ function instrument(coreModule, forceHttps) {
         params = urlAndQuery[1];
       }
 
+      // This needs to be revisited and removed
       span.stack = tracingUtil.getStackTrace(request);
 
+      // Store original request for span stack collection
+      const spanStackRefFn = originalRequest;
+
       const boundCallback = cls.ns.bind(function boundCallback(res) {
         span.data.http = {
           method: clientRequest.method,
@@ -240,9 +244,8 @@ function instrument(coreModule, forceHttps) {
           span.data.http.header = headers;
         }
 
-        span.d = Date.now() - span.ts;
-        span.ec = res.statusCode >= 500 ? 1 : 0;
-        span.transmit();
+        const error = res.statusCode >= 500 ? new Error(`HTTP ${res.statusCode}`) : null;
+        tracingUtil.completeSpan({ span, referenceFunction: spanStackRefFn, error });
 
         if (callback) {
           callback(res);
@@ -266,9 +269,8 @@ function instrument(coreModule, forceHttps) {
           url: completeCallUrl,
           error: e ? e.message : ''
         };
-        span.d = Date.now() - span.ts;
-        span.ec = 1;
-        span.transmit();
+
+        tracingUtil.completeSpan({ span, referenceFunction: spanStackRefFn, error: e });
         throw e;
       }
 
@@ -314,9 +316,8 @@ function instrument(coreModule, forceHttps) {
           url: completeCallUrl,
           error: errorMessage
         };
-        span.d = Date.now() - span.ts;
-        span.ec = 1;
-        span.transmit();
+
+        tracingUtil.completeSpan({ span, referenceFunction: spanStackRefFn, error: err });
       });
     });
     return clientRequest;
@@ -336,10 +337,10 @@ function constructFromUrlOpts(options, self, forceHttps) {
 
   try {
     const agent = options.agent || self.agent;
-    const isHttps = forceHttps || options.protocol === 'https:' || (agent?.protocol === 'https:');
+    const isHttps = forceHttps || options.protocol === 'https:' || agent?.protocol === 'https:';
 
     // Use protocol-aware default port, 443 for HTTPS, 80 for HTTP
-    const port = options.port || options.defaultPort || (agent?.defaultPort) || (isHttps ? 443 : 80);
+    const port = options.port || options.defaultPort || agent?.defaultPort || (isHttps ? 443 : 80);
 
     const protocol =
       (port === 443 && 'https:') ||

packages/core/src/tracing/instrumentation/protocols/nativeFetch.js

@@ -167,7 +167,9 @@ function instrument() {
         params
       };
 
-      span.stack = tracingUtil.getStackTrace(instanaFetch);
+      // span.stack = tracingUtil.getStackTrace(instanaFetch);
+
+      tracingUtil.completeSpan({ span, referenceFunction: instanaFetch });
 
       injectTraceCorrelationHeaders(originalArgs, span, w3cTraceContext);
 
@@ -183,15 +185,16 @@ function instrument() {
           );
         })
         .catch(err => {
-          span.ec = 1;
           span.data.http.error = err.message;
+
+          tracingUtil.completeSpan({ span, referenceFunction: instanaFetch, skipTransmit: true, error: err });
         })
         .finally(() => {
-          span.d = Date.now() - span.ts;
           if (capturedHeaders != null && Object.keys(capturedHeaders).length > 0) {
             span.data.http.header = capturedHeaders;
           }
-          span.transmit();
+
+          tracingUtil.completeSpan({ span, referenceFunction: instanaFetch });
         });
 
       return fetchPromise;