chore(otel-node): disable fastify instrumentation and update docs #616
+68
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
trentm
reviewed
Feb 18, 2025
| @@ -70,6 +70,24 @@ requires: | |||
| | `@opentelemetry/instrumentation-undici` | `undici` version range `>=5.12.0` | [README](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/instrumentation-undici#readme) | | |||
| | `@opentelemetry/instrumentation-winston` | `winston` version range `>1 <4` | [README](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-winston#readme) | | |||
|
|
|||
| ### Disabled instrumentations | |||
There was a problem hiding this comment.
How about adding the following changes to the table above:
- change the "References" column heading to "Notes"
- adding a
, [disabled by default](#disabled-instrumentations)entry to the "Notes" for instr-fastify and instr-fs rows above
| @@ -70,6 +70,24 @@ requires: | |||
| | `@opentelemetry/instrumentation-undici` | `undici` version range `>=5.12.0` | [README](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/instrumentation-undici#readme) | | |||
| | `@opentelemetry/instrumentation-winston` | `winston` version range `>1 <4` | [README](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-winston#readme) | | |||
|
|
|||
| ### Disabled instrumentations | |||
|
|
|||
| This Distro takes the upstream package `@opentelemetry/auto-instrumentations-node` as a baseline for technology support. This means additions or removals in its instrumentation list will be revised and usually applied here. As for now the list includes a couple of instrumentations that are disabled by default: | |||
There was a problem hiding this comment.
Suggested alternative:
The following instrumentations are included in EDOT Node.js, but *disabled by default*:
- `@opentelemetry/instrumentation-fs` (Disabled upstream in [open-telemetry/opentelemetry-js-contrib#2467](https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2467).)
- `@opentelemetry/instrumentation-fastify` (Deprecated upstream and slated for removal. See [open-telemetry/opentelemetry-js-contrib#2652](https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2652))
To enable these instrumentations, use the `OTEL_NODE_ENABLED_INSTRUMENTATIONS` environment variable, as documented [here in the OpenTelemetry documentation](https://opentelemetry.io/docs/zero-code/js/configuration/#excluding-instrumentation-libraries). Make sure you list all the instrumentations
you need for your service since only the ones in that list will be enabled. For example:
... the example code as you have it ...
EDOT Node.js uses the upstream [`@opentelemetry/auto-instrumentations-node` package's set of instrumentations](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/metapackages/auto-instrumentations-node/README.md#supported-instrumentations) as a guide for instrumentations to include, exclude, or disable by default. This is to maximize compatibility between usage of EDOT Node.js and the vanilla OpenTelemetry JS SDK.
- I think we are meant to move away from "the Distro" to using "EDOT Node.js"
- My re-ordering here was to put the "here is how it works and how to configure it" info first, and then finish with the justification/reasoning information.
Comment on lines
+45
to
+50
| // ------ trace 9042af (3 spans) ------ | ||
| // span 5894e1 "GET" (6.8ms, SPAN_KIND_CLIENT, GET http://localhost:3000/ping -> 200) | ||
| // +3ms `- span c793a1 "GET /ping" (2.7ms, SPAN_KIND_SERVER, GET http://localhost:3000/ping -> 200) | ||
| // ------ trace 8f3ed8 (3 spans) ------ | ||
| // span 6e0fc9 "GET" (1.6ms, SPAN_KIND_CLIENT, GET http://localhost:3000/hi/Bob -> 200) | ||
| // +1ms `- span 40c4a8 "GET /hi/:name" (0.3ms, SPAN_KIND_SERVER, GET http://localhost:3000/hi/Bob -> 200) |
There was a problem hiding this comment.
This comment is misleading. The expected server spans do NOT have the route info.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds
@opentelemetry/instrumentation-fastifyinto the list of disabled instrumentations. It also places a new section insupported-tchnologies.mdto let the user know which instrumentations are disabled and how to re-enable it.Closes: #556