dd-trace-rb
dd-trace-rb copied to clipboard
Add `cache_key_enabled` option for ActiveSupport
Change log entry
Adds a new option for ActiveSupport to disable adding the cache_key as a Span Tag with the cache_key_enabled option.
What does this PR do?
- Adds
cache_key_enabledtoActiveSupport(defaults to true) - When
cache_key_enabledisfalseit will no longer set thecache_keyas a Span Tag
Motivation:
Customer feature request TODO GITHUB ISSUE LINK HERE
Additional Notes:
I have basic grasp on Ruby.
An alternative here is to obfuscate/quantize cache_key values when we add them, the issue here is I'm not entirely sure what these values would look like and if they'd be valuable if they were obfuscated/quantized.
How to test the change?
On Windows can't install Ruby :( Can't get WSL to work after Windows Update Can't find tests for ActiveSupport that assert on tags Help
Unsure? Have a question? Request a review!
Benchmarks
Benchmark execution time: 2024-11-29 20:06:00
Comparing candidate commit 3c24e29ee9b71414430954ebbf94b0e6c7c4d1c5 in PR branch steven/activesupport-cachekey-config with baseline commit 28675b6daa898731bc1c9824421b11a539fbde12 in branch master.
Found 0 performance improvements and 1 performance regressions! Performance is the same for 30 metrics, 2 unstable metrics.
scenario:profiler - sample timeline=false
- 🟥
throughput[-0.479op/s; -0.469op/s] or [-7.318%; -7.164%]
(I guess this is related to https://github.com/DataDog/dd-trace-rb/issues/4017 ?)
(I guess this is related to #4017 ?)
Yes it is @ivoanjo! I posted in the slack channel some context surrounding this
I didn't link to it yet as this was just a draft and was waiting to determine whether or not this is something acceptable to do within the Ruby Datadog SDK or if the expected solution would be to add obfuscation rules to this.
Datadog Report
Branch report: steven/activesupport-cachekey-config
Commit report: 3c24e29
Test service: dd-trace-rb
:white_check_mark: 0 Failed, 22050 Passed, 1461 Skipped, 5m 23.17s Total Time
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 97.75%. Comparing base (
28675b6) to head (3c24e29).
Additional details and impacted files
@@ Coverage Diff @@
## master #4022 +/- ##
==========================================
- Coverage 97.76% 97.75% -0.01%
==========================================
Files 1351 1351
Lines 81733 81845 +112
Branches 4147 4150 +3
==========================================
+ Hits 79904 80009 +105
- Misses 1829 1836 +7
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hey @bouwkast thoughts on merging this in? Looks like a nice improvement
Hey @bouwkast thoughts on merging this in? Looks like a nice improvement
I was out on PTO, looked like I just had a couple of conversations that needed to be resolved for it to be merged, so did that
No worries! I was doing a big clean-up and leaving a similar comment on PRs I saw in good shape :D