Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore(telemetry): decouple telemetry writer from global config #10863

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

chore(telemetry): decouple telemetry writer from global config #10863

wants to merge 14 commits into from

Conversation

mabdinur
Copy link
Contributor

@mabdinur mabdinur commented Sep 30, 2024 •
edited
Loading

Description

Currently, the telemetry writer relies on ddtrace.config to enable and send telemetry payloads, while also queuing telemetry configuration events. This creates a circular dependency that can be painful to manage (currently supported via inlined import statements). This change resolves this circular dependency by refactoring ddtrace.internal.telemetry.telemetry_writer to access environment variables directly (instead of accessing ddtrace.config).

This change will allow ddtrace contributors to queue telemetry configurations, metrics, and logs before the global config object is initialized.

Changes

  • Removes all references to ddtrace.config and all envier configuration objects from ddtrace.internal.telemetry package
  • Ensures configuration telemetry is queued when configurations are defined
  • Ensures the origin of configuration is sent sent via instrumentation telemetry (possible values: remote_config, env_var, code, and unknown). Previously the origin of all configurations was set to unknown.
  • Updates the name of telemetry configuration to match the name of environment variables (where possible).

Checklist

  • PR author has checked that all the criteria below are met
  • The PR description includes an overview of the change
  • The PR description articulates the motivation for the change
  • The change includes tests OR the PR description describes a testing strategy
  • The PR description notes risks associated with the change, if any
  • Newly-added code is easy to change
  • The change follows the library release note guidelines
  • The change includes or references documentation updates if necessary
  • Backport labels are set (if applicable)

Reviewer Checklist

  • Reviewer has checked that all the criteria below are met
  • Title is accurate
  • All changes are related to the pull request's stated goal
  • Avoids breaking API changes
  • Testing strategy adequately addresses listed risks
  • Newly-added code is easy to change
  • Release note makes sense to a user of the library
  • If necessary, author has 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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 30, 2024 •
edited
Loading

CODEOWNERS have been resolved as:

ddtrace/settings/_core.py                                               @DataDog/apm-core-python
ddtrace/_monkey.py                                                      @DataDog/apm-core-python
ddtrace/auto.py                                                         @DataDog/apm-core-python
ddtrace/bootstrap/sitecustomize.py                                      @DataDog/apm-core-python
ddtrace/internal/agent.py                                               @DataDog/apm-core-python
ddtrace/internal/runtime/runtime_metrics.py                             @DataDog/apm-core-python
ddtrace/internal/telemetry/constants.py                                 @DataDog/apm-core-python
ddtrace/internal/telemetry/data.py                                      @DataDog/apm-core-python
ddtrace/internal/telemetry/writer.py                                    @DataDog/apm-core-python
ddtrace/settings/asm.py                                                 @DataDog/asm-python
ddtrace/settings/config.py                                              @DataDog/python-guild @DataDog/apm-sdk-api-python
ddtrace/settings/crashtracker.py                                        @DataDog/apm-core-python
ddtrace/settings/dynamic_instrumentation.py                             @DataDog/debugger-python
ddtrace/settings/exception_replay.py                                    @DataDog/apm-core-python
ddtrace/settings/peer_service.py                                        @DataDog/apm-core-python
ddtrace/settings/profiling.py                                           @DataDog/profiling-python
ddtrace/settings/symbol_db.py                                           @DataDog/apm-core-python
tests/conftest.py                                                       @DataDog/apm-core-python
tests/integration/test_settings.py                                      @DataDog/apm-core-python
tests/telemetry/test_writer.py                                          @DataDog/apm-core-python
tests/tracer/test_global_config.py                                      @DataDog/apm-sdk-api-python

datadog-datadog-prod-us1[bot]
datadog-datadog-prod-us1 bot reviewed Sep 30, 2024
View reviewed changes
tests/telemetry/test_writer.py Outdated Show resolved Hide resolved
ddtrace/internal/telemetry/writer.py Outdated Show resolved Hide resolved
tests/telemetry/test_writer.py Show resolved Hide resolved
@P403n1x87
Copy link
Contributor

That's great! Did we consider a callback/core event hub alternative approach to break the circular dependency?

@pr-commenter
Copy link

pr-commenter bot commented Oct 1, 2024 •
edited
Loading

Benchmarks

Benchmark execution time: 2024-10-02 15:44:34

Comparing candidate commit bbf8fda in PR branch munir/decouple-telemetry-writer-from-global-config with baseline commit 099ab94 in branch main.

Found 134 performance improvements and 0 performance regressions! Performance is the same for 237 metrics, 53 unstable metrics.

scenario:coreapiscenario-context_with_data_listeners

  • 🟩 max_rss_usage [-4.123MB; -3.925MB] or [-12.830%; -12.213%]

scenario:coreapiscenario-context_with_data_no_listeners

  • 🟩 max_rss_usage [-3.835MB; -3.541MB] or [-12.075%; -11.147%]

scenario:coreapiscenario-context_with_data_only_all_listeners

  • 🟩 max_rss_usage [-3.915MB; -3.696MB] or [-12.204%; -11.522%]

scenario:coreapiscenario-get_item_exists

  • 🟩 max_rss_usage [-3.890MB; -3.650MB] or [-12.119%; -11.370%]

scenario:coreapiscenario-get_item_missing

  • 🟩 max_rss_usage [-3.788MB; -3.436MB] or [-11.851%; -10.750%]

scenario:coreapiscenario-set_item

  • 🟩 max_rss_usage [-4.064MB; -3.879MB] or [-12.612%; -12.037%]

scenario:flasksqli-baseline

  • 🟩 max_rss_usage [-3.525MB; -3.320MB] or [-7.895%; -7.435%]

scenario:httppropagationextract-all_styles_all_headers

  • 🟩 max_rss_usage [-3.879MB; -3.710MB] or [-12.030%; -11.507%]

scenario:httppropagationextract-b3_headers

  • 🟩 max_rss_usage [-4.114MB; -3.909MB] or [-12.735%; -12.100%]

scenario:httppropagationextract-b3_single_headers

  • 🟩 max_rss_usage [-3.368MB; -3.162MB] or [-10.654%; -10.002%]

scenario:httppropagationextract-datadog_tracecontext_tracestate_not_propagated_on_trace_id_no_match

  • 🟩 max_rss_usage [-4.052MB; -3.833MB] or [-12.535%; -11.859%]

scenario:httppropagationextract-datadog_tracecontext_tracestate_propagated_on_trace_id_match

  • 🟩 max_rss_usage [-3.435MB; -3.269MB] or [-10.828%; -10.306%]

scenario:httppropagationextract-empty_headers

  • 🟩 max_rss_usage [-3.246MB; -3.073MB] or [-10.283%; -9.733%]

scenario:httppropagationextract-full_t_id_datadog_headers

  • 🟩 max_rss_usage [-3.997MB; -3.771MB] or [-12.365%; -11.666%]

scenario:httppropagationextract-invalid_priority_header

  • 🟩 max_rss_usage [-3.855MB; -3.663MB] or [-11.955%; -11.360%]

scenario:httppropagationextract-invalid_span_id_header

  • 🟩 max_rss_usage [-3.271MB; -3.102MB] or [-10.358%; -9.822%]

scenario:httppropagationextract-invalid_tags_header

  • 🟩 max_rss_usage [-3.285MB; -3.058MB] or [-10.383%; -9.663%]

scenario:httppropagationextract-invalid_trace_id_header

  • 🟩 max_rss_usage [-3.958MB; -3.771MB] or [-12.263%; -11.686%]

scenario:httppropagationextract-large_header_no_matches

  • 🟩 max_rss_usage [-3.907MB; -3.721MB] or [-12.109%; -11.535%]

scenario:httppropagationextract-large_valid_headers_all

  • 🟩 max_rss_usage [-3.275MB; -3.079MB] or [-10.361%; -9.743%]

scenario:httppropagationextract-medium_header_no_matches

  • 🟩 max_rss_usage [-3.950MB; -3.757MB] or [-12.237%; -11.637%]

scenario:httppropagationextract-medium_valid_headers_all

  • 🟩 max_rss_usage [-3.391MB; -3.180MB] or [-10.705%; -10.041%]

scenario:httppropagationextract-none_propagation_style

  • 🟩 max_rss_usage [-4.035MB; -3.818MB] or [-12.477%; -11.806%]

scenario:httppropagationextract-tracecontext_headers

  • 🟩 max_rss_usage [-4.043MB; -3.828MB] or [-12.532%; -11.866%]

scenario:httppropagationextract-valid_headers_all

  • 🟩 max_rss_usage [-3.947MB; -3.736MB] or [-12.224%; -11.571%]

scenario:httppropagationextract-valid_headers_basic

  • 🟩 max_rss_usage [-4.012MB; -3.795MB] or [-12.421%; -11.749%]

scenario:httppropagationextract-wsgi_empty_headers

  • 🟩 max_rss_usage [-4.009MB; -3.847MB] or [-12.433%; -11.930%]

scenario:httppropagationextract-wsgi_invalid_priority_header

  • 🟩 max_rss_usage [-3.260MB; -3.032MB] or [-10.306%; -9.585%]

scenario:httppropagationextract-wsgi_invalid_span_id_header

  • 🟩 max_rss_usage [-3.231MB; -3.042MB] or [-10.228%; -9.631%]

scenario:httppropagationextract-wsgi_invalid_tags_header

  • 🟩 max_rss_usage [-3.260MB; -3.056MB] or [-10.315%; -9.669%]

scenario:httppropagationextract-wsgi_invalid_trace_id_header

  • 🟩 max_rss_usage [-4.006MB; -3.797MB] or [-12.388%; -11.740%]

scenario:httppropagationextract-wsgi_large_header_no_matches

  • 🟩 max_rss_usage [-3.266MB; -3.065MB] or [-10.332%; -9.694%]

scenario:httppropagationextract-wsgi_large_valid_headers_all

  • 🟩 max_rss_usage [-3.491MB; -3.262MB] or [-10.980%; -10.260%]

scenario:httppropagationextract-wsgi_medium_header_no_matches

  • 🟩 max_rss_usage [-3.278MB; -3.024MB] or [-10.367%; -9.565%]

scenario:httppropagationextract-wsgi_medium_valid_headers_all

  • 🟩 max_rss_usage [-3.456MB; -3.308MB] or [-10.896%; -10.428%]

scenario:httppropagationextract-wsgi_valid_headers_all

  • 🟩 max_rss_usage [-3.215MB; -2.978MB] or [-10.172%; -9.422%]

scenario:httppropagationextract-wsgi_valid_headers_basic

  • 🟩 max_rss_usage [-3.995MB; -3.830MB] or [-12.375%; -11.864%]

scenario:httppropagationinject-ids_only

  • 🟩 max_rss_usage [-3.852MB; -3.659MB] or [-11.938%; -11.340%]

scenario:httppropagationinject-with_all

  • 🟩 max_rss_usage [-3.890MB; -3.685MB] or [-12.053%; -11.417%]

scenario:httppropagationinject-with_dd_origin

  • 🟩 max_rss_usage [-3.275MB; -3.023MB] or [-10.363%; -9.565%]

scenario:httppropagationinject-with_priority_and_origin

  • 🟩 max_rss_usage [-4.107MB; -3.840MB] or [-12.696%; -11.872%]

scenario:httppropagationinject-with_sampling_priority

  • 🟩 max_rss_usage [-3.951MB; -3.788MB] or [-12.255%; -11.749%]

scenario:httppropagationinject-with_tags

  • 🟩 max_rss_usage [-3.879MB; -3.694MB] or [-12.033%; -11.458%]

scenario:httppropagationinject-with_tags_invalid

  • 🟩 max_rss_usage [-3.981MB; -3.804MB] or [-12.332%; -11.785%]

scenario:httppropagationinject-with_tags_max_size

  • 🟩 max_rss_usage [-3.462MB; -3.198MB] or [-10.950%; -10.115%]

scenario:iast_aspects-aspect_iast_do_os_path_basename

  • 🟩 max_rss_usage [-3.308MB; -2.997MB] or [-7.891%; -7.150%]

scenario:iast_aspects-aspect_iast_do_os_path_join

  • 🟩 max_rss_usage [-3.863MB; -3.503MB] or [-9.192%; -8.336%]

scenario:iast_aspects-aspect_iast_do_os_path_splitdrive

  • 🟩 max_rss_usage [-3.431MB; -3.106MB] or [-8.194%; -7.418%]

scenario:iast_aspects-aspect_no_iast_do_add_and_uppercase

  • 🟩 max_rss_usage [-2.629MB; -2.413MB] or [-8.026%; -7.367%]

scenario:iast_aspects-aspect_no_iast_do_add_re_compile

  • 🟩 max_rss_usage [-2.777MB; -2.571MB] or [-8.426%; -7.803%]

scenario:iast_aspects-aspect_no_iast_do_capitalize

  • 🟩 max_rss_usage [-2.559MB; -2.360MB] or [-7.816%; -7.209%]

scenario:iast_aspects-aspect_no_iast_do_casefold

  • 🟩 max_rss_usage [-2.518MB; -2.456MB] or [-7.686%; -7.496%]

scenario:iast_aspects-aspect_no_iast_do_center

  • 🟩 max_rss_usage [-2.616MB; -2.446MB] or [-7.976%; -7.459%]

scenario:iast_aspects-aspect_no_iast_do_customspec_ascii

  • 🟩 max_rss_usage [-2.894MB; -2.688MB] or [-8.782%; -8.158%]

scenario:iast_aspects-aspect_no_iast_do_encode

  • 🟩 max_rss_usage [-2.888MB; -2.682MB] or [-8.764%; -8.139%]

scenario:iast_aspects-aspect_no_iast_do_expandtabs

  • 🟩 max_rss_usage [-2.507MB; -2.461MB] or [-7.652%; -7.513%]

scenario:iast_aspects-aspect_no_iast_do_exporttype_member_format

  • 🟩 max_rss_usage [-3.724MB; -3.572MB] or [-10.978%; -10.528%]

scenario:iast_aspects-aspect_no_iast_do_fmt_value

  • 🟩 max_rss_usage [-3.260MB; -3.253MB] or [-10.327%; -10.303%]

scenario:iast_aspects-aspect_no_iast_do_format_map

  • 🟩 max_rss_usage [-2.838MB; -2.630MB] or [-8.627%; -7.994%]

scenario:iast_aspects-aspect_no_iast_do_index

  • 🟩 max_rss_usage [-2.643MB; -2.536MB] or [-8.047%; -7.718%]

scenario:iast_aspects-aspect_no_iast_do_index_on_dict

  • 🟩 max_rss_usage [-2.889MB; -2.684MB] or [-8.768%; -8.144%]

scenario:iast_aspects-aspect_no_iast_do_io_bytesio_read

  • 🟩 max_rss_usage [-2.806MB; -2.803MB] or [-8.495%; -8.487%]

scenario:iast_aspects-aspect_no_iast_do_io_stringio_read

  • 🟩 max_rss_usage [-2.826MB; -2.671MB] or [-8.565%; -8.097%]

scenario:iast_aspects-aspect_no_iast_do_join

  • 🟩 max_rss_usage [-2.890MB; -2.685MB] or [-8.771%; -8.147%]

scenario:iast_aspects-aspect_no_iast_do_ljust

  • 🟩 max_rss_usage [-2.625MB; -2.387MB] or [-8.009%; -7.285%]

scenario:iast_aspects-aspect_no_iast_do_lower

  • 🟩 max_rss_usage [-2.777MB; -2.571MB] or [-8.426%; -7.803%]

scenario:iast_aspects-aspect_no_iast_do_lstrip

  • 🟩 max_rss_usage [-2.790MB; -2.636MB] or [-8.458%; -7.990%]

scenario:iast_aspects-aspect_no_iast_do_modulo

  • 🟩 max_rss_usage [-2.753MB; -2.753MB] or [-8.333%; -8.333%]

scenario:iast_aspects-aspect_no_iast_do_multiple_string_assigment

  • 🟩 max_rss_usage [-3.748MB; -3.669MB] or [-11.063%; -10.829%]

scenario:iast_aspects-aspect_no_iast_do_operator_add_inplace_3_params

  • 🟩 max_rss_usage [-2.603MB; -2.372MB] or [-7.977%; -7.267%]

scenario:iast_aspects-aspect_no_iast_do_operator_add_inplace_3_times

  • 🟩 max_rss_usage [-2.777MB; -2.571MB] or [-8.426%; -7.803%]

scenario:iast_aspects-aspect_no_iast_do_operator_add_inplace_params

  • 🟩 max_rss_usage [-2.790MB; -2.636MB] or [-8.458%; -7.990%]

scenario:iast_aspects-aspect_no_iast_do_operator_add_params

  • 🟩 max_rss_usage [-2.507MB; -2.301MB] or [-7.672%; -7.041%]

scenario:iast_aspects-aspect_no_iast_do_os_path_basename

  • 🟩 max_rss_usage [-3.270MB; -3.047MB] or [-10.390%; -9.683%]

scenario:iast_aspects-aspect_no_iast_do_os_path_dirname

  • 🟩 max_rss_usage [-3.397MB; -3.162MB] or [-10.752%; -10.009%]

scenario:iast_aspects-aspect_no_iast_do_os_path_join

  • 🟩 max_rss_usage [-3.996MB; -3.790MB] or [-12.423%; -11.784%]

scenario:iast_aspects-aspect_no_iast_do_os_path_normcase

  • 🟩 max_rss_usage [-3.956MB; -3.751MB] or [-12.300%; -11.661%]

scenario:iast_aspects-aspect_no_iast_do_os_path_split

  • 🟩 max_rss_usage [-2.852MB; -2.850MB] or [-9.152%; -9.147%]

scenario:iast_aspects-aspect_no_iast_do_os_path_splitdrive

  • 🟩 max_rss_usage [-3.298MB; -3.040MB] or [-10.477%; -9.657%]

scenario:iast_aspects-aspect_no_iast_do_os_path_splitext

  • 🟩 max_rss_usage [-3.295MB; -3.084MB] or [-10.461%; -9.791%]

scenario:iast_aspects-aspect_no_iast_do_partition

  • 🟩 max_rss_usage [-2.768MB; -2.764MB] or [-8.382%; -8.367%]

scenario:iast_aspects-aspect_no_iast_do_re_sub

  • 🟩 max_rss_usage [-2.752MB; -2.517MB] or [-8.362%; -7.647%]

scenario:iast_aspects-aspect_no_iast_do_replace

  • 🟩 max_rss_usage [-2.777MB; -2.571MB] or [-8.426%; -7.803%]

scenario:iast_aspects-aspect_no_iast_do_repr

  • 🟩 max_rss_usage [-2.753MB; -2.753MB] or [-8.333%; -8.333%]

scenario:iast_aspects-aspect_no_iast_do_repr_fstring

  • 🟩 max_rss_usage [-4.070MB; -3.916MB] or [-12.486%; -12.013%]

scenario:iast_aspects-aspect_no_iast_do_sensitive_variables

  • 🟩 max_rss_usage [-2.630MB; -2.476MB] or [-8.047%; -7.573%]

scenario:iast_aspects-aspect_no_iast_do_slice

  • 🟩 max_rss_usage [-2.721MB; -2.469MB] or [-8.278%; -7.512%]

scenario:iast_aspects-aspect_no_iast_do_split_separator_and_maxsplit

  • 🟩 max_rss_usage [-2.794MB; -2.589MB] or [-8.480%; -7.856%]

scenario:iast_aspects-aspect_no_iast_do_splitlines_keepends

  • 🟩 max_rss_usage [-2.602MB; -2.563MB] or [-7.951%; -7.830%]

scenario:iast_aspects-aspect_no_iast_do_str

  • 🟩 max_rss_usage [-2.800MB; -2.594MB] or [-8.497%; -7.874%]

scenario:iast_aspects-aspect_no_iast_do_string_assignment

  • 🟩 max_rss_usage [-2.790MB; -2.787MB] or [-8.446%; -8.439%]

scenario:iast_aspects-aspect_no_iast_do_stringio_init_and_read

  • 🟩 max_rss_usage [-2.510MB; -2.296MB] or [-7.681%; -7.026%]

scenario:iast_aspects-aspect_no_iast_do_swapcase

  • 🟩 max_rss_usage [-2.498MB; -2.454MB] or [-7.627%; -7.492%]

scenario:iast_aspects-aspect_no_iast_do_title

  • 🟩 max_rss_usage [-2.777MB; -2.571MB] or [-8.426%; -7.803%]

scenario:iast_aspects-aspect_no_iast_do_translate

  • 🟩 max_rss_usage [-2.495MB; -2.341MB] or [-7.631%; -7.160%]

scenario:iast_aspects-aspect_no_iast_do_tuple_string_assignment

  • 🟩 max_rss_usage [-3.638MB; -3.481MB] or [-10.769%; -10.307%]

scenario:iast_aspects-aspect_no_iast_do_upper

  • 🟩 max_rss_usage [-2.753MB; -2.753MB] or [-8.333%; -8.333%]

scenario:iast_aspects-aspect_no_iast_do_zfill

  • 🟩 max_rss_usage [-2.844MB; -2.535MB] or [-8.630%; -7.694%]

scenario:otelspan-start-finish

  • 🟩 max_rss_usage [-3.209MB; -3.015MB] or [-9.903%; -9.305%]

scenario:otelspan-start-finish-telemetry

  • 🟩 max_rss_usage [-4.317MB; -4.137MB] or [-13.061%; -12.519%]

scenario:ratelimiter-defaults

  • 🟩 max_rss_usage [-4.070MB; -3.979MB] or [-12.444%; -12.166%]

scenario:ratelimiter-high_rate_limit

  • 🟩 max_rss_usage [-4.020MB; -3.915MB] or [-12.305%; -11.986%]

scenario:ratelimiter-long_window

  • 🟩 max_rss_usage [-3.937MB; -3.931MB] or [-12.063%; -12.046%]

scenario:ratelimiter-low_rate_limit

  • 🟩 max_rss_usage [-3.996MB; -3.927MB] or [-12.235%; -12.022%]

scenario:ratelimiter-no_rate_limit

  • 🟩 max_rss_usage [-3.942MB; -3.934MB] or [-12.078%; -12.055%]

scenario:ratelimiter-short_window

  • 🟩 max_rss_usage [-4.042MB; -3.984MB] or [-12.361%; -12.184%]

scenario:samplingrules-average_match

  • 🟩 max_rss_usage [-3.697MB; -3.627MB] or [-11.706%; -11.486%]

scenario:samplingrules-high_match

  • 🟩 max_rss_usage [-3.680MB; -3.559MB] or [-11.655%; -11.269%]

scenario:sethttpmeta-all-disabled

  • 🟩 max_rss_usage [-3.917MB; -3.755MB] or [-12.002%; -11.507%]

scenario:sethttpmeta-all-enabled

  • 🟩 max_rss_usage [-3.826MB; -3.650MB] or [-11.724%; -11.184%]

scenario:sethttpmeta-collectipvariant_exists

  • 🟩 max_rss_usage [-3.943MB; -3.772MB] or [-12.082%; -11.557%]

scenario:sethttpmeta-no-collectipvariant

  • 🟩 max_rss_usage [-3.514MB; -3.328MB] or [-10.760%; -10.192%]

scenario:sethttpmeta-no-useragentvariant

  • 🟩 max_rss_usage [-2.931MB; -2.761MB] or [-9.161%; -8.631%]

scenario:sethttpmeta-obfuscation-disabled

  • 🟩 max_rss_usage [-3.543MB; -3.417MB] or [-10.856%; -10.469%]

scenario:sethttpmeta-obfuscation-no-query

  • 🟩 max_rss_usage [-3.514MB; -3.328MB] or [-10.760%; -10.192%]

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

  • 🟩 max_rss_usage [-3.945MB; -3.723MB] or [-12.064%; -11.387%]

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

  • 🟩 max_rss_usage [-3.053MB; -2.886MB] or [-9.485%; -8.966%]

scenario:sethttpmeta-obfuscation-send-querystring-disabled

  • 🟩 max_rss_usage [-3.444MB; -3.251MB] or [-10.458%; -9.869%]

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

  • 🟩 max_rss_usage [-2.503MB; -2.319MB] or [-7.810%; -7.236%]

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

  • 🟩 max_rss_usage [-3.479MB; -3.286MB] or [-10.546%; -9.959%]

scenario:sethttpmeta-useragentvariant_exists_1

  • 🟩 max_rss_usage [-3.502MB; -3.340MB] or [-10.730%; -10.234%]

scenario:sethttpmeta-useragentvariant_exists_2

  • 🟩 max_rss_usage [-3.502MB; -3.309MB] or [-10.725%; -10.132%]

scenario:sethttpmeta-useragentvariant_exists_3

  • 🟩 max_rss_usage [-3.573MB; -3.394MB] or [-10.943%; -10.394%]

scenario:sethttpmeta-useragentvariant_not_exists_1

  • 🟩 max_rss_usage [-2.969MB; -2.733MB] or [-9.255%; -8.522%]

scenario:sethttpmeta-useragentvariant_not_exists_2

  • 🟩 max_rss_usage [-2.786MB; -2.610MB] or [-8.704%; -8.153%]

scenario:span-add-tags

  • 🟩 max_rss_usage [-2.734MB; -2.624MB] or [-7.487%; -7.186%]

scenario:span-start

  • 🟩 max_rss_usage [-3.492MB; -3.399MB] or [-7.324%; -7.129%]

scenario:span-start-finish

  • 🟩 max_rss_usage [-4.393MB; -4.316MB] or [-13.631%; -13.392%]

scenario:span-start-finish-telemetry

  • 🟩 max_rss_usage [-4.086MB; -4.084MB] or [-12.611%; -12.606%]

scenario:span-start-finish-traceid128

  • 🟩 max_rss_usage [-4.154MB; -3.967MB] or [-12.848%; -12.270%]

scenario:span-start-traceid128

  • 🟩 max_rss_usage [-4.195MB; -4.118MB] or [-8.688%; -8.529%]

scenario:tracer-large

  • 🟩 max_rss_usage [-4.046MB; -3.969MB] or [-12.079%; -11.849%]

scenario:tracer-medium

  • 🟩 max_rss_usage [-3.323MB; -3.259MB] or [-10.541%; -10.338%]

scenario:tracer-small

  • 🟩 max_rss_usage [-3.719MB; -3.675MB] or [-11.619%; -11.480%]

@mabdinur mabdinur force-pushed the munir/decouple-telemetry-writer-from-global-config branch from 3f45532 to 8b9f92f Compare October 1, 2024 21:44
@mabdinur
Copy link
Contributor Author

mabdinur commented Oct 1, 2024

That's great! Did we consider a callback/core event hub alternative approach to break the circular dependency?

Good point. I though this approach could introduce a lot of complexity. We will need to ensure telemetry is started soon after we start queueing events to prevent memory leaks or data loss associated with unbounded metric and logs events.

I think the simplest approach is to decouple the ddtrace.internal.telemetry package from as many ddtrace modules as possible. This will allow us to start collecting telemetry via telemetry_writer in the vast majority of ddtrace modules. This PR doesn't fully achieve this goal but it's a step in that direction.

@mabdinur mabdinur force-pushed the munir/decouple-telemetry-writer-from-global-config branch from 535fd96 to 211bf0b Compare October 1, 2024 22:33
@mabdinur mabdinur marked this pull request as ready for review October 1, 2024 22:33
@mabdinur mabdinur requested review from a team as code owners October 1, 2024 22:33
@mabdinur mabdinur force-pushed the munir/decouple-telemetry-writer-from-global-config branch from d5cde12 to 0bffcd5 Compare October 2, 2024 04:39
@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Oct 2, 2024 •
edited
Loading

Datadog Report

Branch report: munir/decouple-telemetry-writer-from-global-config
Commit report: f07f2bb
Test service: dd-trace-py

✅ 0 Failed, 1286 Passed, 0 Skipped, 29m 41.54s Total duration (7m 23.45s time saved)

mabdinur
mabdinur commented Oct 2, 2024
View reviewed changes
ddtrace/settings/config.py Outdated Show resolved Hide resolved
mabdinur
mabdinur commented Oct 2, 2024
View reviewed changes
tests/telemetry/test_telemetry.py Outdated
@@ -22,6 +22,7 @@ def test_enable(test_agent_session, run_python_code_in_subprocess):


@pytest.mark.snapshot
@pytest.mark.skip("This test will be removed in a another PR")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
@pytest.mark.skip("This test will be removed in a another PR")

@mabdinur mabdinur force-pushed the munir/decouple-telemetry-writer-from-global-config branch from 2bf7b98 to 79611f0 Compare October 2, 2024 14:51
@mabdinur mabdinur added the changelog/no-changelog A changelog entry is not required for this PR. label Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datadog-datadog-prod-us1 Datadog - DataDog prod us1 Datadog - DataDog prod us1 left review comments

@gnufede gnufede Awaiting requested review from gnufede gnufede is a code owner automatically assigned from DataDog/python-guild

@rachelyangdog rachelyangdog Awaiting requested review from rachelyangdog rachelyangdog is a code owner automatically assigned from DataDog/apm-sdk-api-python

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

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mabdinur @P403n1x87