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

fix: broken traces #467

Closed
wants to merge 1 commit into from
Closed

fix: broken traces #467

wants to merge 1 commit into from

Conversation

simaoribeiro
Copy link
Contributor

@simaoribeiro simaoribeiro commented Nov 9, 2023

Description

Fixes broken traces by changing the Activity creation to KafkaFlow project, just before the messages are consumed and produced.

The error appeared because the Activity context, being created in KafkaFlow.OpenTelemetry, could not be propagated through the system and the next time a span would be created the existing Activity would be null, forcing the creation of a new trace instead of using the previous one

Closes #468

How Has This Been Tested?

Integration tests

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have added tests to cover my changes
  • I have made corresponding changes to the documentation

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement

@simaoribeiro simaoribeiro force-pushed the fix/-broken-traces branch 4 times, most recently from 29ce080 to 9eaa029 Compare November 13, 2023 14:41
@simaoribeiro simaoribeiro changed the title Fix broken traces fix: broken traces Nov 13, 2023
@simaoribeiro simaoribeiro marked this pull request as ready for review November 13, 2023 14:46
@simaoribeiro simaoribeiro force-pushed the fix/-broken-traces branch 2 times, most recently from d11e439 to d6d3f7e Compare November 13, 2023 15:08
Copy link
Contributor

@brmagadutra brmagadutra left a comment

Choose a reason for hiding this comment

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

Is it possible to have some tests to ensure that the traces don't break?

Copy link
Contributor

@joelfoliveira joelfoliveira left a comment

Choose a reason for hiding this comment

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

Looks good. Just added some minor comments

@simaoribeiro
Copy link
Contributor Author

Is it possible to have some tests to ensure that the traces don't break?

@brmagadutra 3e926e7

Activity.Current is a static variable that uses AsyncLocal internally,
which means that it flows into async calls,
but not back out into the caller (message consumer and producer).

As so, the activity.current is null after the produce and consume of the
message, which means that all the spans created after it will be
generate a new trace because the context is not being propagated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report]: KafkaFlow OTEL trace is breaking after message consumption
4 participants