opentelemetry-js
opentelemetry-js copied to clipboard
Add instrumentation for `fetch` API
Is your feature request related to a problem? Please describe.
NodeJS's global fetch API is not instrumented therefore we do not get traces for requests made with it and also the tracecontext is not propagated. The lack of this instrumentation may raise more issues like this one #4247 and switching from fetch to http or any packages which has an available instrumentation like undici may be a no go for the devs
There's a package @opentelemetry/instrumentation-fetch but is specific for web platform since its depending @opentelemetry/sdk-trace-web so we need an instrumetation that fits for nodejs' fetch. There are some questions though.
- should we make the current instrumentation agnostic of the platform (web/node)?
- if not how should we name this new package?
@opentelemetry/instrumentation-node-fetchcould make users think is for node-fetch package- is there any way to deduct from the name if the package is targeted for web or node? or if its for instrumenting a package or a platform API?
Trying to answer myself to the 1st question I'd say we can not forecast what are the instrumentation needs for each platform. As an example we see in the current code that is taking in consideration CORS preflight requests which is very common in web but not so much in node. So I'd prefer to have a specific instrumentation for nodejs' fetch
About the naming I do not know if there is a convention so feedback from @dyladan or @pichlermarc would be appreciated :)
Refs: #4247
Describe the solution you'd like
Create a new instrumentation package for nodejs' fetch global API and place it in this repository. There is a mention in https://github.com/open-telemetry/opentelemetry-js/discussions/3346 of a couple of instrumentations that we could take as a good stating point
- https://github.com/gadget-inc/opentelemetry-instrumentations/tree/main/packages/opentelemetry-instrumentation-undici
- https://github.com/gas-buddy/service/blob/main/src/telemetry/fetchInstrumentation.ts#L44
IMO 2nd implementation is the way to go since fetch is hooking internally to undici and it won't detected by our require/import hooks. See section below.
Describe alternatives you've considered
- Refactor the app to use
httpmodule or a package that could be instrumented. - install
opentelemetry-instrumentation-undicito check if the internal module is instrumented
See https://github.com/open-telemetry/opentelemetry-js/issues/4247#issuecomment-1813236388
Additional context
@dyladan @pichlermarc could you assign this one to me? Thanks :)
@david-luna Perhaps clarify that this is about Node.js's global fetch(). IIUC https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/packages/opentelemetry-instrumentation-fetch/ instruments fetch() for web usage. I wonder if that instrumentation might be close to also being able to instrument Node.js's fetch(). When enabled it shims globalThis.fetch. That said, the undici diagnostics_channel events from Node's implementation of fetch() might provide more interesting details for tracing.
yeah, it was very short. I've updated with more info :)
I've reached the dev who shared his fetch implementation in the same conversation
https://github.com/open-telemetry/opentelemetry-js/discussions/3346#discussioncomment-7856098
I've try another channel if there is no answer in few days
@djMax thanks on answering on the question about donating the implementation of https://github.com/gas-buddy/service/blob/main/src/telemetry/fetchInstrumentation.ts to this repo.
This is the issue we're using to track it's progress. As discussed in the JavaScript SIG (see Dec 13 notes) the goal would be to have a new package named @opentelemetry/instrumentation-undici which will serve to instrument undici package and also the global fetch API. This new package would be placed within experimental/packages folder along with HTTP & GRPC instrumentations.
Tasks to be done
- [ ] Port the code
- [ ] Create tests for global
fetch - [ ] Create tests for
undicipackage - [ ] Add documentation
I can create a draft PR to start so you can give a 1st review or we could do it the other way around. Up to you. :)
Happy to review a PR, and to try it out in a place with tests that verify it's function from a consumer perspective.
This was discussed in the OTel JS SIG today (https://docs.google.com/document/d/1tCyoQK49WVcE-x8oryZOTTToFm7sIeUhxFPm9g-qL1k/edit#heading=h.ry29ajruzxyg). It was concluded to move this instrumentation to the contrib repo.
Link to the contrib PR: https://github.com/open-telemetry/opentelemetry-js-contrib/pull/1951