-
Notifications
You must be signed in to change notification settings - Fork 4
Fix: Preserve Authorization headers in fetch instrumentation #208
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
Fix: Preserve Authorization headers in fetch instrumentation #208
Conversation
The OpenTelemetry fetch instrumentation was overwriting existing headers when adding trace context. This caused issues with third-party SDKs like E2B that set Authorization headers, resulting in 'authorization header is missing' errors. Changes: - Only add trace context headers if they don't already exist - Prevents overwriting critical headers like Authorization - Properly preserves all existing headers from init.headers
|
Warning Rate limit exceeded@jhaynie has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughModified header injection logic in the instrumented fetch path to conditionally apply trace context headers only when not already present, replacing unconditional header overwriting with safer preservation of existing headers. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/otel/fetch.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/{logger,otel}/**
📄 CodeRabbit inference engine (AGENT.md)
Infrastructure code goes under src/logger/ and src/otel/ (OpenTelemetry)
Files:
src/otel/fetch.ts
{src,test}/**/!(*.d).ts
📄 CodeRabbit inference engine (AGENT.md)
{src,test}/**/!(*.d).ts: Use strict TypeScript and prefer unknown over any
Use ESM import/export syntax; avoid CommonJS require/module.exports
Use relative imports for internal modules
Keep imports organized (sorted, no unused imports)
Use tabs with a visual width of 2 spaces
Limit lines to a maximum of 80 characters
Use single quotes for strings
Use proper Error types; do not throw strings
Prefer template literals over string concatenation
Files:
src/otel/fetch.ts
The OpenTelemetry fetch instrumentation was overwriting existing headers when adding trace context. This caused issues with third-party SDKs like E2B that set Authorization headers, resulting in 'authorization header is missing' errors.
Changes:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.