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

Integration with Opentelemetry standard. #1025

Open
maksim77 opened this issue Nov 6, 2022 · 13 comments
Open

Integration with Opentelemetry standard. #1025

maksim77 opened this issue Nov 6, 2022 · 13 comments

Comments

@maksim77
Copy link
Contributor

maksim77 commented Nov 6, 2022

I think it would be great if the library supported integration with the Opentelemetry standard.

Obvious places for such integration seem to be:

  • Inserting a traceparent header into the message at the time of sending. Inheriting it from the context passed to the sending methods.
  • Reading this header from messages read from the topic.
  • Maybe this logic should be applied for the rest of the requests sent through the Connection object.

For competing solutions, such integration is possible. And it would be great if it was supported in kafka-go.

@achille-roussel
Copy link
Contributor

Hello @maksim77, thanks for starting this discussion!

I agree with you, this would be a valuable addition.

The OpenTelemetry SDK for Go has a lot of dependencies that I don't think we should impose on users of kafka-go (many of which may not need it). There may also be cases where forcing the injection of the message header is not desirable.

I would like us to find a way to make this integration opt-in, would you have suggestions on how to achieve this?

@maksim77
Copy link
Contributor Author

maksim77 commented Nov 7, 2022

Yes, of course, bringing all opentelemetry dependencies to the library is a very strange solution.

For example, the following solution comes to my mind:
We have a Logger object that is called in certain places. It may be worth considering a similar Tracer object that simply defined the interface and was called (if specified) at the main points of the message path lifecycle. Its interface-declared method will receive a typed message from kafka-go telling exactly what happened.
Thus, it will be possible to integrate not only with the modern OpenTelemetry standard, but also potentially with any other tracing mechanism.

@straysh
Copy link

straysh commented Dec 22, 2022

@PatrikSteuer
Copy link

It would also make sense to comply with the cloud (events specification)[https://github.com/cloudevents/spec#cloudevents] for the headers (traceparent/tracestate) used to exchange context information.

@rafaribe
Copy link

This would be very valuable. I did a "kind-of-works" solution for one of my projects by adapting one of the other libraries but it's a very dirty hacky solution to say the least. I would say this should be optional and put it it's own repo. Given the Standard status of OpenTelemetry and the integration with the existing Cloud Native tooling I would dare to say this should be one of the most sought after features of this simply amazing Kafka Library. 🙏

@achille-roussel
Copy link
Contributor

Hello @rafaribe, thanks for chiming in!

Since you mentioned that you already built something somewhat similar you probably have relevant experience to contribute here!

Do you think you would have cycles available to submit a PR adding an OpenTelemetry integration for kafka-go?

@phillip055
Copy link

Hey, is this being worked on?

@rafaribe
Copy link

rafaribe commented Mar 27, 2023

Hey @achille-roussel I've built some code in a separate repository. It only works for traces though.

The repo itself only contains code (no readme, no tests) and was done in a way that it worked for my use-case, would need to be adapted and polished to be integrated for sure.

I took some inspiration from an already existing wrapper for Sarama. It works well so far to propagate context, but I'm unsure how you would like to handle that in the scope of the kafka-go project.

If a separate library that wraps kafka-go or integrate it within your library and give the users the option to use OpenTelemetry instrumentation or not.

@Abdulsametileri
Copy link

Is there any update here? @achille-roussel

@achille-roussel
Copy link
Contributor

Hello @Abdulsametileri, this feature seems helpful, but unfortunately I am not personally involved with kafka-go anymore.

If you have cycles to contribute, I am sure someone in the maintainer team would be happy to merge it. Maybe @rhansen2 can help?

@Abdulsametileri
Copy link

Thank you @achille-roussel

Based on @rhansen2 repository, I little bit refactored and added examples (jaeger and stdout) in this repo

I will integrate into our kafka-go wrapper library kafka-konsumer asap.

After that, maybe we can open a PR about that:)

@abh
Copy link

abh commented Oct 21, 2023

The OpenTelemetry SDK for Go has a lot of dependencies that I don't think we should impose on users of kafka-go (many of which may not need it).

In Go versions from the last few years, isn't Go pretty good about only imposing the cost of the dependencies when they are used? The module cache in the background, only compiling code that's needed, etc?

Many other similar packages have put in hooks so a tracing package can be plugged in.

@lazyboson
Copy link

lazyboson commented Jan 23, 2024

Hey @rafaribe Do you have any example, i have tried using ur library, but the context object passed at the time of WriteMessages is not used in generating spancontext and when I read on kafka (in other service), i don't have option of getting previous ctx post propagation. so i am able to get it right. I was just wandering if you could just provide some example to test, as library support is unavailable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants