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 Exception on using OpenTelemetry and ScatterGather queries #2960

Merged
merged 1 commit into from Jan 19, 2024

Conversation

CodeDrivenMitch
Copy link
Member

The onClose handler on ScatterGather queries calls the end operation on Spans a second time. This, however, has been restricted by an exception as this can indicate thread leaks.

However, calling the Span.end on both close and finishing of advancing the stream is necessary, as users might not always close a stream explicitly. Likewise, the stream may be closed without reading it completely.

I have added an AtomicBoolean as close gate on the closeHandler, so it's always called exactly once.

The TestSpanFactory has been extended with the same behavior as the OpenTelemetrySpan to catch any other instances of this happening during the tests

@CodeDrivenMitch CodeDrivenMitch added Type: Bug Use to signal issues that describe a bug within the system. Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. Status: In Progress Use to signal this issue is actively worked on. labels Jan 18, 2024
@CodeDrivenMitch CodeDrivenMitch added this to the Release 4.9.2 milestone Jan 18, 2024
@CodeDrivenMitch CodeDrivenMitch requested a review from a team January 18, 2024 15:36
@CodeDrivenMitch CodeDrivenMitch self-assigned this Jan 18, 2024
@CodeDrivenMitch CodeDrivenMitch requested review from gklijs and smcvb and removed request for a team January 18, 2024 15:36
The onClose handler on ScatterGather queries calls the `end` operation on Spans a second time. This, however, has been restricted by an exception as this can indicate thread leaks.

However, calling the `Span.end` on both `close` and finishing of advancing the stream is necessary, as users might not always close a stream explicitly. Likewise, the stream may be closed without reading it completely.

I have added an AtomicBoolean as close gate on the closeHandler, so it's always called exactly once.

The TestSpanFactory has been extended with the same behavior as the OpenTelemetrySpan to catch any other instances of this happening during the tests
@CodeDrivenMitch CodeDrivenMitch force-pushed the fix/otel-subscription-query-error branch from 6064171 to fd656a7 Compare January 18, 2024 15:43
Copy link
Member

@smcvb smcvb 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 to me 👍

@smcvb smcvb merged commit 2beb5da into master Jan 19, 2024
5 of 7 checks passed
@smcvb smcvb deleted the fix/otel-subscription-query-error branch January 19, 2024 09:56
@smcvb smcvb added Status: Resolved Use to signal that work on this issue is done. and removed Status: In Progress Use to signal this issue is actively worked on. labels Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. Status: Resolved Use to signal that work on this issue is done. Type: Bug Use to signal issues that describe a bug within the system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants