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.

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

chore(di): trigger probes #10942

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

chore(di): trigger probes #10942

wants to merge 11 commits into from

Conversation

P403n1x87
Copy link
Contributor

@P403n1x87 P403n1x87 commented Oct 4, 2024

We implement trigger probes. These allows triggering the capture of debug information along a trace, ensuring all the relevant probes are also triggered.

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 changelog/no-changelog A changelog entry is not required for this PR. Dynamic Instrumentation Dynamic Instrumentation/Live Debugger labels Oct 4, 2024
@P403n1x87 P403n1x87 self-assigned this Oct 4, 2024
Copy link
Contributor

github-actions bot commented Oct 4, 2024

CODEOWNERS have been resolved as:

ddtrace/debugging/_live.py                                              @DataDog/debugger-python
ddtrace/debugging/_products/live_debugger.py                            @DataDog/debugger-python
ddtrace/debugging/_session.py                                           @DataDog/debugger-python
ddtrace/debugging/_signal/trigger.py                                    @DataDog/debugger-python
ddtrace/settings/live_debugging.py                                      @DataDog/apm-core-python
tests/debugging/live/__init__.py                                        @DataDog/debugger-python
tests/debugging/live/test_live_debugger.py                              @DataDog/debugger-python
tests/debugging/test_session.py                                         @DataDog/debugger-python
ddtrace/_trace/context.py                                               @DataDog/apm-sdk-api-python
ddtrace/contrib/trace_utils.py                                          @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/debugging/_exception/replay.py                                  @DataDog/debugger-python
ddtrace/debugging/_origin/span.py                                       @DataDog/debugger-python
ddtrace/debugging/_probe/model.py                                       @DataDog/debugger-python
ddtrace/debugging/_probe/remoteconfig.py                                @DataDog/debugger-python
ddtrace/debugging/_signal/__init__.py                                   @DataDog/debugger-python
ddtrace/debugging/_signal/model.py                                      @DataDog/debugger-python
ddtrace/debugging/_uploader.py                                          @DataDog/debugger-python
ddtrace/propagation/http.py                                             @DataDog/apm-sdk-api-python
docs/configuration.rst                                                  @DataDog/python-guild
pyproject.toml                                                          @DataDog/python-guild
tests/debugging/conftest.py                                             @DataDog/debugger-python
tests/debugging/mocking.py                                              @DataDog/debugger-python
tests/debugging/origin/test_span.py                                     @DataDog/debugger-python
tests/debugging/signal/test_collector.py                                @DataDog/debugger-python
tests/debugging/test_debugger_span_decoration.py                        @DataDog/debugger-python
tests/debugging/utils.py                                                @DataDog/debugger-python
tests/submod/traced_stuff.py                                            @DataDog/apm-core-python
tests/tracer/test_context.py                                            @DataDog/apm-sdk-api-python

@pr-commenter
Copy link

pr-commenter bot commented Oct 4, 2024

Benchmarks

Benchmark execution time: 2025-01-09 11:17:08

Comparing candidate commit 2d85475 in PR branch chore/trigger-probe with baseline commit 6bfe77e in branch main.

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

scenario:httppropagationextract-all_styles_all_headers

  • 🟩 execution_time [-5.733µs; -5.398µs] or [-8.568%; -8.068%]

@P403n1x87 P403n1x87 force-pushed the chore/trigger-probe branch from a804567 to 766843f Compare October 5, 2024 09:59
@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Oct 5, 2024

Datadog Report

Branch report: chore/trigger-probe
Commit report: f108c5a
Test service: dd-trace-py

✅ 0 Failed, 1468 Passed, 0 Skipped, 22m 20.08s Total duration (14m 7.33s time saved)

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.

very cool progress, thank you

ddtrace/contrib/trace_utils.py Show resolved Hide resolved
ddtrace/debugging/_origin/span.py Outdated Show resolved Hide resolved
ddtrace/debugging/_session.py Outdated Show resolved Hide resolved
ddtrace/debugging/_session.py Outdated Show resolved Hide resolved
ddtrace/debugging/_session.py Outdated Show resolved Hide resolved
ddtrace/debugging/_live.py Outdated Show resolved Hide resolved
@P403n1x87 P403n1x87 force-pushed the chore/trigger-probe branch 3 times, most recently from 965b6e6 to 59e7d30 Compare October 19, 2024 19:56
@P403n1x87 P403n1x87 force-pushed the chore/trigger-probe branch 3 times, most recently from f8ea66c to 07f7061 Compare October 31, 2024 11:46
We implement trigger probes. These allows triggering the capture of
debug information along a trace, ensuring all the relevant probes are
also triggered.
@P403n1x87 P403n1x87 force-pushed the chore/trigger-probe branch from 07f7061 to a6af837 Compare November 6, 2024 16:13
@P403n1x87 P403n1x87 marked this pull request as ready for review December 19, 2024 11:28
@P403n1x87 P403n1x87 requested review from a team as code owners December 19, 2024 11:28
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.

Thank you!

ddtrace/debugging/_origin/span.py Show resolved Hide resolved
ddtrace/debugging/_probe/model.py Outdated Show resolved Hide resolved
ddtrace/debugging/_session.py Outdated Show resolved Hide resolved
ddtrace/debugging/_signal/model.py Show resolved Hide resolved
ddtrace/debugging/_signal/trigger.py Outdated Show resolved Hide resolved
ddtrace/settings/code_origin.py Outdated Show resolved Hide resolved
tests/debugging/test_session.py Outdated Show resolved Hide resolved
tests/debugging/test_session.py Outdated Show resolved Hide resolved
@P403n1x87 P403n1x87 force-pushed the chore/trigger-probe branch 2 times, most recently from 89c83aa to 118ecb1 Compare January 6, 2025 11:21
@P403n1x87 P403n1x87 force-pushed the chore/trigger-probe branch from 118ecb1 to 346a578 Compare January 6, 2025 14:57
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 the only thing that left is probe budget for probe with session

except KeyError:
# No snapshot was created
return
self.set("start_time", monotonic_ns())
Copy link
Contributor

Choose a reason for hiding this comment

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

is now happen at the end of the signal. maybe this line should go back to enter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah of course 🤦

"a,b",
[
("session1:1.session2:2", "session1.session2:2"),
("session1:0.session2:2", "session1:0.session2:2"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think those test could benefit with validate that parsed session are correct as well. that we decode sesion id and level as expected and we got sessions.

second, can you add a test with just session1.session2 to ensure default level works?

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. Dynamic Instrumentation Dynamic Instrumentation/Live Debugger
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants