Skip to content

Commit

Permalink
fix(plugin-express): fix double span end open-telemetry#908
Browse files Browse the repository at this point in the history
  • Loading branch information
vmarchaud committed Apr 5, 2020
1 parent de0e26e commit 4978047
Showing 1 changed file with 29 additions and 5 deletions.
34 changes: 29 additions & 5 deletions packages/opentelemetry-plugin-express/src/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,36 @@ export class ExpressPlugin extends BasePlugin<typeof express> {
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;
};
});
Expand Down

0 comments on commit 4978047

Please sign in to comment.