Skip to content

fix(instrumentation-fetch): do not consume Request before ignoreUrls - #7038

Open
BetterAndBetterII wants to merge 3 commits into
open-telemetry:mainfrom
BetterAndBetterII:fix/fetch-request-init-ignore-urls
Open

fix(instrumentation-fetch): do not consume Request before ignoreUrls#7038
BetterAndBetterII wants to merge 3 commits into
open-telemetry:mainfrom
BetterAndBetterII:fix/fetch-request-init-ignore-urls

Conversation

@BetterAndBetterII

Copy link
Copy Markdown

Fixes #7037. Delay Request clone until after the ignoreUrls span check so fetch(Request, init) does not throw on ignored URLs.

new Request(existing, init) locks the original body. Creating that
clone before the span check meant ignored fetch(Request, init) calls
threw when the wrapper forwarded the consumed Request.

Fixes open-telemetry#7037

Signed-off-by: Yuzhong Zhang <BetterAndBetterII@users.noreply.github.com>
@BetterAndBetterII
BetterAndBetterII requested review from a team as code owners August 27, 2026 17:26
@github-actions

Copy link
Copy Markdown

Welcome, contributor! Thank you for your contribution to opentelemetry-js.

Important reminders:

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 27, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on the author · refreshed 2026-09-02 19:23 UTC

Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):

  • Inline threads: 1
Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
  • Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

Comment thread experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts Outdated
Keep _createSpan after new Request so a constructor throw cannot leave a span open.

Signed-off-by: Yuzhong Zhang <BetterAndBetterII@users.noreply.github.com>
@BetterAndBetterII

Copy link
Copy Markdown
Author

ignoreUrls is now checked first, and _createSpan runs after new Request, so a constructor throw cannot leave a span open. Added a regression test for that path.

@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.97%. Comparing base (82a5831) to head (da22748).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...s/opentelemetry-instrumentation-fetch/src/fetch.ts 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7038      +/-   ##
==========================================
- Coverage   94.99%   94.97%   -0.03%     
==========================================
  Files         409      409              
  Lines       14338    14344       +6     
  Branches     3280     3284       +4     
==========================================
+ Hits        13621    13623       +2     
- Misses        717      721       +4     
Files with missing lines Coverage Δ
...s/opentelemetry-instrumentation-fetch/src/fetch.ts 81.81% <75.00%> (-1.36%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

}
const createdSpan = plugin._createSpan(url, options);

const createdSpan = plugin._createSpan(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we pass options to _createSpan here? Reading args[1].method again can trigger getters twice and cause unexpected errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

instrumentation-fetch: fetch(Request, init) throws "Request object that has already been used" when the URL is not traced (ignoreUrls / sampled out)

2 participants