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 InstrumentedRecordInterceptor closing the trace too early #11471

Merged

Conversation

Questlog
Copy link
Contributor

The Spring Kafka InstrumentedRecordInterceptor was closing the trace before calling the decorated RecordInterceptor.

We print logs and do some other tasks inside the Kafka RecordInterceptor, especially if a failure occured after consuming an event.

This caused our logs not to be part of the trace when the kafka event got finished successfully or with failures.

@Questlog Questlog requested a review from a team May 28, 2024 08:17
Copy link

linux-foundation-easycla bot commented May 28, 2024

CLA Signed


The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot added the test native This label can be applied to PRs to trigger them to run native tests label May 28, 2024
…ll decorated RecordInterceptor before closing the trace.
@Questlog Questlog force-pushed the feature/295413-fix-record-interceptor branch from c99d9f4 to 9b15525 Compare May 28, 2024 08:23
@laurit
Copy link
Contributor

laurit commented May 28, 2024

@Questlog could you sign the CLA, we can not accept the PR without that.
When you call the original interceptor before ending the span you may need to consider the possibility of the original interceptor throwing an exception. Could you make the same change in https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/spring/spring-kafka-2.7/library/src/main/java/io/opentelemetry/instrumentation/spring/kafka/v2_7/InstrumentedBatchInterceptor.java

…mentedRecordInterceptor.

Close trace in case of exceptions.
@Questlog
Copy link
Contributor Author

To sign the CLA, I have to get in contact with my companys CLA Manager to sort this out.

@laurit laurit added this to the v2.5.0 milestone Jun 13, 2024
@trask trask merged commit f602e19 into open-telemetry:main Jun 13, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test native This label can be applied to PRs to trigger them to run native tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants