fastify parent-child relationship of middleware and request handler is wrong

AbhiPrasad commented 5 months ago

What version of OpenTelemetry are you using?

"@opentelemetry/api": "1.7.0",
"@opentelemetry/context-async-hooks": "1.21.0",
"@opentelemetry/core": "1.21.0",
"@opentelemetry/instrumentation": "0.48.0",
"@opentelemetry/instrumentation-fastify": "0.33.0",

What version of Node are you using?

Node v21.6.2

What did you do?

Using default fastify instrumentation as per https://github.com/getsentry/sentry-javascript/blob/f6a3e026ac7f04eec78495a14c0c68f9e553e22d/packages/node/src/integrations/tracing/fastify.ts#L8-L23

What did you expect to see?

parent spans fully encapsulate their children.

What did you see instead?

The middleware spans end early (they only time middleware logic itself), which means that the child request handler span is longer than it's parent. I would expect middleware spans to contain the request handler in entirely, or the request handler to be parent under http server request span directly.

Additional context

raphael-theriault-swi commented 5 months ago

Related to #2022

AbhiPrasad commented 5 months ago

If the SIG feels like the fix is to just adjust the middleware/handler spans duration/end timestamps, happy to help with a PR (can take a look at express as well). Just need to be pointing in right direction here 😄

dyladan commented 4 months ago

The timestamps are not the issue here. The context actually needs to be propagated correctly and the structure of these packages makes it difficult to properly address parent-child relations.

I'm not sure this is actually the way we want to go though. These could be considered siblings or children in my opinion. Can you provide a code example?

open-telemetry / opentelemetry-js-contrib