-
Notifications
You must be signed in to change notification settings - Fork 469
chore: use static versions instead of setuptools-scm #15245
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
Conversation
|
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 253 ± 3 ms. The average import time from base is: 265 ± 4 ms. The import time difference between this PR and base is: -12.7 ± 0.2 ms. Import time breakdownThe following import paths have disappeared:
|
BenchmarksBenchmark execution time: 2025-11-13 19:45:45 Comparing candidate commit 9ef9050 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 365 metrics, 3 unstable metrics. |
Performance SLOsComparing candidate brettlangdon/static.versions (659766c) with baseline main (09f2961) 🟡 Near SLO Breach (2 suites)🟡 iastpropagation - 8/8✅ no-propagationTime: ✅ 48.709µs (SLO: <60.000µs 📉 -18.8%) vs baseline: +0.1% Memory: ✅ 39.479MB (SLO: <40.500MB -2.5%) vs baseline: +4.9% ✅ propagation_enabledTime: ✅ 166.218µs (SLO: <190.000µs 📉 -12.5%) vs baseline: ~same Memory: ✅ 39.459MB (SLO: <40.000MB 🟡 -1.4%) vs baseline: +4.6% ✅ propagation_enabled_100Time: ✅ 1.861ms (SLO: <2.300ms 📉 -19.1%) vs baseline: ~same Memory: ✅ 39.558MB (SLO: <40.000MB 🟡 -1.1%) vs baseline: +5.0% ✅ propagation_enabled_1000Time: ✅ 32.079ms (SLO: <34.550ms -7.2%) vs baseline: ~same Memory: ✅ 39.400MB (SLO: <40.000MB 🟡 -1.5%) vs baseline: +4.6% 🟡 otelspan - 22/22✅ add-eventTime: ✅ 38.604ms (SLO: <47.150ms 📉 -18.1%) vs baseline: +0.1% Memory: ✅ 39.064MB (SLO: <47.000MB 📉 -16.9%) vs baseline: +4.7% ✅ add-metricsTime: ✅ 260.026ms (SLO: <344.800ms 📉 -24.6%) vs baseline: +1.9% Memory: ✅ 43.391MB (SLO: <47.500MB -8.6%) vs baseline: +5.0% ✅ add-tagsTime: ✅ 315.524ms (SLO: <321.000ms 🟡 -1.7%) vs baseline: +0.7% Memory: ✅ 43.395MB (SLO: <47.500MB -8.6%) vs baseline: +4.9% ✅ get-contextTime: ✅ 78.400ms (SLO: <92.350ms 📉 -15.1%) vs baseline: ~same Memory: ✅ 39.420MB (SLO: <46.500MB 📉 -15.2%) vs baseline: +4.8% ✅ is-recordingTime: ✅ 36.150ms (SLO: <44.500ms 📉 -18.8%) vs baseline: -0.3% Memory: ✅ 38.985MB (SLO: <47.500MB 📉 -17.9%) vs baseline: +4.9% ✅ record-exceptionTime: ✅ 57.217ms (SLO: <67.650ms 📉 -15.4%) vs baseline: -0.3% Memory: ✅ 39.637MB (SLO: <47.000MB 📉 -15.7%) vs baseline: +5.1% ✅ set-statusTime: ✅ 42.799ms (SLO: <50.400ms 📉 -15.1%) vs baseline: +0.7% Memory: ✅ 38.985MB (SLO: <47.000MB 📉 -17.1%) vs baseline: +5.3% ✅ startTime: ✅ 35.457ms (SLO: <43.450ms 📉 -18.4%) vs baseline: +0.1% Memory: ✅ 39.062MB (SLO: <47.000MB 📉 -16.9%) vs baseline: +5.3% ✅ start-finishTime: ✅ 81.922ms (SLO: <88.000ms -6.9%) vs baseline: -0.6% Memory: ✅ 36.746MB (SLO: <46.500MB 📉 -21.0%) vs baseline: +5.3% ✅ start-finish-telemetryTime: ✅ 83.679ms (SLO: <89.000ms -6.0%) vs baseline: -0.2% Memory: ✅ 36.785MB (SLO: <46.500MB 📉 -20.9%) vs baseline: +5.3% ✅ update-nameTime: ✅ 36.791ms (SLO: <45.150ms 📉 -18.5%) vs baseline: -0.9% Memory: ✅ 38.947MB (SLO: <47.000MB 📉 -17.1%) vs baseline: +4.5%
|
emmettbutler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. My only concern is the fact that there are currently three places where the version information is duplicated (pyproject and two lines in version.py). With importlib.metadata, it's possible to eliminate the ones in version.py.
You've already got the script checking consistency between these duplicated pieces of information. To merge with the duplication intact, I think a CI check that runs that script and fails on mismatches would be sufficient.
Circular import analysis🚨 New circular imports detected 🚨The following circular imports among modules have been detected on this PR, when compared to the base branch: Please consider refactoring your changes in accordance to the Separation of Concerns principle. |
|
ah, we need to keep When I bring it back, I'll be sure to leave a note on why it is that way |
emmettbutler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice approach with the version checker job. The wheel filename is a useful indicator to check.
We also added a CI job to ensure the version in This does mean we need to ensure our git tags are PEP 440 tags, I think this only impacts |
Description
APMLP-587
Remove
setuptools-scmand rely on statically defined versions inpyproject.tomlandddtrace/version.py.A lot of the changes are moving components away from using
ddtrace.version.get_version()to compute the version vs using the statically definedddtrace.__version__/ddtrace.version.__version__Testing
Risks
Additional Notes
We are dropping the version specific public s3 folders. This is because the static versions mean we are likely to easily overwrite the contents of a bucket on PRs/feature branches/new runs on main or release branches/etc. Removing them and only relying on the pipeline id is the safest way to not have any conflicts between versions and files uploaded.
This PR also adds a helper script
scripts/verify-package-version: