Skip to content

Add sidekiq config propagate_traces to control trace header injection#2588

Merged
sl0thentr0py merged 1 commit intomasterfrom
neel/sidekiq-propagate-traces
Apr 9, 2025
Merged

Add sidekiq config propagate_traces to control trace header injection#2588
sl0thentr0py merged 1 commit intomasterfrom
neel/sidekiq-propagate-traces

Conversation

@sl0thentr0py
Copy link
Copy Markdown
Member

closes #2531
part of #2476

@sl0thentr0py sl0thentr0py force-pushed the neel/sidekiq-propagate-traces branch from c38d916 to 02a5e26 Compare March 19, 2025 13:57
@sl0thentr0py sl0thentr0py requested a review from solnic March 19, 2025 13:58
@sl0thentr0py sl0thentr0py changed the title Add sidekiq config propagate_traces to control trace header injection Add sidekiq config propagate_traces to control trace header injection Mar 19, 2025
it "has correct default value" do
expect(subject.report_only_dead_jobs).to eq(false)
end
end
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems unrelated?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

was missing so just added it

Copy link
Copy Markdown
Collaborator

@solnic solnic left a comment

Choose a reason for hiding this comment

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

Left a question


if Sentry.configuration.sidekiq.propagate_traces
job["trace_propagation_headers"] ||= Sentry.get_trace_propagation_headers
end
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn't this a breaking change in a way? Previously it would do it unconditionally. Now you have to explicitly enable it. I understand that the original behavior was problematic though so maybe it's fine?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it defaults to true

@sl0thentr0py sl0thentr0py requested a review from solnic March 20, 2025 12:31
@sl0thentr0py sl0thentr0py merged commit 6b15516 into master Apr 9, 2025
135 checks passed
@sl0thentr0py sl0thentr0py deleted the neel/sidekiq-propagate-traces branch April 9, 2025 06:23
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.

Unrelated Sidekiq job runs being grouped in same trace

3 participants