fix(lib-injection): do not import user installed ddtrace

[brettlangdon]

One of the checks we do when auto-injecting into a process is to see if the user already has ddtrace installed. If we do, then we will not inject into it.

However, the way we check if the user has ddtrace installed is we import ddtrace. This may have an unintended consequence of importing ddtrace into a process which otherwise wouldn't normally.

The impact of importing ddtrace should be fairly low, since we won't instrument anything, but it is better to avoid the import overhead if the user wasn't going to import it anyways.

Instead we move to using importlib.util.find_spec to search for the module on the sys.meta_path. The downside to doing this is we won't know the installed ddtrace version.

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]

CODEOWNERS have been resolved as:

releasenotes/notes/fix-lib-injection-import-f0e5e2715854d3e0.yaml       @DataDog/apm-python
lib-injection/sources/sitecustomize.py                                  @DataDog/apm-core-python

[P403n1x87]

The downside to doing this is we won't know the installed ddtrace version.

But once we have the spec we could potentially get hold of the _version.py source and retrieve the version from there, no?

[brettlangdon]

The downside to doing this is we won't know the installed ddtrace version.

But once we have the spec we could potentially get hold of the _version.py source and retrieve the version from there, no?

Yes, but is it worth the overhead to do that? 🤷🏻 not sure it gets us anything else of value, especially since this is only a debug log, at least origin gives us some hints as to what is up / where the user has it installed.

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-11-08 00:08:41

Comparing candidate commit 5d7dbfe in PR branch brettlangdon/fix.ddtrace.import with baseline commit 8230f9e in branch main.

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

[avara1986]

Wouldn't we need unit or regression tests to validate this change?

[brettlangdon]

Wouldn't we need unit or regression tests to validate this change?

Yes, but I have no idea where this regression test should go. I didn't see any lib-injection tests in this repo, so probably needs to be a system test change?

I can take a quick look into that today, I would like a regression test.

[brettlangdon]

/merge

[dd-devflow]

Devflow running: /merge

View all feedbacks in Devflow UI.

---

2024-11-07 19:41:19 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

---

2024-11-07 19:49:30 UTC ℹ️ MergeQueue: pull request added to the queue

The median merge time in main is 2m.

---

2024-11-07 20:12:53 UTC ❌ MergeQueue: The checks failed on this merge request

Tests failed on this commit 85dccb2:

• test

What to do next?

• Investigate the failures and when ready, re-add your pull request to the queue!
• Any question, go check the FAQ.

[brettlangdon]

/merge

[dd-devflow]

Devflow running: /merge

View all feedbacks in Devflow UI.

---

2024-11-07 23:29:25 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

---

2024-11-08 00:31:06 UTC ℹ️ MergeQueue: pull request added to the queue

The median merge time in main is 34m.

---

2024-11-08 01:06:36 UTC ❌ MergeQueue: The checks failed on this merge request

Tests failed on this commit c3e9acb:

• test

What to do next?

• Investigate the failures and when ready, re-add your pull request to the queue!
• Any question, go check the FAQ.