Skip to content

chore: introduce APM_TRACING RC product #11980

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

Merged
merged 17 commits into from
Apr 2, 2025
Merged

chore: introduce APM_TRACING RC product #11980

merged 17 commits into from
Apr 2, 2025

Conversation

P403n1x87
Copy link
Contributor

@P403n1x87 P403n1x87 commented Jan 16, 2025

We refactor the APM_TRACING remote configuration product that allows dispatching remote configuration to the library for remote enablement/configuration of library components and features. Firstly, we make this an actual product, in the sense of the product protocol. We then extend the latter to modularise capabilities and remote APM configuration handling.

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

@P403n1x87 P403n1x87 added the changelog/no-changelog A changelog entry is not required for this PR. label Jan 16, 2025
@P403n1x87 P403n1x87 self-assigned this Jan 16, 2025
Copy link
Contributor

github-actions bot commented Jan 16, 2025

CODEOWNERS have been resolved as:

ddtrace/internal/remoteconfig/products/__init__.py                      @DataDog/remote-config @DataDog/apm-core-python
ddtrace/internal/remoteconfig/products/apm_tracing.py                   @DataDog/remote-config @DataDog/apm-core-python
ddtrace/_trace/product.py                                               @DataDog/apm-sdk-api-python
ddtrace/internal/README.md                                              @DataDog/python-guild @DataDog/apm-core-python
ddtrace/settings/_config.py                                             @DataDog/apm-core-python
pyproject.toml                                                          @DataDog/python-guild
riotfile.py                                                             @DataDog/apm-python
tests/integration/test_settings.py                                      @DataDog/apm-core-python
tests/internal/test_settings.py                                         @DataDog/apm-core-python
ddtrace/internal/remoteconfig/products/client.py                        @DataDog/remote-config @DataDog/apm-core-python

@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Jan 16, 2025

Datadog Report

Branch report: chore/apm-tracing-rc
Commit report: 7c4ca59
Test service: dd-trace-py

✅ 0 Failed, 130 Passed, 1468 Skipped, 4m 50.35s Total duration (35m 10.03s time saved)

@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from 6367ef6 to 13eabbc Compare January 16, 2025 11:33
@pr-commenter
Copy link

pr-commenter bot commented Jan 16, 2025

Benchmarks

Benchmark execution time: 2025-04-02 12:15:44

Comparing candidate commit 76de753 in PR branch chore/apm-tracing-rc with baseline commit 5b3a8f2 in branch main.

Found 2 performance improvements and 0 performance regressions! Performance is the same for 496 metrics, 2 unstable metrics.

scenario:iast_aspects-ospathbasename_aspect

  • 🟩 execution_time [-495.560ns; -439.787ns] or [-13.280%; -11.786%]

scenario:iast_aspects-ospathdirname_aspect

  • 🟩 execution_time [-362.416ns; -305.018ns] or [-8.900%; -7.491%]

@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from 13eabbc to 7c4ca59 Compare January 20, 2025 13:52
@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from 7c4ca59 to a793862 Compare January 30, 2025 10:46
@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Jan 30, 2025

Datadog Report

Branch report: chore/apm-tracing-rc
Commit report: c6ce9ed
Test service: dd-trace-py

✅ 0 Failed, 43 Passed, 290 Skipped, 53.52s Total duration (5m 2.68s time saved)

@erikayasuda erikayasuda force-pushed the main branch 2 times, most recently from 1247ac2 to 2ccaaef Compare February 6, 2025 20:43
Copy link
Contributor

@shatzi shatzi left a comment

Choose a reason for hiding this comment

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

I think we should also record the right capabilities once they set.

@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch 2 times, most recently from 5eb55df to e55bebb Compare February 14, 2025 16:06
@P403n1x87 P403n1x87 marked this pull request as ready for review February 19, 2025 15:05
@P403n1x87 P403n1x87 requested review from a team as code owners February 19, 2025 15:05
@P403n1x87 P403n1x87 requested a review from juanjux February 19, 2025 15:05
@P403n1x87 P403n1x87 enabled auto-merge (squash) February 19, 2025 15:08
@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from 3d93ec6 to d15eb79 Compare February 21, 2025 10:00
We introduce the APM_TRACING remote configuration product that allows
dispatching remote configuration to the library for remote enablement/
configuration of library components and features.
@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from 5fabcb6 to ac960ab Compare February 27, 2025 11:29
@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from 466e7a0 to 2eee4f4 Compare March 6, 2025 14:17
@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from 2eee4f4 to 89b3d7e Compare March 6, 2025 16:00
@mabdinur
Copy link
Contributor

mabdinur commented Mar 9, 2025

Benchmarks

Benchmark execution time: 2025-03-06 17:07:38

Comparing candidate commit 89b3d7e in PR branch chore/apm-tracing-rc with baseline commit 63c1237 in branch main.

Found 0 performance improvements and 9 performance regressions! Performance is the same for 451 metrics, 2 unstable metrics.

scenario:startup-ddtrace_run

  • 🟥 execution_time [+109.758ms; +111.183ms] or [+10.931%; +11.073%]

scenario:startup-ddtrace_run_django

  • 🟥 execution_time [+132.938ms; +134.796ms] or [+11.895%; +12.061%]

scenario:startup-ddtrace_run_flask

  • 🟥 execution_time [+125.581ms; +127.328ms] or [+10.802%; +10.953%]

scenario:startup-ddtrace_run_import_ddtrace_auto

  • 🟥 execution_time [+109.066ms; +110.516ms] or [+10.866%; +11.010%]

scenario:startup-ddtrace_run_import_ddtrace_auto_django

  • 🟥 execution_time [+134.688ms; +136.274ms] or [+12.057%; +12.199%]

scenario:startup-ddtrace_run_import_ddtrace_auto_flask

  • 🟥 execution_time [+126.786ms; +128.407ms] or [+10.906%; +11.045%]

scenario:startup-import_ddtrace_auto

  • 🟥 execution_time [+109.915ms; +111.070ms] or [+16.861%; +17.038%]

scenario:startup-import_ddtrace_auto_django

  • 🟥 execution_time [+126.351ms; +127.887ms] or [+16.331%; +16.529%]

scenario:startup-import_ddtrace_auto_flask

  • 🟥 execution_time [+126.095ms; +127.408ms] or [+15.585%; +15.747%]

The new plugin interface increases start up time by over 10%. Can we try to optimize this? This seems like a blocker

Copy link
Contributor

@mabdinur mabdinur left a comment

Choose a reason for hiding this comment

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

left a comment about the overhead of this new API

@mabdinur mabdinur self-requested a review March 9, 2025 20:27
@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from 2030f87 to 67b394c Compare March 31, 2025 14:37
Copy link
Contributor

github-actions bot commented Mar 31, 2025

Bootstrap import analysis

Comparison of import times between this PR and base.

Summary

The average import time from this PR is: 244 ± 8 ms.

The average import time from base is: 247 ± 8 ms.

The import time difference between this PR and base is: -3.2 ± 0.3 ms.

Import time breakdown

The following import paths have grown:

ddtrace.auto 0.812 ms (0.33%)
ddtrace.bootstrap.sitecustomize 0.719 ms (0.29%)
ddtrace.bootstrap.preload 0.447 ms (0.18%)
ddtrace.internal.products 0.184 ms (0.08%)
ddtrace.settings.dynamic_instrumentation 0.109 ms (0.04%)
multiprocessing 0.074 ms (0.03%)
multiprocessing.context 0.074 ms (0.03%)
multiprocessing.process 0.074 ms (0.03%)
ddtrace.settings.crashtracker 0.105 ms (0.04%)
ddtrace.internal.flare.flare 0.086 ms (0.04%)
logging.handlers 0.086 ms (0.04%)
ddtrace.settings.profiling 0.073 ms (0.03%)
ddtrace.vendor.psutil 0.073 ms (0.03%)
ddtrace.vendor.psutil._pslinux 0.073 ms (0.03%)
glob 0.073 ms (0.03%)
ddtrace.appsec._common_module_patches 0.109 ms (0.04%)
ddtrace.appsec._asm_request_context 0.109 ms (0.04%)
shlex 0.101 ms (0.04%)
ddtrace._trace.trace_handlers 0.061 ms (0.03%)
ddtrace._trace._inferred_proxy 0.061 ms (0.03%)
ddtrace 0.094 ms (0.04%)
ddtrace.settings._config 0.094 ms (0.04%)
ddtrace.internal.gitmetadata 0.094 ms (0.04%)
ddtrace.ext.ci 0.094 ms (0.04%)

The following import paths have shrunk:

ddtrace.auto 2.329 ms (0.96%)
ddtrace.bootstrap.sitecustomize 1.498 ms (0.61%)
ddtrace.bootstrap.preload 1.237 ms (0.51%)
ddtrace.internal.products 0.801 ms (0.33%)
ddtrace.internal.remoteconfig.client 0.653 ms (0.27%)
multiprocessing 0.149 ms (0.06%)
multiprocessing.context 0.103 ms (0.04%)
ddtrace.appsec._remoteconfiguration 0.134 ms (0.06%)
ddtrace.appsec._capabilities 0.134 ms (0.06%)
ddtrace.settings.crashtracker 0.124 ms (0.05%)
ddtrace.internal.datadog.profiling.crashtracker 0.124 ms (0.05%)
ddtrace.internal.datadog.profiling.crashtracker._crashtracker 0.124 ms (0.05%)
ddtrace.internal.datadog.profiling._types 0.124 ms (0.05%)
ddtrace.settings.profiling 0.103 ms (0.04%)
ddtrace.vendor.psutil 0.103 ms (0.04%)
ddtrace.internal.flare.flare 0.075 ms (0.03%)
ddtrace.internal.flare 0.075 ms (0.03%)
ddtrace._trace.trace_handlers 0.154 ms (0.06%)
ddtrace.appsec._common_module_patches 0.106 ms (0.04%)
ddtrace.appsec._asm_request_context 0.106 ms (0.04%)
ddtrace.appsec._utils 0.106 ms (0.04%)
ddtrace 0.832 ms (0.34%)
ddtrace.settings._config 0.111 ms (0.05%)
ddtrace.internal.gitmetadata 0.111 ms (0.05%)

@mabdinur mabdinur force-pushed the chore/apm-tracing-rc branch from 9774d1f to da8200b Compare April 1, 2025 16:30
@P403n1x87 P403n1x87 force-pushed the chore/apm-tracing-rc branch from da8200b to 252be2c Compare April 2, 2025 08:08
@P403n1x87 P403n1x87 requested a review from a team as a code owner April 2, 2025 08:39
@P403n1x87 P403n1x87 merged commit 7955b92 into main Apr 2, 2025
683 checks passed
@P403n1x87 P403n1x87 deleted the chore/apm-tracing-rc branch April 2, 2025 13:50
chojomok pushed a commit that referenced this pull request Apr 7, 2025
We introduce the APM_TRACING remote configuration product that allows
dispatching remote configuration to the library for remote enablement/
configuration of library components and features.

## 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants