Add support for Kafka consumer and producer interceptors. #4065

alesj · 2021-09-08T12:18:55Z

Sending a PR after a short discussion #4061

The goal is to have an equivalent interceptors implementation to this one:

https://github.com/opentracing-contrib/java-kafka-client/tree/master/opentracing-kafka-client/src/main/java/io/opentracing/contrib/kafka

The main stuff is in TracingKafkaUtils class.

I'm not 100% I got all of the things, specially scopes and contexts right,
so a more experienced eye is welcome.

Plus some suggestions on what to check in the test(s) -- currently I just print out headers.

linux-foundation-easycla · 2021-09-08T12:18:58Z

The committers are authorized under a signed CLA.

✅ Aleš Justin (fea4a6a)

alesj · 2021-09-08T12:27:43Z

This EasyCLA doesn't work ... it's generating forever ...

trask · 2021-09-08T23:50:45Z

This EasyCLA doesn't work ... it's generating forever ...

hey @alesj, give it another try tomorrow, hopefully it was just having temporary problems, if not, post a screenshot and we can reach out to the EasyCLA team

alesj · 2021-09-10T07:46:58Z

@trask OK, it works now ... signed

alesj · 2021-09-13T12:03:07Z

Uh, any (better) idea what went wrong here?
As I fail to find anything relevant to my PR ...

laurit · 2021-09-13T12:15:46Z

@alesj try running ./gradlew :instrumentation:kafka-clients:kafka-clients-0.11:library:checkstyleMain

alesj · 2021-09-13T12:45:05Z

@laurit ah, OK, tnx ... fixed now

trask · 2021-09-13T16:45:55Z

@alesj latest failure is just formatting. running ./gradlew spotlessApply locally should fix it up.

mateuszrzeszutek

Hey @alesj ,
Please take a look at the our Instrumenter API (posted a link to the class and some examples in one comment) - in general we want all our instrumentations to use it to generate telemetry (although there are still some that we haven't refactored yet).

...nts-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafka/TracingKafkaUtils.java

...-0.11/library/src/test/groovy/io/opentelemetry/instrumentation/kafka/InterceptorsTest.groovy

....11/library/src/main/java/io/opentelemetry/instrumentation/kafka/ClientSpanNameProvider.java

alesj · 2021-09-15T09:48:29Z

@mateuszrzeszutek OK, moved my stuff to Instrumenter API + moved the common code to library ...

alesj · 2021-09-15T09:49:22Z

Uh merge conflicts ... fixing ...

instrumentation/kafka-clients/kafka-clients-0.11/library/build.gradle.kts

...brary/src/main/java/io/opentelemetry/instrumentation/kafkaclients/StandardSpanDecorator.java

...-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/SpanDecorator.java

.../src/main/java/io/opentelemetry/instrumentation/kafkaclients/TracingConsumerInterceptor.java

...1/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/TracingKafkaUtils.java

...-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/KafkaHeadersGetter.java

...nts-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/KafkaSingletons.java

alesj · 2021-09-17T19:07:41Z

@mateuszrzeszutek @trask any idea how to limit logging in the test?
Since that log4j.properties doesn't help ...

...t/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaSingletons.java

...s-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracing.java

...library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracingBuilder.java

instrumentation/kafka-clients/kafka-clients-0.11/library/src/test/resources/log4j.properties

...ain/java/io/opentelemetry/javaagent/instrumentation/kafkastreams/KafkaStreamsSingletons.java

...s-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracing.java

.../src/main/java/io/opentelemetry/instrumentation/kafkaclients/TracingConsumerInterceptor.java

...-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/KafkaUtils.java

settings.gradle.kts

...n/groovy/io/opentelemetry/instrumentation/kafkaclients/KafkaClientPropagationBaseTest.groovy

.../library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracingHolder.java

...library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracingBuilder.java

...s-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracing.java

… Kafka code to library module.

alesj · 2021-09-29T15:55:56Z

@mateuszrzeszutek @trask i've added producer / consumer tracing wrappers to this PR ... let me know what you think

...s-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracing.java

.../library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracingHolder.java

.../src/main/java/io/opentelemetry/instrumentation/kafkaclients/TracingConsumerInterceptor.java

instrumentation/kafka-clients/kafka-clients-0.11/library/build.gradle.kts

...library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracingBuilder.java

...2.6/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/TracingConsumer.java

...2.6/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/TracingProducer.java

...library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracingBuilder.java

...ts-2.6/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracing.java

trask · 2021-09-30T18:02:48Z

thanks @alesj! can you go through and mark conversations resolved where applicable, and make a list of any open questions that were raised either by reviewers or by yourself? then we'll go ahead and merge and we can create a tracking issue for the open questions / follow-ups (if any) that anyone can take

alesj · 2021-09-30T20:03:43Z

OK, all conversations resolved.
Atm I cannot think of any open questions -- imho, the docs are in-place for interceptors, wrappers as well, and we established that config(Map) doesn't work for a more custom OpenTelemetry instance.

trask

thanks @alesj, kafka library instrumentation is a great addition to the project! and thanks for your patience with the reviews and merge conflicts! I will merge shortly 🎉

mateuszrzeszutek · 2021-10-04T08:09:01Z

Thanks @alesj! 🚀

rahja · 2021-10-06T04:21:17Z

Hi folks, we were looking to add instrumentation for our kafka apps and not able to find any documentation how to do that using opentelemetry-java-instrumentation javaagent. I stumbled upon this commit, which looks like came jit for us :). Can you guys help with how do we use this feature. We have all of kafka consumer, producer and streaming clients in our code and need to instrument all of these.

trask · 2021-10-06T05:10:26Z

hey @rahja!

documentation how to do that using opentelemetry-java-instrumentation javaagent

if you want to use the javaagent, you can follow the getting started instructions and it should automatically instrument kafka for you.

this PR is about using kafka instrumentation without the javaagent

rahja · 2021-10-06T08:29:49Z

Hi @trask, thanks for your response. We did follow getting started document and started getting all the traces, though producer and consumer traces don't get connected with each other. Link between them is getting broken somewhere, looks like parent trace-ids are not being sent from producer to consumer. Any suggestions on how to make that work? it will be very helpful if you can point me to right support channel or document for this. Appreciate you response.

mateuszrzeszutek · 2021-10-06T08:54:58Z

Hey @rahja ,
Can you create a new issue wit the details of your setup? E.g. what kafka-related libraries you're using, what versions, etc.

alesj requested review from anuraaga, iNikem, jkwatson, laurit, mateuszrzeszutek, pavolloffay, trask and tylerbenson as code owners September 8, 2021 12:18

alesj force-pushed the kafka_lib_1 branch from fea4a6a to 32042f1 Compare September 13, 2021 12:43

alesj force-pushed the kafka_lib_1 branch 2 times, most recently from c65ec26 to 08f72be Compare September 13, 2021 14:03

mateuszrzeszutek reviewed Sep 13, 2021

View reviewed changes

...nts-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafka/TracingKafkaUtils.java Outdated Show resolved Hide resolved

...-0.11/library/src/test/groovy/io/opentelemetry/instrumentation/kafka/InterceptorsTest.groovy Outdated Show resolved Hide resolved

alesj force-pushed the kafka_lib_1 branch from 08f72be to 0b2e03f Compare September 14, 2021 07:41

trask reviewed Sep 15, 2021

View reviewed changes

....11/library/src/main/java/io/opentelemetry/instrumentation/kafka/ClientSpanNameProvider.java Outdated Show resolved Hide resolved

alesj force-pushed the kafka_lib_1 branch from 0b2e03f to f140564 Compare September 15, 2021 09:46

alesj force-pushed the kafka_lib_1 branch 2 times, most recently from 49c2070 to 170837c Compare September 15, 2021 22:29

mateuszrzeszutek reviewed Sep 16, 2021

View reviewed changes

alesj force-pushed the kafka_lib_1 branch from 170837c to 3d5843a Compare September 17, 2021 12:42

trask reviewed Sep 27, 2021

View reviewed changes

alesj force-pushed the kafka_lib_1 branch 2 times, most recently from 40dcf49 to 78c75b7 Compare September 28, 2021 12:02

mateuszrzeszutek reviewed Sep 28, 2021

View reviewed changes

alesj force-pushed the kafka_lib_1 branch 2 times, most recently from 4b56eb2 to f3574a4 Compare September 29, 2021 13:02

alesj added 5 commits September 29, 2021 17:55

Add support for Kafka consumer and producer interceptors, move common…

b2b32df

… Kafka code to library module.

Apply feedback

673aca0

Apply feedback, open-telemetry#3.

13b9635

Apply feedback, open-telemetry#4.

d2cf033

Add producer / consumer wrappers.

c697ba6

alesj force-pushed the kafka_lib_1 branch from 21081a0 to c697ba6 Compare September 29, 2021 15:55

mateuszrzeszutek reviewed Sep 29, 2021

View reviewed changes

...s-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/KafkaTracing.java Outdated Show resolved Hide resolved

trask reviewed Sep 29, 2021

View reviewed changes

mateuszrzeszutek reviewed Sep 30, 2021

View reviewed changes

Move to kafka-clients-2.6.

0912ff3

alesj force-pushed the kafka_lib_1 branch from cfc9613 to 0912ff3 Compare September 30, 2021 11:19

Apply feedback open-telemetry#5.

bd8a5d5

mateuszrzeszutek mentioned this pull request Sep 30, 2021

Move ExperimentalConfig to javaagent-instrumentation-api #4255

Closed

Merge remote-tracking branch 'upstream/main' into prprkafka_lib_1

1d96d04

trask approved these changes Oct 2, 2021

View reviewed changes

trask merged commit ff0bf0a into open-telemetry:main Oct 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Kafka consumer and producer interceptors. #4065

Add support for Kafka consumer and producer interceptors. #4065

alesj commented Sep 8, 2021

linux-foundation-easycla bot commented Sep 8, 2021 •

edited

Loading

alesj commented Sep 8, 2021

trask commented Sep 8, 2021

alesj commented Sep 10, 2021 •

edited

Loading

alesj commented Sep 13, 2021

laurit commented Sep 13, 2021

alesj commented Sep 13, 2021

trask commented Sep 13, 2021

mateuszrzeszutek left a comment

alesj commented Sep 15, 2021

alesj commented Sep 15, 2021

alesj commented Sep 17, 2021

alesj commented Sep 29, 2021

trask commented Sep 30, 2021

alesj commented Sep 30, 2021

trask left a comment

mateuszrzeszutek commented Oct 4, 2021

rahja commented Oct 6, 2021

trask commented Oct 6, 2021

rahja commented Oct 6, 2021

mateuszrzeszutek commented Oct 6, 2021

Add support for Kafka consumer and producer interceptors. #4065

Add support for Kafka consumer and producer interceptors. #4065

Conversation

alesj commented Sep 8, 2021

linux-foundation-easycla bot commented Sep 8, 2021 • edited Loading

alesj commented Sep 8, 2021

trask commented Sep 8, 2021

alesj commented Sep 10, 2021 • edited Loading

alesj commented Sep 13, 2021

laurit commented Sep 13, 2021

alesj commented Sep 13, 2021

trask commented Sep 13, 2021

mateuszrzeszutek left a comment

Choose a reason for hiding this comment

alesj commented Sep 15, 2021

alesj commented Sep 15, 2021

alesj commented Sep 17, 2021

alesj commented Sep 29, 2021

trask commented Sep 30, 2021

alesj commented Sep 30, 2021

trask left a comment

Choose a reason for hiding this comment

mateuszrzeszutek commented Oct 4, 2021

rahja commented Oct 6, 2021

trask commented Oct 6, 2021

rahja commented Oct 6, 2021

mateuszrzeszutek commented Oct 6, 2021

linux-foundation-easycla bot commented Sep 8, 2021 •

edited

Loading

alesj commented Sep 10, 2021 •

edited

Loading