Skip to content
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

feat(monitors): Pass SENTRY_TRACE_ID down execution path #1441

Conversation

evanpurkhiser
Copy link
Member

@evanpurkhiser evanpurkhiser commented Jan 26, 2023

This adds SENTRY_TRACE_ID to the environ of the executing command.

This is in support of getsentry/sentry#43647
and will allow SDKs embedded in the executing command to associate the
monitor checkin to any events produced by the SDK.

This is the second part of #1438

Comment on lines 63 to 65
// Inherit outer SENTRY_TRACE_ID if present
if env::var_os("SENTRY_TRACE_ID").is_none() {
p.env("SENTRY_TRACE_ID", trace_id.to_string().replace("-", ""));
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Inherit outer SENTRY_TRACE_ID if present
if env::var_os("SENTRY_TRACE_ID").is_none() {
p.env("SENTRY_TRACE_ID", trace_id.to_string().replace("-", ""));
}
// Inherit outer SENTRY_TRACE_ID if present
let trace_id = env::var_os("SENTRY_TRACE_ID").unwrap_or_else(|| Uuid::new_v4().simple().to_string());
p.env("SENTRY_TRACE_ID", trace_id);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check if it compiles correctly, as I wrote it directly in a suggestion 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also run make lint and make format.

Comment on lines 46 to 47
let trace_id = Uuid::new_v4();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let trace_id = Uuid::new_v4();

@kamilogorek kamilogorek force-pushed the evanpurkhiser/feat-monitors-pass-sentry-trace-id-down-execution-path branch from 089b78f to b0c09df Compare January 30, 2023 08:46
@kamilogorek kamilogorek enabled auto-merge (squash) January 30, 2023 08:46
Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing a release, so will allow myself to merge that one

@kamilogorek kamilogorek merged commit de0895c into master Jan 30, 2023
@kamilogorek kamilogorek deleted the evanpurkhiser/feat-monitors-pass-sentry-trace-id-down-execution-path branch January 30, 2023 09:03
@evanpurkhiser
Copy link
Member Author

Oh wait, I wasn't ready to merge this hahaha

@kamilogorek
Copy link
Contributor

Oh, then it should be a draft 🌝 but no harm done, as it's a self-contained change and will not break anything.

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.

None yet

2 participants