fix: pin importlib to 6.5.0 #8075

juanjux · 2024-01-11T15:56:13Z

Description

Pin importlib to 6.5.0 to avoid issue Uncaught Exception when reading Egg information python/importlib_metadata#455
Fix a potential unbound local error.

Fixes #8068

Checklist

Change(s) are motivated and described in the PR description.
Testing strategy is described if automated tests are not included in the PR.
Risk is outlined (performance impact, potential for breakage, maintainability, etc).
Change is maintainable (easy to change, telemetry, documentation).
Library release note guidelines are followed. If no release note is required, add label changelog/no-changelog.
Documentation is included (in-code, generated user docs, public corp docs).
Backport labels are set (if applicable)

Reviewer Checklist

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

gnufede · 2024-01-11T16:43:37Z

Doesn't the dependency need to be pinned in setup.py as well?
https://github.com/DataDog/dd-trace-py/blob/main/setup.py#L579

stobias123 · 2024-01-11T22:50:24Z

I just force updated my importlib to this version and still get the error.

python 3.10.13

ddtrace==2.4.0

juanjux · 2024-01-12T10:20:15Z

@stobias123 you have to pin importlib_metadata, not importlib since the error is with the former.

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

P403n1x87 · 2024-01-12T12:21:15Z

🤔 Pinning the version of a potentially common dependency concerns me a bit. @majorgreys I think we had issues with a similar approach in the past. Is that correct?

pr-commenter · 2024-01-12T13:14:17Z

Benchmarks

Benchmark execution time: 2024-01-25 20:49:54

Comparing candidate commit d8fcb8f in PR branch juanjux/pin-importlib with baseline commit 0720c68 in branch main.

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

scenario:span-add-metrics

🟥 max_rss_usage [+21.429MB; +21.564MB] or [+52.913%; +53.247%]

pyproject.toml

ddtrace/internal/packages.py

ddtrace/internal/utils/cache.py

Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com>

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

releasenotes/notes/pin-importlib-to-6.5.0-to-avoid-importlib-issue-455-d098894f7a29591b.yaml

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

## Description - Pin importlib to 6.5.0 to avoid issue python/importlib_metadata#455 - Fix a potential unbound local error. Fixes #8068 ## 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] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [X] Change is maintainable (easy to change, telemetry, documentation). - [X] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [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)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly 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) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. --------- Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com> Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com> Co-authored-by: Zachary Groves <32471391+ZStriker19@users.noreply.github.com> (cherry picked from commit c8d5e9f)

Backport c8d5e9f from #8075 to 2.5. ## Description - Pin importlib to 6.5.0 to avoid issue python/importlib_metadata#455 - Fix a potential unbound local error. Fixes #8068 ## 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] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [X] Change is maintainable (easy to change, telemetry, documentation). - [X] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [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)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly 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) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. Co-authored-by: Juanjo Alvarez Martinez <juanjo.alvarezmartinez@datadoghq.com>

Backport c8d5e9f from #8075 to 2.3. ## Description - Pin importlib to 6.5.0 to avoid issue python/importlib_metadata#455 - Fix a potential unbound local error. Fixes #8068 ## 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] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [X] Change is maintainable (easy to change, telemetry, documentation). - [X] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [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)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly 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) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. Co-authored-by: Juanjo Alvarez Martinez <juanjo.alvarezmartinez@datadoghq.com>

Backport c8d5e9f from #8075 to 2.4. ## Description - Pin importlib to 6.5.0 to avoid issue python/importlib_metadata#455 - Fix a potential unbound local error. Fixes #8068 ## 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] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [X] Change is maintainable (easy to change, telemetry, documentation). - [X] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [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)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly 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) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. Co-authored-by: Juanjo Alvarez Martinez <juanjo.alvarezmartinez@datadoghq.com> Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>

juanjux added 3 commits January 11, 2024 16:50

Pin importlib_metadata version

d0463d7

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

changelog

75f8d4f

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

Fix unbound local

ec978d7

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

juanjux marked this pull request as ready for review January 11, 2024 15:56

juanjux requested review from a team as code owners January 11, 2024 15:56

juanjux requested review from majorgreys, gnufede and ZStriker19 January 11, 2024 15:56

juanjux added ASM Application Security Monitoring backport 2.4 labels Jan 11, 2024

juanjux self-assigned this Jan 11, 2024

juanjux mentioned this pull request Jan 11, 2024

fix: workaround potencial valueerror form importlib_metadata #8069

Closed

18 tasks

gnufede approved these changes Jan 11, 2024

View reviewed changes

juanjux added 2 commits January 12, 2024 13:03

Add ValueError handler to not stop catching paths

4c24d27

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

Also pin importlib_metadata in setup.py

ec1775e

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

majorgreys requested changes Jan 12, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

ddtrace/internal/packages.py Outdated Show resolved Hide resolved

ddtrace/internal/utils/cache.py Outdated Show resolved Hide resolved

juanjux and others added 3 commits January 12, 2024 16:21

Update pyproject.toml

ec59e3e

Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com>

update setup dependency

be49b29

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

Changes from review feedback

9dfee41

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

juanjux requested a review from majorgreys January 15, 2024 08:43

juanjux added 3 commits January 15, 2024 09:43

Merge branch 'main' into juanjux/pin-importlib

0dc6a55

Merge branch 'main' into juanjux/pin-importlib

5763b42

Merge branch 'main' into juanjux/pin-importlib

aa3401d

juanjux added 2 commits January 16, 2024 10:49

Merge branch 'main' into juanjux/pin-importlib

575c142

Merge branch 'main' into juanjux/pin-importlib

651d6f0

majorgreys reviewed Jan 17, 2024

View reviewed changes

releasenotes/notes/pin-importlib-to-6.5.0-to-avoid-importlib-issue-455-d098894f7a29591b.yaml Outdated Show resolved Hide resolved

juanjux added 2 commits January 18, 2024 09:38

Merge branch 'main' into juanjux/pin-importlib

e23f5ec

Better descriptive message

c397b5a

Signed-off-by: Juanjo Alvarez <juanjo.alvarezmartinez@datadoghq.com>

econno11y mentioned this pull request Jan 22, 2024

'function' object has no attribute '__callonce_result__' #8068

Closed

gnufede added backport 2.3 backport 2.5 labels Jan 23, 2024

ZStriker19 approved these changes Jan 25, 2024

View reviewed changes

Merge branch 'main' into juanjux/pin-importlib

d8fcb8f

majorgreys approved these changes Jan 25, 2024

View reviewed changes

majorgreys merged commit c8d5e9f into main Jan 25, 2024
143 checks passed

majorgreys deleted the juanjux/pin-importlib branch January 25, 2024 20:53

github-actions bot mentioned this pull request Jan 25, 2024

fix: pin importlib to 6.5.0 [backport 2.3] #8194

Merged

18 tasks

github-actions bot mentioned this pull request Jan 25, 2024

fix: pin importlib to 6.5.0 [backport 2.4] #8195

Merged

18 tasks

github-actions bot mentioned this pull request Jan 25, 2024

fix: pin importlib to 6.5.0 [backport 2.5] #8196

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: pin importlib to 6.5.0 #8075

fix: pin importlib to 6.5.0 #8075

juanjux commented Jan 11, 2024 •

edited by gnufede

gnufede commented Jan 11, 2024

stobias123 commented Jan 11, 2024 •

edited

juanjux commented Jan 12, 2024 •

edited

P403n1x87 commented Jan 12, 2024

pr-commenter bot commented Jan 12, 2024 •

edited

fix: pin importlib to 6.5.0 #8075

fix: pin importlib to 6.5.0 #8075

Conversation

juanjux commented Jan 11, 2024 • edited by gnufede

Description

Checklist

Reviewer Checklist

gnufede commented Jan 11, 2024

stobias123 commented Jan 11, 2024 • edited

juanjux commented Jan 12, 2024 • edited

P403n1x87 commented Jan 12, 2024

pr-commenter bot commented Jan 12, 2024 • edited

Benchmarks

scenario:span-add-metrics

juanjux commented Jan 11, 2024 •

edited by gnufede

stobias123 commented Jan 11, 2024 •

edited

juanjux commented Jan 12, 2024 •

edited

pr-commenter bot commented Jan 12, 2024 •

edited