-
Notifications
You must be signed in to change notification settings - Fork 395
Add Rails cache store instrumentation filter #4693
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4693 +/- ##
==========================================
+ Coverage 97.52% 97.53% +0.01%
==========================================
Files 1484 1484
Lines 88624 88660 +36
Branches 4598 4602 +4
==========================================
+ Hits 86431 86475 +44
+ Misses 2193 2185 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Left a suggestion from Docs and approved the PR.
BenchmarksBenchmark execution time: 2025-07-08 16:23:05 Comparing candidate commit 2516895 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 42 metrics, 6 unstable metrics. scenario:tracing - Propagation - Trace Context
|
o.type :array, nilable: true | ||
o.default nil | ||
o.after_set do |stores| | ||
stores&.map!(&:to_s) # Convert symbols to strings to match the Rails configuration format |
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.
Is the customer-supplied array dup
ed during assignment? Otherwise this would mutate their data.
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.
This does mutate customer-supplied array:
irb(main):022> stores=[:foo]
=> [:foo]
irb(main):023> Datadog.configure { |c| c.tracing.instrument :active_support, cache_store: stores }
# ...
irb(main):024> stores
=> ["foo"]
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.
Overall looks fine to me, but one suggestion for the configuration option that may be an improvement.
What does this PR do?
This PR introduces a new
:cache_store
configuration option to the ActiveSupport integration. This option allows users to specify which cache stores should be instrumented by Datadog tracing.Motivation:
This is important for very fast backends, like MemoryStore, which might not provide useful spans.
Change log entry
Yes. Add
:cache_store
option to ActiveSupport integration to allow tracing only specified cache backends.Additional Notes:
['file_store', :memory_store]
).How to test the change?
:cache_store
includes, excludes, or is unset for various backends.Datadog.configure { |c| c.tracing.instrument :active_support, cache_store: [:file_store] }
and verify that only the specified store is traced.