Add HttpClient tracer header filter - #6697
Merged
tim-smart merged 3 commits intoJul 30, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: dd259e8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughHttpClient tracing now exposes a context-configurable header predicate. Request and response span attributes are emitted only for matching headers, while the default predicate preserves existing behavior. Tests cover default recording and suffix-based filtering. ChangesHTTP client tracing
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant FiberContext
participant HttpClient
participant Tracer
FiberContext->>HttpClient: Provide TracerHeaderFilter
HttpClient->>Tracer: Record matching request headers
HttpClient->>Tracer: Record matching response headers
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Comment |
Contributor
Author
|
Local verification is green:
Remote CI is pending Effect-TS maintainer workflow approval for this first-time fork contribution; it is not reporting a test failure. This implements the Posted on behalf of @schickling
|
schickling-assistant
marked this pull request as ready for review
July 28, 2026 09:29
schickling-assistant
added a commit
to overengineeringstudio/effect-utils
that referenced
this pull request
Jul 28, 2026
The @effect/platform patch allowlists seven HTTP headers before they become span attributes. That package is merged into core in Effect 4, so the patch cannot survive as-is, and dropping it would silently change what we emit to telemetry. The obvious lead - beta.102's new HttpClientRequest.updateHeaders/removeHeader - was probed and refuted on two counts: those combinators mutate the request actually sent to transport, so filtering telemetry with them drops real headers from the wire, and they cannot reach response span attributes at all. Precise beta.102 behaviour, verified in source: every header name becomes a span attribute on both request and response paths. Values matching Headers.CurrentRedactedNames render as <redacted>, and that list is configurable, so value protection is already solved. What has no hook is suppressing the attribute entirely - a telemetry-shape and cardinality concern rather than a credential-leak one. The upstream fix already existed as our own issue #6363 (its v3 predecessor #6198 was closed only because v3 was frozen). That issue proposed a predicate and offered a PR that nobody built, so we built it: Effect-TS/effect#6697 adds a TracerHeaderFilter reference defaulting to constTrue and applies it to both attribute loops. Until that lands, a minimal core patch is the interim bridge, deliberately scoped to be deleted rather than maintained. Refs #925
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
schickling-assistant
added a commit
to overengineeringstudio/effect-utils
that referenced
this pull request
Jul 28, 2026
The @effect/platform patch allowlists seven HTTP headers before they become span attributes. That package is merged into core in Effect 4, so the patch cannot survive as-is, and dropping it would silently change what we emit to telemetry. The obvious lead - beta.102's new HttpClientRequest.updateHeaders/removeHeader - was probed and refuted on two counts: those combinators mutate the request actually sent to transport, so filtering telemetry with them drops real headers from the wire, and they cannot reach response span attributes at all. Precise beta.102 behaviour, verified in source: every header name becomes a span attribute on both request and response paths. Values matching Headers.CurrentRedactedNames render as <redacted>, and that list is configurable, so value protection is already solved. What has no hook is suppressing the attribute entirely - a telemetry-shape and cardinality concern rather than a credential-leak one. The upstream fix already existed as our own issue #6363 (its v3 predecessor #6198 was closed only because v3 was frozen). That issue proposed a predicate and offered a PR that nobody built, so we built it: Effect-TS/effect#6697 adds a TracerHeaderFilter reference defaulting to constTrue and applies it to both attribute loops. Until that lands, a minimal core patch is the interim bridge, deliberately scoped to be deleted rather than maintained. Refs #925
schickling-assistant
added a commit
to overengineeringstudio/effect-utils
that referenced
this pull request
Jul 28, 2026
* Add Effect 4 migration pattern catalog and verification Phase 0 of the Effect 3 to 4 migration (#925) proved each migration pattern in isolation before touching real packages, using a differential harness that runs both majors as separate processes and diffs normalized behaviour traces. These are the durable outputs: - recipes/ - 17 per-pattern v3 to v4 recipes with equivalence evidence, gotchas, and codemod rules where the rewrite is mechanical - alignment-register.md - every intended v3/v4 behaviour difference with a proposed decision and blast radius; the sign-off artifact - register-verification.md - an adversarial pass over the register that tried to refute each accepted difference; 10 upheld, 2 refuted - idiom-catalog.md - decision-ready refactor patterns with adopt/defer/skip dispositions, evidence drawn from LiveStore's completed migration The harness itself stays in tmp/ as throwaway; only the findings are durable. Notable results: span shape is identical under both recording and non-recording tracers, so otel-contract's published contract is unaffected. Fork defaults are identical, so the compatibility options LiveStore cargo-culted are unnecessary. Two register entries were refuted and need reclassification before migration starts - the RPC failure envelope changes shape across a versionless browser/server protocol, and Restate embeds parser text verbatim in an HTTP 400 payload. The CLI work also surfaced an upstream bug, filed as Effect-TS/effect#6690 with a fix PR. Refs #925, #981 * Record tsgo compatibility with Effect 4 beta.102 The pinned effect-tsgo (8d34c0a, locked 2026-06-17) was the last unanswered blocker for the Effect 4 migration: whether a six-week-old compiler pin could typecheck v4's module layout, and whether the Effect diagnostics gate would survive the flip. It can, and it does. The probe resolves every v4 subpath we use and proves all three diagnostic categories still affect the build exit code - warning-only and suggestion-only probes each exited 2, so the gate is not silently degraded to errors-only. Consequences: no tsgo bump is on the migration's critical path, so the flip and any future compiler bump stay independent changes; and the errors-only waiver stays an unused contingency rather than a planned step. Also records the exact waiver mechanism should it ever be needed, and corrects an earlier misreading - the comment at genie/external.ts:718-735 is accurate, since the gate fields are the inverse of the generated ignore flags, as documented at :768-771. Refs #925 * Record the platform header-patch analysis and its upstream path The @effect/platform patch allowlists seven HTTP headers before they become span attributes. That package is merged into core in Effect 4, so the patch cannot survive as-is, and dropping it would silently change what we emit to telemetry. The obvious lead - beta.102's new HttpClientRequest.updateHeaders/removeHeader - was probed and refuted on two counts: those combinators mutate the request actually sent to transport, so filtering telemetry with them drops real headers from the wire, and they cannot reach response span attributes at all. Precise beta.102 behaviour, verified in source: every header name becomes a span attribute on both request and response paths. Values matching Headers.CurrentRedactedNames render as <redacted>, and that list is configurable, so value protection is already solved. What has no hook is suppressing the attribute entirely - a telemetry-shape and cardinality concern rather than a credential-leak one. The upstream fix already existed as our own issue #6363 (its v3 predecessor #6198 was closed only because v3 was frozen). That issue proposed a predicate and offered a PR that nobody built, so we built it: Effect-TS/effect#6697 adds a TracerHeaderFilter reference defaulting to constTrue and applies it to both attribute loops. Until that lands, a minimal core patch is the interim bridge, deliberately scoped to be deleted rather than maintained. Refs #925 * Close the three Phase 0 coverage gaps and record the wave-0 constraint Phase 0 left three boundaries uncovered - filesystem watch ordering, Terminal and Prompt PTY behaviour, and child-process backpressure. Each maps to a package where a silent break was plausible, so they are closed before wave repair reaches them. Two of the three turned out to be non-deterministic within a single major: filesystem watch varies in ordering, coalescing and even delete-tag classification, and child-process traces vary in chunk boundaries and cross-stream interleaving. Their gates are therefore invariant-based rather than exact-trace: path membership and final state for watch, whole-stream byte counts and lossless completion under a slow consumer for child processes. That distinction is now a documented harness design rule. Pure patterns can be exact-ordered-trace gated; anything crossing the OS must be invariant-gated, and each recipe states which properties are deliberately not gated so a later reader does not mistake the gap for an oversight and pin something flaky. One real cross-major difference emerged: beta.102 removes recursive control from FileSystem.watch and forces recursive Node and Bun watches, which silently widens watch scope. Filed upstream as Effect-TS/effect#6698. The watch work also prompted an audit of our own tag-dependent consumers, which found one fragile optimisation guarded by a polling fallback - filed as #986, independent of this migration. Also records why wave 0 could not proceed naively: every candidate needs a still-separate @effect/* package, and strict peer resolution rejects a partial cohort. The flip must therefore move core effect and every still-separate @effect/* package in one commit. Refs #925, #986 * Record operational notes for writing Phase 1 baselines Capturing the Effect 3 baselines that gate the Effect 4 flip surfaced a set of traps that are invisible until they bite, and several of them look exactly like success: a test task that does not exist, a test file that is never collected, and a validation command that does not run tests all present as green. Written down so the remaining baselines and the downstream migration do not rediscover them: - check:quick is TypeScript and lint only; for a test-only PR it proves almost nothing. Validate with the package test task on a committed worktree and confirm the test count via the otel-scrape summary, since exit 0 is compatible with collecting zero tests. - devenv generates a test task only for packages listed in packagesWithTests, and there is no root vitest run in the task graph, so an unlisted package is never tested by CI. Includes the verification-worktree technique for validating a baseline whose task does not yet exist on main. - Vitest include patterns are package-relative and differ per package, so a root-path invocation can silently collect nothing. - A lockfile change cascades across eight fixed-output derivations, with the exact evergreen invocations, because chase-fod-closure cannot help when consumers lack fodGraph metadata. - The lint gate covers only three paths, and the instrumented task reports the offending file by hash rather than path, so a single-line error can present as an opaque failure. Also records which artifacts carry migration markers: baselines are permanent gates and get inline TODOs at most, never bridge blocks. Refs #925 * test: check Effect 4 baseline migration markers * style: format Effect 4 migration findings with oxfmt The findings live under context/, which the lint gate covers, and they were written by hand without running the formatter. CI caught it at lint:check:format.
tim-smart
reviewed
Jul 30, 2026
agent-session-id: 133b5d2a-3d5d-4f52-8f55-f12a834f6b13 agent-tool: Codex CLI agent-tool-version: 0.145.0 agent-model: unknown agent-runtime-profile: /nix/store/ph8rlhdj25mg71v81jsfzy6dq4xpcs9m-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/lsykz8x5481xrpbgk280xh3pypk1c5jy-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@3649b53
tim-smart
force-pushed
the
schickling-assistant/2026-07-28-httpclient-header-filter
branch
from
July 30, 2026 23:28
5fa1c49 to
dd259e8
Compare
tim-smart
enabled auto-merge (squash)
July 30, 2026 23:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
HttpClientcurrently emits every request and response header as a span attribute. Value redaction is configurable independently, but there is no way to control which header attributes are included.Closes #6363.
What
Add
HttpClient.TracerHeaderFilter, a context reference that filters request and response header span attributes by normalized header name.How
The predicate defaults to
constTrue, preserving current behavior. Both header attribute loops read the same predicate without mutating transmitted or received headers.Tests cover unchanged default inclusion and selective suppression on both request and response paths.
Validation
pnpm lint-fixpnpm --filter effect test --run test/unstable/http/HttpClient.test.tspnpm test-types packages/effect/typetest/unstable/http/HttpClient.tst.tspnpm checkPosted on behalf of @schickling
agent_nameagent_session_idagent_toolagent_tool_versionagent_runtimeagent_modelruntime_profileskills_manifestworktreemachinetooling_profileSummary by CodeRabbit