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: put ddtrace site-packages on the end of the pythonpath [APMS-12275] #9496

Merged
merged 85 commits into from
Jun 11, 2024

Conversation

emmettbutler
Copy link
Collaborator

@emmettbutler emmettbutler commented Jun 7, 2024

This change adjusts single-step instrumentation to put ddtrace's customizations on the end of the pythonpath rather than the beginning. This makes it much less likely that instrumentation will corrupt the app by causing different versions of requirements to be used. Because the injection guardrails are now handling the task of ensuring compatibility between ddtrace and app requirements, which prepending to the pythonpath was a workaround for, this new less destructive approach is available.

Checklist

  • 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.

emmettbutler and others added 3 commits June 4, 2024 11:45
Co-authored-by: Zachary Groves <32471391+ZStriker19@users.noreply.github.com>
…ginning

to avoid corrupting the app by installing different versions of its dependencies
@emmettbutler emmettbutler requested a review from a team as a code owner June 7, 2024 14:12
@emmettbutler emmettbutler requested review from brettlangdon and removed request for a team June 7, 2024 14:12
@emmettbutler emmettbutler marked this pull request as draft June 7, 2024 14:13
@datadog-dd-trace-py-rkomorn
Copy link

Datadog Report

Branch report: emmett.butler/end-pypath
Commit report: 14abb42
Test service: dd-trace-py

❌ 16 Failed (0 Known Flaky), 631 Passed, 5340 Skipped, 3m 18.17s Total duration (11m 33.96s time saved)

❌ Failed Tests (16)

This report shows up to 5 failed tests.

  • test_and_emit_get_version - test_httpx_patch.py - Details

    Expand for error
     Subprocess Test "python -m unittest tests.contrib.httpx.test_httpx_patch.TestHttpxPatch.test_and_emit_get_version" Failed (exit code 1):
     E
     ======================================================================
     ERROR: test_and_emit_get_version (tests.contrib.httpx.test_httpx_patch.TestHttpxPatch.test_and_emit_get_version)
     Each contrib module should implement a get_version() function. This function is used for
     ----------------------------------------------------------------------
     Traceback (most recent call last):
       File "/root/project/tests/contrib/patch.py", line 788, in test_and_emit_get_version
         emit_integration_and_version_to_test_agent(
       File "/root/project/tests/contrib/patch.py", line 120, in emit_integration_and_version_to_test_agent
     ...
    
  • test_and_emit_get_version - test_httpx_patch.py - Details

    Expand for error
     Subprocess Test "python -m unittest tests.contrib.httpx.test_httpx_patch.TestHttpxPatch.test_and_emit_get_version" Failed (exit code 1):
     E
     ======================================================================
     ERROR: test_and_emit_get_version (tests.contrib.httpx.test_httpx_patch.TestHttpxPatch)
     Each contrib module should implement a get_version() function. This function is used for
     ----------------------------------------------------------------------
     Traceback (most recent call last):
       File "/root/project/tests/contrib/patch.py", line 789, in test_and_emit_get_version
         self.__integration_name__, version, module_name=self.__module_name__
       File "/root/project/tests/contrib/patch.py", line 120, in emit_integration_and_version_to_test_agent
     ...
    
  • test_configured_format - test_loguru_logging.py - Details

    Expand for error
     STDERR: Expected [] got [b'2024-06-07 14:20:53,213 ERROR [ddtrace.internal.writer.writer] [writer.py:399] [dd.service=logging dd.env=global.env dd.version=global.version dd.trace_id=0 dd.span_id=0] - failed to send, dropping 1 traces to intake at http://localhost:9126/v0.5/traces after 3 retries\n']
    
  • test_log_DD_TAGS - test_loguru_logging.py - Details

    Expand for error
     STDERR: Expected [] got [b'2024-06-07 14:20:51,134 ERROR [ddtrace.internal.writer.writer] [writer.py:399] [dd.service=ddtagservice dd.env=ddenv dd.version=ddversion dd.trace_id=0 dd.span_id=0] - failed to send, dropping 1 traces to intake at http://localhost:9126/v0.5/traces after 3 retries\n']
    
  • test_log_trace_128bit_trace_ids - test_loguru_logging.py - Details

    Expand for error
     STDERR: Expected [] got [b'2024-06-07 14:20:52,136 ERROR [ddtrace.internal.writer.writer] [writer.py:399] [dd.service=logging dd.env=global.env dd.version=global.version dd.trace_id=0 dd.span_id=0] - failed to send, dropping 1 traces to intake at http://localhost:9126/v0.5/traces after 3 retries\n']
    

@emmettbutler emmettbutler changed the title chore: put ddtrace bootstrap on the end of the pythonpath chore: put ddtrace site-packages on the end of the pythonpath Jun 7, 2024
Base automatically changed from zachg/guardrails_poc to main June 11, 2024 14:08
@emmettbutler emmettbutler changed the title chore: put ddtrace site-packages on the end of the pythonpath chore: put ddtrace site-packages on the end of the pythonpath [APMS-12275] Jun 11, 2024
@emmettbutler emmettbutler added the changelog/no-changelog A changelog entry is not required for this PR. label Jun 11, 2024
@emmettbutler emmettbutler marked this pull request as ready for review June 11, 2024 15:30
@pr-commenter
Copy link

pr-commenter bot commented Jun 11, 2024

Benchmarks

Benchmark execution time: 2024-06-11 15:39:30

Comparing candidate commit 4c1871c in PR branch emmett.butler/end-pypath with baseline commit 0c38e09 in branch main.

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

@emmettbutler emmettbutler enabled auto-merge (squash) June 11, 2024 15:44
@emmettbutler emmettbutler merged commit 4da8ed6 into main Jun 11, 2024
68 checks passed
@emmettbutler emmettbutler deleted the emmett.butler/end-pypath branch June 11, 2024 16:15
github-actions bot pushed a commit that referenced this pull request Jun 12, 2024
…2275] (#9496)

This change adjusts single-step instrumentation to put ddtrace's
customizations on the end of the pythonpath rather than the beginning.
This makes it much less likely that instrumentation will corrupt the app
by causing different versions of requirements to be used. Because the
injection guardrails are now handling the task of ensuring compatibility
between ddtrace and app requirements, which prepending to the pythonpath
was a workaround for, this new less destructive approach is available.

## Checklist

- [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`.

---------

Co-authored-by: ZStriker19 <zach.groves@datadoghq.com>
Co-authored-by: Zachary Groves <32471391+ZStriker19@users.noreply.github.com>
(cherry picked from commit 4da8ed6)
emmettbutler added a commit that referenced this pull request Jun 12, 2024
…2275] [backport 2.10] (#9532)

Backport 4da8ed6 from #9496 to 2.10.

This change adjusts single-step instrumentation to put ddtrace's
customizations on the end of the pythonpath rather than the beginning.
This makes it much less likely that instrumentation will corrupt the app
by causing different versions of requirements to be used. Because the
injection guardrails are now handling the task of ensuring compatibility
between ddtrace and app requirements, which prepending to the pythonpath
was a workaround for, this new less destructive approach is available.

## Checklist

- [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`.

Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.10 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.

3 participants