From aff48a1f0045c85d0b97d15ea98d03f5909b14bc Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 23 Feb 2024 02:02:06 -0800 Subject: [PATCH] fix(instr-fetch): do not enable in Node.js; clarify in docs this instr is for web fetch only (#4498) * fix(instr-fetch): do not enable in Node.js; clarify in docs this instr is for web fetch only * add a changelog entry * add a diagnostic warning if attempting to use instr-fetch in Node.js * fixup! add a diagnostic warning if attempting to use instr-fetch in Node.js --------- Co-authored-by: Marc Pichler --- experimental/CHANGELOG.md | 1 + .../opentelemetry-instrumentation-fetch/README.md | 3 ++- .../src/fetch.ts | 13 +++++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 2442005860e..000ef35e3bc 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -26,6 +26,7 @@ All notable changes to experimental packages in this project will be documented * fix(instrumentation): normalize paths for internal files in scoped packages [#4467](/~https://github.com/open-telemetry/opentelemetry-js/pull/4467) @pichlermarc * Fixes a bug where, on Windows, internal files on scoped packages would not be instrumented. * fix(otlp-transformer): only use BigInt inside hrTimeToNanos() [#4484](/~https://github.com/open-telemetry/opentelemetry-js/pull/4484) @pichlermarc +* fix(instrumentation-fetch): do not enable in Node.js; clarify in docs this instr is for web fetch only [#4498](/~https://github.com/open-telemetry/opentelemetry-js/pull/4498) @trentm ### :books: (Refine Doc) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/README.md b/experimental/packages/opentelemetry-instrumentation-fetch/README.md index 2b5336be0b9..367d5c9ef1f 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/README.md +++ b/experimental/packages/opentelemetry-instrumentation-fetch/README.md @@ -5,7 +5,8 @@ **Note: This is an experimental package under active development. New releases may include breaking changes.** -This module provides auto instrumentation for web using fetch. +This module provides auto instrumentation for web using [fetch](https://developer.mozilla.org/en-US/docs/Web/API/fetch). +(Note: This instrumentation does **not** instrument [Node.js' fetch](https://nodejs.org/api/globals.html#fetch). See [this issue](/~https://github.com/open-telemetry/opentelemetry-js/issues/4333).) ## Installation diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index 7960b38535c..1f80bfd953a 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -35,6 +35,8 @@ import { _globalThis } from '@opentelemetry/core'; // safe enough const OBSERVER_WAIT_TIME_MS = 300; +const isNode = typeof process === 'object' && process.release?.name === 'node'; + export interface FetchCustomAttributeFunction { ( span: api.Span, @@ -465,6 +467,14 @@ export class FetchInstrumentation extends InstrumentationBase< * implements enable function */ override enable(): void { + if (isNode) { + // Node.js v18+ *does* have a global `fetch()`, but this package does not + // support instrumenting it. + this._diag.warn( + "this instrumentation is intended for web usage only, it does not instrument Node.js's fetch()" + ); + return; + } if (isWrapped(fetch)) { this._unwrap(_globalThis, 'fetch'); this._diag.debug('removing previous patch for constructor'); @@ -476,6 +486,9 @@ export class FetchInstrumentation extends InstrumentationBase< * implements unpatch function */ override disable(): void { + if (isNode) { + return; + } this._unwrap(_globalThis, 'fetch'); this._usedResources = new WeakSet(); }