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

feat(onboarding): early exit conditions in lib-injection [backport 2.10] #9512

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

github-actions[bot]
Copy link

@github-actions github-actions bot commented Jun 11, 2024

Backport 0c38e09 from #9323 to 2.10.

This pull request adds "guardrails" to the "library injection" process. These are early exit conditions from the instrumentation process intended to avoid sending any traces when undefined behavior is likely. The code makes this determination on the basis of software versions present in the application environment, both of Python packages and the Python runtime itself.

The biggest risk here is that instrumentation is disabled when it's not intended to be. I think existing tests in tests/lib-injection cover this pretty well. There's a new test added that verifies instrumentation was cancelled when an unsupported package version is present.

Contains changes from #9418
Related RFC: "[RFC] One Step Guardrails"

Checklist

  • minimum package version checks
  • Testing
  • replace envvars with inject_force
  • figure out what to use instead of pkg_resources
  • replace local file path with DD_TELEMETRY_FORWARDER_PATH
  • Change(s) are motivated and described in the PR description
  • Testing strategy is described if automated tests are not included in the PR
  • Risks are described (performance impact, potential for breakage, maintainability)
  • Change is maintainable (easy to change, telemetry, documentation)
  • Library release note guidelines are followed or label changelog/no-changelog is set
  • Documentation is included (in-code, generated user docs, public corp docs)
  • Backport labels are set (if applicable)
  • If this PR changes the public interface, I've notified @DataDog/apm-tees.

Reviewer Checklist

  • Title is accurate
  • All changes are related to the pull request's stated goal
  • Description motivates each change
  • Avoids breaking API changes
  • Testing strategy adequately addresses listed risks
  • Change is maintainable (easy to change, telemetry, documentation)
  • Release note makes sense to a user of the library
  • 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

This pull request adds "guardrails" to the "library injection" process.
These are early exit conditions from the instrumentation process
intended to avoid sending any traces when undefined behavior is likely.
The code makes this determination on the basis of software versions
present in the application environment, both of Python packages and the
Python runtime itself.

The biggest risk here is that instrumentation is disabled when it's not
intended to be. I think existing tests in `tests/lib-injection` cover
this pretty well. There's a new test added that verifies instrumentation
was cancelled when an unsupported package version is present.

Contains changes from #9418
Related RFC: "[RFC] One Step Guardrails"

## Checklist

- [x] minimum package version checks
- [x] Testing
- [x] replace envvars with inject_force
- [x] figure out what to use instead of pkg_resources
- [x] replace local file path with `DD_TELEMETRY_FORWARDER_PATH`
- [x] Change(s) are motivated and described in the PR description
- [x] Testing strategy is described if automated tests are not included
in the PR
- [x] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [x] 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)

---------

Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
Co-authored-by: Emmett Butler <emmett.butler321@gmail.com>
(cherry picked from commit 0c38e09)
@github-actions github-actions bot requested review from a team as code owners June 11, 2024 14:09
@emmettbutler emmettbutler enabled auto-merge (squash) June 11, 2024 14:12
@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Jun 11, 2024

Datadog Report

Branch report: backport-9323-to-2.10
Commit report: 34bdd36
Test service: dd-trace-py

✅ 0 Failed, 147399 Passed, 31488 Skipped, 9h 31m 55.96s Total duration (1h 57m 27.03s time saved)
❄️ 1 New Flaky

New Flaky Tests (1)

  • test_accuracy - test_accuracy.py - Last Failure

    Expand for error
     Expected status 0, got 1.
     === Captured STDOUT ===
     === End of captured STDOUT ===
     === Captured STDERR ===
     Traceback (most recent call last):
       File "tests/profiling/test_accuracy.py", line 96, in <module>
         assert almost_equal(total_time(time_spent_ns, "spend_4"), 4e9)
     AssertionError
     === End of captured STDERR ===
    

@emmettbutler emmettbutler added the changelog/no-changelog A changelog entry is not required for this PR. label Jun 11, 2024
@emmettbutler emmettbutler merged commit db6fe36 into 2.10 Jun 11, 2024
106 checks passed
@emmettbutler emmettbutler deleted the backport-9323-to-2.10 branch June 11, 2024 14:45
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.

None yet

2 participants