Skip to content

Fix InstrumentedRecordInterceptor closing the trace too early#11471

Merged
trask merged 2 commits into
open-telemetry:mainfrom
Questlog:feature/295413-fix-record-interceptor
Jun 13, 2024
Merged

Fix InstrumentedRecordInterceptor closing the trace too early#11471
trask merged 2 commits into
open-telemetry:mainfrom
Questlog:feature/295413-fix-record-interceptor

Conversation

@Questlog

Copy link
Copy Markdown
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
@linux-foundation-easycla

linux-foundation-easycla Bot commented May 28, 2024

Copy link
Copy Markdown

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

laurit commented May 28, 2024

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor Author

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

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