Skip to content

feat(tracing): allow tracing to be disabled remotely (via Datadog UI)#8080

Merged
emmettbutler merged 34 commits intomainfrom
emmett.butler/remotely-disable-tracing
Feb 1, 2024
Merged

feat(tracing): allow tracing to be disabled remotely (via Datadog UI)#8080
emmettbutler merged 34 commits intomainfrom
emmett.butler/remotely-disable-tracing

Conversation

@emmettbutler
Copy link
Collaborator

@emmettbutler emmettbutler commented Jan 11, 2024 •
edited by majorgreys
Loading

This pull request makes Tracer.enabled configurable via remote. Semantically this configuration is a one-way switch: it can be remotely set to False, but once it's False it cannot be remotely set to True. When False, it can be set to True via code or environment variable.

There isn't much risk in this change because it's a no-op until the UI starts sending the relevant remote configuration flag.

The remote config backend change supporting this feature is dd-go PR 118200.
The system-tests change validating this new functionality is DataDog/system-tests#2039.

Checklist

Reviewer Checklist

  • Title is accurate.
  • No unnecessary changes are introduced.
  • Description motivates each change.
  • Avoids breaking API changes unless absolutely necessary.
  • Testing strategy adequately addresses listed risk(s).
  • Change is maintainable (easy to change, telemetry, documentation).
  • Release note makes sense to a user of the library.
  • Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.
  • Backport labels are set in a manner that is consistent with the release branch maintenance policy
  • If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

APMON-578

@emmettbutler emmettbutler requested a review from a team as a code owner January 11, 2024 19:49
@emmettbutler emmettbutler marked this pull request as draft January 11, 2024 19:49
@pr-commenter
Copy link

pr-commenter bot commented Jan 11, 2024 •
edited
Loading

Benchmarks

Benchmark execution time: 2024-02-01 22:50:10

Comparing candidate commit a6b3112 in PR branch emmett.butler/remotely-disable-tracing with baseline commit 507510d in branch main.

Found 12 performance improvements and 16 performance regressions! Performance is the same for 167 metrics, 9 unstable metrics.

scenario:coreapiscenario-context_with_data_listeners

  • 🟩 max_rss_usage [-801.219KB; -618.045KB] or [-2.682%; -2.069%]

scenario:coreapiscenario-context_with_data_no_listeners

  • 🟩 max_rss_usage [-807.804KB; -663.070KB] or [-2.702%; -2.218%]

scenario:coreapiscenario-context_with_data_only_all_listeners

  • 🟩 max_rss_usage [-942.409KB; -779.140KB] or [-3.148%; -2.603%]

scenario:coreapiscenario-core_dispatch_listeners_and_all_listeners

  • 🟩 max_rss_usage [-753.855KB; -597.825KB] or [-2.529%; -2.006%]

scenario:coreapiscenario-core_dispatch_with_results_listeners

  • 🟩 max_rss_usage [-881.638KB; -723.584KB] or [-2.950%; -2.422%]

scenario:coreapiscenario-set_item

  • 🟥 max_rss_usage [+648.545KB; +805.535KB] or [+2.227%; +2.766%]

scenario:flasksimple-appsec-telemetry

  • 🟥 max_rss_usage [+922.283KB; +1074.107KB] or [+2.575%; +2.999%]

scenario:httppropagationextract-b3_headers

  • 🟩 max_rss_usage [-875.238KB; -671.002KB] or [-2.921%; -2.239%]

scenario:httppropagationextract-b3_single_headers

  • 🟥 max_rss_usage [+627.098KB; +815.513KB] or [+2.147%; +2.792%]

scenario:httppropagationextract-datadog_tracecontext_tracestate_not_propagated_on_trace_id_no_match

  • 🟥 max_rss_usage [+841.399KB; +1041.942KB] or [+2.898%; +3.589%]

scenario:httppropagationextract-full_t_id_datadog_headers

  • 🟥 max_rss_usage [+643.428KB; +847.106KB] or [+2.201%; +2.897%]

scenario:httppropagationextract-invalid_trace_id_header

  • 🟥 max_rss_usage [+634.085KB; +820.815KB] or [+2.166%; +2.804%]

scenario:httppropagationextract-wsgi_invalid_trace_id_header

  • 🟥 max_rss_usage [+615.956KB; +830.341KB] or [+2.104%; +2.837%]

scenario:httppropagationextract-wsgi_valid_headers_basic

  • 🟥 max_rss_usage [+655.109KB; +878.843KB] or [+2.244%; +3.011%]

scenario:httppropagationinject-ids_only

  • 🟥 max_rss_usage [+675.994KB; +822.732KB] or [+2.323%; +2.827%]

scenario:httppropagationinject-with_sampling_priority

  • 🟥 max_rss_usage [+658.459KB; +803.813KB] or [+2.256%; +2.754%]

scenario:httppropagationinject-with_tags

  • 🟩 max_rss_usage [-811.286KB; -648.528KB] or [-2.711%; -2.167%]

scenario:httppropagationinject-with_tags_invalid

  • 🟥 max_rss_usage [+744.915KB; +885.703KB] or [+2.558%; +3.041%]

scenario:otelspan-start-finish

  • 🟥 max_rss_usage [+619.953KB; +739.100KB] or [+2.030%; +2.421%]

scenario:otelspan-start-finish-telemetry

  • 🟥 max_rss_usage [+697.390KB; +815.263KB] or [+2.288%; +2.675%]

scenario:sethttpmeta-collectipvariant_exists

  • 🟥 max_rss_usage [+655.791KB; +752.414KB] or [+2.219%; +2.546%]

scenario:sethttpmeta-obfuscation-regular-case-explicit-query

  • 🟥 max_rss_usage [+673.400KB; +782.318KB] or [+2.266%; +2.633%]

scenario:sethttpmeta-obfuscation-regular-case-implicit-query

  • 🟥 max_rss_usage [+657.984KB; +764.557KB] or [+2.214%; +2.573%]

scenario:sethttpmeta-obfuscation-worst-case-explicit-query

  • 🟩 max_rss_usage [-730.360KB; -622.139KB] or [-2.401%; -2.045%]

scenario:sethttpmeta-useragentvariant_not_exists_2

  • 🟩 max_rss_usage [-784.554KB; -680.585KB] or [-2.593%; -2.249%]

scenario:span-start-finish

  • 🟩 max_rss_usage [-762.058KB; -641.641KB] or [-2.546%; -2.143%]

scenario:span-start-finish-telemetry

  • 🟩 max_rss_usage [-884.953KB; -741.569KB] or [-2.949%; -2.471%]

scenario:span-start-finish-traceid128

  • 🟩 max_rss_usage [-801.891KB; -692.739KB] or [-2.675%; -2.311%]

@emmettbutler emmettbutler changed the title sketch of remote disabling of tracing feat(tracing): allow tracing to be disabled remotely (via Datadog UI) Jan 12, 2024
@emmettbutler emmettbutler marked this pull request as ready for review January 12, 2024 20:26
@emmettbutler emmettbutler requested review from a team as code owners January 12, 2024 20:26
@emmettbutler emmettbutler marked this pull request as draft January 16, 2024 18:13
@emmettbutler emmettbutler mentioned this pull request Jan 19, 2024
Merged
9 tasks
@emmettbutler emmettbutler marked this pull request as ready for review January 19, 2024 19:51
@emmettbutler
Copy link
Collaborator Author

emmettbutler commented Jan 19, 2024

@Kyle-Verhoog this is ready to go as far as I can tell. let me know if you think it needs changes.

Kyle-Verhoog
Kyle-Verhoog reviewed Jan 26, 2024
View reviewed changes
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple thoughts. Depending on your answers I think this is good to go.

@emmettbutler emmettbutler mentioned this pull request Jan 29, 2024
Closed
18 tasks
@emmettbutler emmettbutler marked this pull request as draft January 29, 2024 17:51
@emmettbutler emmettbutler marked this pull request as ready for review January 29, 2024 20:32
@emmettbutler emmettbutler enabled auto-merge (squash) February 1, 2024 15:37
@emmettbutler emmettbutler merged commit a6d4c8d into main Feb 1, 2024
@emmettbutler emmettbutler deleted the emmett.butler/remotely-disable-tracing branch February 1, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@majorgreys majorgreys majorgreys approved these changes

@ZStriker19 ZStriker19 Awaiting requested review from ZStriker19 ZStriker19 is a code owner automatically assigned from DataDog/apm-core-python

@mabdinur mabdinur Awaiting requested review from mabdinur mabdinur is a code owner automatically assigned from DataDog/apm-core-python

@erikayasuda erikayasuda Awaiting requested review from erikayasuda

@Kyle-Verhoog Kyle-Verhoog Awaiting requested review from Kyle-Verhoog

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@emmettbutler @majorgreys @Kyle-Verhoog