-
Notifications
You must be signed in to change notification settings - Fork 377
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
Add Kafka integration #1070
Add Kafka integration #1070
Conversation
@tjwp This is awesome! Thanks for putting this together. We'll definitely setup some time to review this as this is something I'd love to add to our library. Before I get a chance to look deeper in the code, can you explain how |
@delner As with It is not a hard dependency for |
I noticed the changes here #1075 and I've updated this branch to use the new contrib spec helper. |
@tjwp Gotcha, so this integration will only work if both If this is the case, we might need to make sure that this requirement is reflected appropriately in the |
Is there a pattern for this that you can point me to for this? The code in this PR handles it the same way as the existing Racecar integration: https://github.com/DataDog/dd-trace-rb/pull/1070/files#diff-4b58bb2b027c784de6ca989f77238eb9R20-R22 |
db280b4
to
1f190b2
Compare
@marcotc thanks, rebased! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much @tjwp! 🎉
We'll let you know when a release goes out including your changes.
@marcotc Any update on when this may be included in a release? Thanks! |
Hey @tjwp, we are planning on releasing soon, tentatively next week. I'll keep you posted. Thank you once again for contribution! |
This change adds a
kafka
integration based on theruby-kafka
gem.The implementation follows the template of the existing
racecar
integration which also builds on top ofActiveSupport::Notifications
.The motivation for this change was to provide more detailed information on interactions with Kafka. We use
racecar
but for publishing we have implemented our own "outbox pattern". This new integration provides more visibility into the message delivery to Kafka.Support is included for all of the blocks that
ruby-kafka
instruments. Sorry about the size of the change :)The events are broken into internal modules based on the
ruby-kafka
class that emits them. I was originally thinking that the events could be separately enabled based on these modules, but ultimately decided not to implement that.Currently the span names are prefixed by
kafka.
, e.g.kafka.producer.deliver_messages
but I could be convinced to drop that first part of the name.CC @dasch