Skip to content

Conversation

@maxday
Copy link
Contributor

@maxday maxday commented Aug 25, 2021

Description

pkg_resources is quite heavy and slows down cold start when dd-trace-py is used in a AWS Lambda environment.
This PR uses the write_to mecanism to write this version to a file (and defaulting to pkg_resources in case of an unexpected error)

Checklist

  • Added to the correct milestone.
  • Tests provided or description of manual testing performed is included in the code or PR.
  • Library documentation is updated.
  • Corp site documentation is updated (link to the PR).

@maxday maxday requested a review from a team as a code owner August 25, 2021 17:26
Copy link
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

big picture looks good

Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

@maxday @brettlangdon what are the plans for how/when the _version file will be generated?

@maxday
Copy link
Contributor Author

maxday commented Aug 25, 2021

@maxday @brettlangdon what are the plans for how/when the _version file will be generated?

config.py is doing it thanks to write_to (the file was not committed when you made the comment)

@brettlangdon
Copy link
Member

@maxday do you have performance/profile results with this change? how much of an improvement does it make?

@maxday
Copy link
Contributor Author

maxday commented Aug 25, 2021

@maxday do you have performance/profile results with this change? how much of an improvement does it make?

@brettlangdon Yes : over 10 runs, result is the average

Before : 853ms
With this PR : 665ms

This is a massive win for our cold start duration.

(Full disclosure, I had to comment the import in debug.py as well since it also imports pkg_resources)
EDIT : no need to edit debug.py manually anymore since #2803 has been merged

@maxday maxday changed the title [WIP] Change version detection mecanism Change version detection mecanism Aug 25, 2021
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

awesome! lgtm with just a tweak to the test cases

"Programming Language :: Python :: 3.9",
],
use_scm_version=True,
use_scm_version={"write_to": "ddtrace/_version.py"},
Copy link
Member

Choose a reason for hiding this comment

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

oh sweet - that was easy!

@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2021

Codecov Report

Merging #2802 (a436d4a) into master (d6bbd13) will increase coverage by 0.00%.
The diff coverage is 90.69%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2802   +/-   ##
=======================================
  Coverage   84.87%   84.87%           
=======================================
  Files         592      595    +3     
  Lines       43384    43421   +37     
=======================================
+ Hits        36820    36855   +35     
- Misses       6564     6566    +2     
Impacted Files Coverage Δ
ddtrace/internal/debug.py 35.29% <0.00%> (-1.97%) ⬇️
tests/tracer/test_version.py 89.65% <89.65%> (ø)
ddtrace/__init__.py 100.00% <100.00%> (+11.11%) ⬆️
ddtrace/version.py 100.00% <100.00%> (ø)
tests/tracer/_version.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e8581f...a436d4a. Read the comment docs.

brettlangdon
brettlangdon previously approved these changes Aug 25, 2021
@brettlangdon brettlangdon added the changelog/no-changelog A changelog entry is not required for this PR. label Aug 25, 2021
@maxday
Copy link
Contributor Author

maxday commented Aug 25, 2021

@mergify mergify bot merged commit d995d1d into master Aug 26, 2021
@mergify mergify bot deleted the maxday/change-version-detection-mecanism branch August 26, 2021 07:52
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.

6 participants