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

Populate ActiveSupport Notification spans as early as possible #3725

Merged
merged 4 commits into from
Jun 26, 2024

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Jun 18, 2024

Fixes #2846

This PR populates spans create it from ActiveSupport Notification spans as early as possible.

Before this PR, a span was started when the ActiveSupport Notification event started but the notification callback was only called when the event ended. This meant that during the event execution, the active span would not have all its fields populated.

This causes issues with children span's service entry detection and service name propagation.

How to test the change?
All changes are covered by tests.

@marcotc marcotc self-assigned this Jun 18, 2024
@marcotc marcotc requested a review from a team as a code owner June 18, 2024 23:10
@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Jun 18, 2024
@marcotc marcotc requested a review from TonyCTHsu June 19, 2024 20:34
@ivoanjo
Copy link
Member

ivoanjo commented Jun 20, 2024

❤️ Adding info to spans as early as possible also opens up possibilities for the profiler to do more with this data so thanks for all this work :)

@marcotc marcotc enabled auto-merge June 26, 2024 19:39
@marcotc marcotc merged commit a13cb0f into master Jun 26, 2024
141 of 144 checks passed
@marcotc marcotc deleted the as-events branch June 26, 2024 19:51
@github-actions github-actions bot added this to the 2.2.0 milestone Jun 26, 2024
@TonyCTHsu TonyCTHsu mentioned this pull request Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActiveSupport events cause child spans to be mislabeled as service-entry
3 participants