dd-trace-rb
dd-trace-rb copied to clipboard
Move `_dd.apm.enabled` tag in tracing root span
What does this PR do?
- It moves the addition of
_dd.apm.enabledtag to spans in tracing root spans. - It removes most references to AppSec standalone and replace it by "non_billing" mode (as "APM tracing disabled" can be confusing because this does not completely disables tracing, and we already have a way to completely disable tracing)
It does NOT change the env var DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED to a new one (as the discussion is still ongoing and most likely will stay continue until next quarter)
Motivation:
During the review of Standalone SCA system-tests, I found out that enabling 'Standalone Appsec Billing' (DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED=true) and disabling 'Appsec Threats' ( DD_APPSEC_ENABLED=false ) would make the tests fails as _dd.apm.enabled is set in appsec Rack middleware.
This is not correct as this tag is suppose to communicate to the Agent that we should not bill for APM traces, and Standalone Appsec Billing will still limit the number of traces to 1 per minute to keep services alive on the backend side even if Appsec Threats is not enabled, thus the traces must contain that tag. But appsec can have a lot of different service as root span, so we must add it in other integrations too.
Change log entry
None.
How to test the change?
Standalone SCA system-tests (./run.sh SCA_STANDALONE) should XPASS (or pass if force executed)
Benchmarks
Benchmark execution time: 2024-12-10 15:36:54
Comparing candidate commit eeffb42656b81a2e3f69cb94bbb56884b1f6fc37 in PR branch vpellan/move-dd-apm-enabled-tag with baseline commit c07a8d4cac55ddcea42341787ab5753f910dc162 in branch master.
Found 1 performance improvements and 0 performance regressions! Performance is the same for 30 metrics, 2 unstable metrics.
scenario:profiler - sample timeline=false
- 🟩
throughput[+0.547op/s; +0.562op/s] or [+9.088%; +9.344%]
Codecov Report
Attention: Patch coverage is 98.93048% with 2 lines in your changes missing coverage. Please review.
Project coverage is 97.78%. Comparing base (
c07a8d4) to head (fd7b84c). Report is 13 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| lib/datadog/tracing/configuration/settings.rb | 84.61% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #4141 +/- ##
==========================================
+ Coverage 97.77% 97.78% +0.01%
==========================================
Files 1357 1355 -2
Lines 81973 82015 +42
Branches 4168 4173 +5
==========================================
+ Hits 80145 80195 +50
+ Misses 1828 1820 -8
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
As of 27th. of November, the scope of this PR has been changed to moving all "non-billing" mode code to Tracing part. ("Non billing" mode is what was previously referred as "ASM Standalone" mode, that is now referred as "APM Tracing disabled" mode, but since we have already have a different way to completely disable tracing, I refer to it as non-billing mode in this PR to differentiate both, but please note that this is not the official naming for this feature) This is still a WIP (specs not updated) so I changed this PR back to Draft
Datadog Report
Branch report: vpellan/move-dd-apm-enabled-tag
Commit report: eeffb42
Test service: dd-trace-rb
:white_check_mark: 0 Failed, 22057 Passed, 1458 Skipped, 6m 10.58s Total Time :hourglass: 1 Performance Regression
:hourglass: Performance Regressions vs Default Branch (1)
loading of products datadog/di loads successfully by itself-rspec14.75s (+13.48s, +1062%) - Details
:wave: Hey @DataDog/ruby-guild, please fill "Change log entry" section in the pull request description.
If changes need to be present in CHANGELOG.md you can state it this way
**Change log entry**
Yes. A brief summary to be placed into the CHANGELOG.md
(possible answers Yes/Yep/Yeah)
Or you can opt out like that
**Change log entry**
None.
(possible answers No/Nope/None)
Visited at: 2024-12-10 15:07:36 UTC