Skip to content

Conversation

@gregkalapos
Copy link
Contributor

Regression reported by @AlexanderWert.

Problem was missing ( ) when dependencies were calculated resulting in mapping a non-root span partly as a transaction and not setting processor.event.

Also added a test covering such cases.

@gregkalapos gregkalapos self-assigned this Apr 16, 2025
@gregkalapos gregkalapos requested a review from a team as a code owner April 16, 2025 09:41
@gregkalapos
Copy link
Contributor Author

gregkalapos commented Apr 16, 2025

Since this causes missing spans (as processor.event is not set), we should make sure this is included in 9.0.1 und 8.18.1.

@lahsivjar assuming the PR is ok, could we push a release of this repo in the near future? Or maybe @rogercoll can help with that?

// In such cases, the span is still mapped into a transaction, but enriched
// with additional attributes that are specific to the outgoing call or producer span.
isExitRootSpan := s.isTransaction && span.Kind() == ptrace.SpanKindClient || span.Kind() == ptrace.SpanKindProducer
isExitRootSpan := s.isTransaction && (span.Kind() == ptrace.SpanKindClient || span.Kind() == ptrace.SpanKindProducer)
Copy link
Contributor

@lahsivjar lahsivjar Apr 16, 2025

Choose a reason for hiding this comment

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

Thanks a lot for finding and fixing this.

@lahsivjar
Copy link
Contributor

@lahsivjar assuming the PR is ok, could we push a release of this repo in the near future?

I can cut a new release, the dependabot should do the rest.

@gregkalapos gregkalapos merged commit 426c75b into elastic:main Apr 16, 2025
3 checks passed
@gregkalapos gregkalapos deleted the fix_non_root_dependency_span branch April 16, 2025 10:25
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.

2 participants