Skip to content

wip trace header#13646

Merged
petebacondarwin merged 2 commits into
mainfrom
walshy/trace-id-remote
May 13, 2026
Merged

wip trace header#13646
petebacondarwin merged 2 commits into
mainfrom
walshy/trace-id-remote

Conversation

@emily-shen

@emily-shen emily-shen commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Makes getting a trace for debugging easier.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows: setting the env var and getting traces successfully
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: i don't think external users get a trace id on their own?

A picture of a cute animal (not mandatory, but encouraged)


Open in Devin Review

@changeset-bot

changeset-bot Bot commented Apr 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 60ee54d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
miniflare Patch
@cloudflare/pages-shared Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-pool-workers Patch
wrangler Patch

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

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Apr 23, 2026
@workers-devprod workers-devprod requested review from a team and jamesopstad and removed request for a team April 23, 2026 09:50
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • packages/miniflare/src/plugins/shared/constants.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/workers/dispatch-namespace/dispatch-namespace-proxy.worker.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/workers/shared/remote-bindings-utils.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/workers/shared/remote-proxy-client.worker.ts: [@cloudflare/wrangler]

@ask-bonk

ask-bonk Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

The non-fetch RPC path (Reflect.get(stub, p) at line 108) goes through the capnweb WebSocket session which doesn't have the trace ID. This means trace ID propagation is incomplete for non-fetch binding methods (e.g., KV get, put, etc. that use RPC rather than HTTP). However, this might be intentional for a first pass or the WebSocket connection URL might not support custom headers — so it's more of a note than a blocking issue.

The code itself is correct for the fetch path. The changes are small and consistent. Let me check one more thing about the PR — whether this is labeled WIP:

The PR title says "wip trace header" and the body says "trying to get a pre-release". This is clearly a work-in-progress. The code changes are mechanically sound. There are no logic bugs, no security issues, and no backward compatibility concerns (the cfTraceId parameter is optional everywhere, and the env var binding is conditionally added).

LGTM

github run

@devin-ai-integration devin-ai-integration Bot left a comment

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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@pkg-pr-new

pkg-pr-new Bot commented Apr 23, 2026

Copy link
Copy Markdown
create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13646

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13646

miniflare

npm i https://pkg.pr.new/miniflare@13646

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13646

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13646

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13646

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13646

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13646

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@13646

wrangler

npm i https://pkg.pr.new/wrangler@13646

commit: 60ee54d

@jamesopstad jamesopstad marked this pull request as draft April 24, 2026 07:59
@jamesopstad jamesopstad removed their request for review April 24, 2026 07:59
@petebacondarwin

Copy link
Copy Markdown
Contributor

Hi @emily-shen — closing as part of stale-draft cleanup. The description indicates this was a WIP for getting a pre-release. Please reopen as a new PR if you'd like to land trace header support properly with tests/docs.

@github-project-automation github-project-automation Bot moved this from Untriaged to Done in workers-sdk Apr 28, 2026
@emily-shen emily-shen reopened this May 13, 2026
@github-project-automation github-project-automation Bot moved this from Done to Untriaged in workers-sdk May 13, 2026
@emily-shen emily-shen force-pushed the walshy/trace-id-remote branch from 1c531bf to e26d1a0 Compare May 13, 2026 09:28
@emily-shen emily-shen marked this pull request as ready for review May 13, 2026 16:19
@emily-shen

Copy link
Copy Markdown
Contributor Author

/bonk write a brief changeset for this.

@workers-devprod workers-devprod requested review from a team and ascorbic and removed request for a team May 13, 2026 16:19
@workers-devprod

workers-devprod commented May 13, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

Co-authored-by: emily-shen <emily-shen@users.noreply.github.com>
@ask-bonk

ask-bonk Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Created the changeset at .changeset/propagate-trace-id-header.md. It's a patch for miniflare describing the trace ID propagation for remote binding proxy requests.

github run

@github-actions

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@workers-devprod workers-devprod left a comment

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.

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk May 13, 2026
@petebacondarwin petebacondarwin merged commit f781a2b into main May 13, 2026
63 of 64 checks passed
@petebacondarwin petebacondarwin deleted the walshy/trace-id-remote branch May 13, 2026 20:10
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk May 13, 2026
petebacondarwin pushed a commit to timoconnellaus/workers-sdk that referenced this pull request May 13, 2026
Co-authored-by: Daniel Walsh <walshy@cloudflare.com>
Co-authored-by: ask-bonk[bot] <ask-bonk[bot]@users.noreply.github.com>
Co-authored-by: emily-shen <emily-shen@users.noreply.github.com>
penalosa pushed a commit that referenced this pull request May 28, 2026
Co-authored-by: Daniel Walsh <walshy@cloudflare.com>
Co-authored-by: ask-bonk[bot] <ask-bonk[bot]@users.noreply.github.com>
Co-authored-by: emily-shen <emily-shen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants