-
Notifications
You must be signed in to change notification settings - Fork 398
DSM Implementation for Ruby with Karafka instrumented #4901
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
Conversation
2b9b824 to
cb04944
Compare
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: 728b9c7 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
2738b51 to
29cf40f
Compare
3a25253 to
072e090
Compare
Typing analysisIgnored filesThis PR introduces 1 ignored file. It increases the percentage of typed files from 35.94% to 36.87% (+0.93%). Ignored files (+1-0)❌ Introduced:Note: Ignored files are excluded from the next sections. Untyped methodsThis PR introduces 7 untyped methods and 8 partially typed methods. It increases the percentage of typed methods from 51.66% to 53.09% (+1.43%). Untyped methods (+7-0)❌ Introduced:Partially typed methods (+8-0)❌ Introduced:If you believe a method or an attribute is rightfully untyped or partially typed, you can add |
37705c0 to
7ab70ad
Compare
BenchmarksBenchmark execution time: 2025-11-03 18:46:49 Comparing candidate commit 2ae6bf8 in PR branch Found 2 performance improvements and 1 performance regressions! Performance is the same for 41 metrics, 2 unstable metrics. scenario:profiling - Allocations (baseline)
scenario:tracing - Propagation - Datadog
scenario:tracing - Propagation - Trace Context
|
77dfd10 to
8242233
Compare
|
Thank you for updating Change log entry section 👏 Visited at: 2025-10-23 12:59:15 UTC |
0d9c1ce to
b1f669a
Compare
efd426f to
988cd35
Compare
|
Removed a ton of worthless comments and de-namespaced it from tracing. It is ready for re-review @y9v |
0de7f3c to
5abf907
Compare
lib/datadog/tracing/contrib/kafka/events/consumer/process_message.rb
Outdated
Show resolved
Hide resolved
- Move Data Streams configuration to use self.extended hook pattern - Add Extensions module to activate Data Streams configuration - Move ENV_ENABLED constant to lib/datadog/data_streams/ext.rb - Add comprehensive RBS type signatures for Data Streams modules - Fix type errors: add missing timestamp field to consumer stats - Improve type coverage to 88-100% across data_streams files - Add configuration settings tests This addresses review comments to align with the existing AppSec configuration pattern and improve type safety.
Update set_produce_checkpoint and set_consume_checkpoint methods to accept
tags as Hash instead of Array<String>, aligning with the common pattern used
throughout the Datadog library.
Tags are now passed as { key: 'value' } and internally converted to the
'key:value' string format used by the DSM pathway hash computation.
This change addresses @marcotc's review feedback.
The processor method is an internal implementation detail and should not be part of the public API. Users should interact with DataStreams through the public methods (set_produce_checkpoint, set_consume_checkpoint, etc.). - Move processor method to private section - Update tests to use .send(:processor) for internal access - Update RBS to mark processor as private This addresses @marcotc's review feedback.
Add comprehensive documentation to encode_var_int_64 and decode_varint methods explaining that they implement unsigned LEB128 (Little Endian Base 128) encoding as specified in DWARF5 standard section 7.6. This addresses @marcotc's review feedback.
…ta structures These methods were writing to @produce_offsets and @commit_offsets instance variables, but serialize_buckets was reading from bucket[:latest_produce_offsets] and bucket[:latest_commit_offsets]. This meant offset data would never be sent to the agent. Fixed by writing directly to the bucket structure like track_kafka_consume does. Related to @marcotc's review feedback about unused methods.
8c47ea9 to
6d9d801
Compare
6d9d801 to
182ce81
Compare
vpellan
left a comment
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.
LGTM for tracing part! (non blocker comment)
- Fix hook from 'included' to 'prepended' in Producer/Consumer instrumentation - Add missing env.verb in Stats::API::Endpoint#call - Update tests to use prepend instead of include - Fix async buffer test to call process_events - Remove track_kafka_commit tests (method was removed)
fad63ac to
728b9c7
Compare
|
I have the follow-up PR to improve ddsketch loading: #5008. I think it should stay as a separate PR because I would like the guild to review it since it is a bit of a conceptual change in how loading/referencing of components from libdatadog is accomplished. This present PR does not add any circular dependencies as far as I can tell, I think it's OK to be merged in its present state. |
What does this PR do?
This implements Data Streams Monitoring in Ruby. It autoinstruments 2 libraries:
Data Streams Monitoring works by setting a checkpoint with
topic(i.e. queue),direction(i.e. produce/consume = in/out) andservice(plus a few other tags). Produces then set the context of the checkpoint in the message (usually via the headers, but for SQS and non-header libraries, it will be in the message body) and consumers take the message context and set a checkpoint with a link to the previous checkpoint. Thus, we can give latency stats (i.e. non-sampled) through queues and long pipelines.Motivation:
There are have been many requests for DSM in Ruby over the years and there is an issue on the repo to add it.
Change log entry
Yes. DataStreams: Adds Data Streams Monitoring capability, instrumenting ruby-kafka and karafka consumers with manual instrumentation available
Additional Notes:
How to test the change?
The following branch is being tested on staging with our test apps. You can see them here
They are defined here:
https://github.com/DataDog/data-streams-dev/pull/252