Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auto-instrumentation-fetch replaces headers when req.headers input is of type Map #4347

Closed
rdeavila94 opened this issue Dec 6, 2023 · 1 comment · Fixed by #4348
Closed
Labels
bug Something isn't working priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies, etc

Comments

@rdeavila94
Copy link
Contributor

rdeavila94 commented Dec 6, 2023

What happened?

Steps to Reproduce

initialize fetch instrumentation

fetch('https://foobar.com', {headers: new Map().set('foo','bar')}

Expected Result

A request to foobar.com, with the header foo in addition to any trace propagation headers

Actual Result

The header foo is omitted, and only the trace propagation headers are sent

Additional Details

This is a result of this function. Since it is not handling a type of Map, it is resorting to extracting the keys as if the headers are an object literal, which it isn't.

It is important to note that a type of Map is not, according to the type definition of Request.Header, a valid type. However, it is fairly reasonable to expect that some folk may pass a Map type as their headers when using fetch, and in JavaScript will not result in any type errors/warnings. A Map type functionality-wise is valid, as it works as a type for headers when using fetch. The current code results in a bug where users who use fetch and have set their headers with a Map type have their headers entirely replaced by instrumentation-fetch when using it, where they were previously present without instrumentation-fetch.

OpenTelemetry Setup Code

// see PR

package.json

// see PR

Relevant log output

No response

@rdeavila94
Copy link
Contributor Author

I've opened this PR to fix the issue

@pichlermarc pichlermarc added priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies, etc and removed triage labels Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies, etc
Projects
None yet
2 participants