diff --git a/packages/opentelemetry-plugin-express/src/express.ts b/packages/opentelemetry-plugin-express/src/express.ts index b519ad0cbc5..5f60e18f434 100644 --- a/packages/opentelemetry-plugin-express/src/express.ts +++ b/packages/opentelemetry-plugin-express/src/express.ts @@ -204,12 +204,36 @@ export class ExpressPlugin extends BasePlugin { return patchEnd(span, plugin._tracer.bind(next))(); }; } + /** + * Ending a layer's span at the right time is quite difficult because + * of their nature: they can be either sync or async and either it call + * their callback or not, and we can't know that before calling it. + * + * Here are the different state that can result from calling the layer code: + * 1. The layer is sync and call its callback + * - it will be correctly closed by the above code + * 2. The layer is sync and never call its callback + * - we need to end the span manually after calling the layer code + * 3. The layer is async and call its callback + * - it will be correctly closed by the above code + * 4. The layer is async and never call its callback + * - we need to end the span manually + * - however it will not represent the actual time that the span took + * - we would need to hook into the http response's `end` event. + * + * The fix for case 4) is to hook into the `end` event, but it would apply + * to case 2) and 3), which would cost quite a lot of memory for big express apps. + * + * The workaround is to schedule a callback to run on next tick and check if the + * callback has been called. If not we close the span because we assume being + * in the case 2) or 4). + */ const result = original.apply(this, arguments); - // if the layer return a response, the callback will never - // be called, so we need to manually close the span - if (callbackHasBeenCalled === false) { - span.end(); - } + setImmediate(() => { + if (callbackHasBeenCalled === false) { + span.end() + } + }) return result; }; });