-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Simplifying Trace Propagation Examples; Adding origin callout. #13148
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Bundle ReportChanges will decrease total bundle size by 15 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-server-cjsAssets Changed:
view changes for bundle: sentry-docs-client-array-pushAssets Changed:
|
|
|
||
| Note: port numbers are relevant for trace propagation and the origin. You may need to configure the `tracePropagationTargets` to ensure that traces are propagated across your services if they run on different ports. | ||
|
|
||
| For example, if you have a Vite frontend running on `http://localhost:5173` and a Bun backend running on `http://localhost:3000`, both `http://localhost:5173` and `http://localhost:3000` should be added to the `tracePropagationTargets` array. |
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.
Hmm this is not correct I believe, you do not need to add your frontend as traceProapgationTargets as you'll usually not make API requests to it?
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.
Yup - you're absolutely correct, I have a fix pushed up for this. I kept with the previous examples that had localhost in them - but its not entirely useful or needed. Thanks for catching.
|
|
||
| When you are interacting with other external API systems, you might have to define `tracePropagationTargets` to get around possible [Browser CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS) issues. | ||
|
|
||
| <Alert> |
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.
It seems like this 'Alert' block is being used in several places with identical content, it's probably worth creating an include to simplify the maintenance of this content. This can totally be done in a follow-up PR if you want to merge this first.
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.
Yeah, definitely. Ill refactor it to include it short term, just to keep things clean, before merging.
coolguyzone
left a 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.
🛳️
DESCRIBE YOUR PR
Simplifying initial trace propagation example to not use a regex based example. Creates a more gradual increase in complexity through the other examples.
Added a callout that for services running on different ports on the same origin.
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.