Skip to content

fix(sdk): stop the browser client setting user-agent on its OTLP requests - #1338

Open
JeremyFunk wants to merge 1 commit into
mainfrom
fix/sdk-browser-drop-user-agent
Open

JeremyFunk wants to merge 1 commit into
mainfrom
fix/sdk-browser-drop-user-agent

Conversation

@JeremyFunk

@JeremyFunk JeremyFunk commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

What

The browser client entry (@maple-dev/effect-sdk/client) no longer sets user-agent on its OTLP requests. Server and Cloudflare presets are unchanged.

  • buildResolved (shared/flush-core.ts) takes a per-preset sendUserAgent. x-maple-sdk is always set; user-agent only when the preset asks for it. Client: false. Server, Cloudflare: true.
  • Maple.layer (client) is built on Effect's Otlp.layerJson, whose exporter sets user-agent: effect-opentelemetry-<OtlpTracer|OtlpLogger|OtlpMetrics>/0.0.0 and merges it over caller headers, so it cannot be overridden through headers. The client's HTTP client wrapper (client/consent-http-client.ts) now removes it before the request goes out.
  • Comments at buildResolved and in version.ts updated: some browsers do send a script-set user-agent.

Why

User-Agent is not on the Fetch standard's forbidden request-header list, and it is not a CORS-safelisted request header (those are Accept, Accept-Language, Content-Language, Content-Type, Range). A browser that honours a script-set value therefore lists it in the preflight, and an endpoint whose CORS policy does not allow it rejects the preflight and with it the POST.

Measured 2026-10-08 against a mock ingest whose CORS answers mirror ingest's allow-list:

Browser Script-set user-agent Cross-origin OTLP POSTs delivered
Chrome 154, Chromium 153 dropped all
Firefox 157 sent 0 of 7 spans, with and without an ingest key

Firefox's preflight: Access-Control-Request-Headers: authorization,content-type,user-agent,x-maple-sdk. With user-agent allowed by the endpoint, Firefox delivers everything.

Safari/WebKit: not measured. WebKit's isForbiddenHeaderName (Source/WebCore/platform/network/HTTPParsers.cpp) does not list User-Agent, so by source WebKit accepts a script-set value like Firefox does. MDN's forbidden-request-header page names only Chrome as still dropping it (Chromium bug 571722).

The ingest allow-list is extended separately. Removing the header on the SDK side also covers proxies and self-hosted collectors that have their own CORS policy.

Headers per preset after this change

Preset user-agent x-maple-sdk
MapleFlush.make server maple-effect-sdk-server/<v> same value
Cloudflare maple-effect-sdk-cloudflare/<v> same value
MapleFlush.make client not set (browser's own) maple-effect-sdk-client/<v>
Maple.layer client not set (browser's own) not set (as before)

Session and replay posts were already fine: ingestHeaders in packages/browser-session never sets user-agent.

What ingest reads

record_client_identity in apps/ingest/src/main.rs records two attributes on the request handler span: maple.sdk from x-maple-sdk, and user_agent.original from user-agent (each cut to 128 characters). For the browser client, maple.sdk is unchanged. user_agent.original becomes the browser's own string in every browser, which is what Chromium already produced.

Every preset that sent x-maple-sdk still sends it, so maple.sdk keeps being recorded. The review bot's "removes attribute maple.sdk" line points at flush-core.ts:93, where only the comment above the header moved.

Decisions

  • React Native: it uses the same /client entry and the same transport as the browser; there is no React Native branch in the code. It follows the browser path, so the runtime's default user-agent goes out and x-maple-sdk identifies the SDK for MapleFlush.make. No runtime detection added to keep it one code path; there is no CORS in React Native, so either choice delivers.
  • Maple.layer and x-maple-sdk: not added here. A new request header has to be allowed by every cross-origin endpoint's CORS policy, which is the same class of change this PR removes.
  • Docs: no sentence in the README, docs/ or the landing docs describes the old behaviour, and none was added.
  • No new config option, no version bump.

Tests

  • shared/flush-core.test.ts: sendUserAgent: true sets both headers; false sets x-maple-sdk only.
  • client/flushable.test.ts: the client's trace POST carries x-maple-sdk and no user-agent (checked case-insensitively).
  • client/layer.test.ts, client/react-native.test.ts: no request from Maple.layer or MapleFlush.make carries user-agent. Both fail without the change.
  • server/flushable.test.ts, cloudflare/index.test.ts: both presets still send user-agent equal to x-maple-sdk.

Run locally: those six files plus client/consent-http-client.test.ts, and the package typecheck.

A real-browser cross-origin check of this branch (Firefox, Chrome) will be run separately; it has not been run yet.

…ests

Firefox sends a script-set user-agent, and the header is not CORS-safelisted, so a cross-origin endpoint that does not allow it rejects the preflight and every OTLP POST with it. The client entry now leaves user-agent to the browser in both MapleFlush.make and Maple.layer; x-maple-sdk still carries the SDK identity. Server and Cloudflare presets are unchanged.
@maple-review-bot

maple-review-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 9/10 · safe to merge
Change is header-only and symmetric across presets, pinned by a wire-level assertion in client/layer.test.ts; the only unverified part is Effect's exporter header, which that test covers.
quality 100/100 · no findings · tests covered · risk medium

Both browser presets stop sending a script-set user-agent on OTLP posts: a per-preset sendUserAgent flag in buildResolved (false for the client) plus a header strip in the client's HTTP wrapper for Effect's own exporter header. Server and Cloudflare are untouched. Safe to merge.

  • buildResolved takes sendUserAgent; x-maple-sdk is always sent
  • Client MapleFlush passes sendUserAgent: false; server and Cloudflare pass true
  • consentHttpClientLayer strips user-agent off Effect's OTLP exports for Maple.layer

Production impact

Telemetry this change adds and removes (1)
  • ➖ attribute maple.sdk (~309k/day today) · packages/effect-sdk/src/shared/flush-core.ts:93
What was checked
  • All three buildResolved call sites updated and no fourth caller exists (grep of buildResolved, packages/effect-sdk only)
  • consentHttpClientLayer is used by the client Maple.layer alone (client/layer.ts:151), so server/Cloudflare OTLP paths keep their header
  • removeHeader with a lowercase name has precedent in this repo (lib/safe-fetch/src/index.ts:196-199) and the wire request is asserted in client/layer.test.ts:86

67ed6a8 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7f9b74e9-d810-4377-ab74-67327838ddb6
📥 Commits

Reviewing files that changed from the base of the PR and between 4bf4b7a and 67ed6a8.

📒 Files selected for processing (12)
  • packages/effect-sdk/src/client/consent-http-client.ts
  • packages/effect-sdk/src/client/flushable.test.ts
  • packages/effect-sdk/src/client/flushable.ts
  • packages/effect-sdk/src/client/layer.test.ts
  • packages/effect-sdk/src/client/react-native.test.ts
  • packages/effect-sdk/src/cloudflare/index.test.ts
  • packages/effect-sdk/src/cloudflare/index.ts
  • packages/effect-sdk/src/server/flushable.test.ts
  • packages/effect-sdk/src/server/flushable.ts
  • packages/effect-sdk/src/shared/flush-core.test.ts
  • packages/effect-sdk/src/shared/flush-core.ts
  • packages/effect-sdk/src/version.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The shared flush builder now controls whether requests include user-agent, while always setting x-maple-sdk. Server and Cloudflare presets enable user-agent; the browser client omits it. Tests verify these header behaviors across SDK clients.

Changes

SDK Header Configuration

Layer / File(s) Summary
Shared header contract and behavior
packages/effect-sdk/src/shared/flush-core.ts, packages/effect-sdk/src/shared/flush-core.test.ts
buildResolved now requires sendUserAgent. It always sets x-maple-sdk and sets user-agent only when enabled. Tests cover the option and keyless cases.
Server and Cloudflare header settings
packages/effect-sdk/src/server/flushable.ts, packages/effect-sdk/src/server/flushable.test.ts, packages/effect-sdk/src/cloudflare/index.ts, packages/effect-sdk/src/cloudflare/index.test.ts
The server and Cloudflare presets enable user-agent. Their trace-flush tests check the SDK identifier headers.
Client header handling and request checks
packages/effect-sdk/src/client/flushable.ts, packages/effect-sdk/src/client/consent-http-client.ts, packages/effect-sdk/src/client/flushable.test.ts, packages/effect-sdk/src/client/layer.test.ts, packages/effect-sdk/src/client/react-native.test.ts, packages/effect-sdk/src/version.ts
The browser preset disables user-agent; the consent client removes it before forwarding filtered requests. Client tests check the outgoing headers.

Priority: ⚪ Not assessed

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: makisuo

Merge Risk: ⚪ Minimal · up to 67ed6

Browser OTLP requests omit the script-set User-Agent while retaining SDK identity. The available evidence does not establish a current-head Firefox delivery failure; real-browser cross-origin effectiveness remains unverified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: the browser client no longer sets the user-agent header on OTLP requests.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

1 participant