dd-trace-py
dd-trace-py copied to clipboard
chore(tracer): use simple queue file for extra service names instead of multiprocessing
To prevent further problems with multiprocessing in config:
- remove multiprocessing queue from main config file
- add a new File_Queue using low level locking to write into temporary file
- Use the new queue to exchange messages between multiple processes for extra service names
- update tests
- added a new unit-tests workflow on github to launch any tests on linux(x86), macos(arm64) and windows(x86). (For now, only the test relevant to that PR is used).
This is an alternative solution to https://github.com/DataDog/dd-trace-py/pull/9626 ensuring that we still report extra service names.
Checklist
- [x] The PR description includes an overview of the change
- [x] The PR description articulates the motivation for the change
- [x] The change includes tests OR the PR description describes a testing strategy
- [x] The PR description notes risks associated with the change, if any
- [x] Newly-added code is easy to change
- [x] The change follows the library release note guidelines
- [x] The change includes or references documentation updates if necessary
- [x] Backport labels are set (if applicable)
Reviewer Checklist
- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Avoids breaking API changes
- [x] Testing strategy adequately addresses listed risks
- [x] Newly-added code is easy to change
- [x] Release note makes sense to a user of the library
- [x] If necessary, author has 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
Datadog Report
Branch report: christophe-papazian/use_file_queue_for_extra_service_names
Commit report: 688d707
Test service: dd-trace-py
:white_check_mark: 0 Failed, 175545 Passed, 1909 Skipped, 10h 28m 44.39s Total duration (41m 4.97s time saved)
Codecov Report
Attention: Patch coverage is 37.50000% with 35 lines in your changes missing coverage. Please review.
Project coverage is 10.46%. Comparing base (
b21d904) to head (6635198). Report is 2 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #9701 +/- ##
===========================================
- Coverage 74.60% 10.46% -64.15%
===========================================
Files 1385 1354 -31
Lines 128327 126328 -1999
===========================================
- Hits 95744 13214 -82530
- Misses 32583 113114 +80531
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Benchmarks
Benchmark execution time: 2024-07-04 09:08:27
Comparing candidate commit 688d7071aa496cbbaec68a9cf462b768d17490da in PR branch christophe-papazian/use_file_queue_for_extra_service_names with baseline commit d10c434d3469e74d9874f5072c70c34955a8d544 in branch main.
Found 4 performance improvements and 0 performance regressions! Performance is the same for 205 metrics, 9 unstable metrics.
scenario:httppropagationinject-ids_only
- 🟩
max_rss_usage[-1.930MB; -1.841MB] or [-8.712%; -8.311%]
scenario:otelspan-add-metrics
- 🟩
max_rss_usage[-6.096MB; -5.611MB] or [-14.885%; -13.702%]
scenario:otelspan-add-tags
- 🟩
max_rss_usage[-4.635MB; -4.220MB] or [-11.407%; -10.388%]
scenario:span-add-metrics
- 🟩
max_rss_usage[-16.962MB; -16.655MB] or [-25.788%; -25.322%]
The backport to 2.10 failed:
The process '/usr/bin/git' failed with exit code 1
To backport manually, run these commands in your terminal:
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.10 2.10
# Navigate to the new working tree
cd .worktrees/backport-2.10
# Create a new branch
git switch --create backport-9701-to-2.10
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f85625adceb18107014c3229b834a1df7e912395
# Push it to GitHub
git push --set-upstream origin backport-9701-to-2.10
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.10
Then, create a pull request where the base branch is 2.10 and the compare/head branch is backport-9701-to-2.10.
If anyone else comes across this, my Python project runs in a container with read_only filesystem and this PR broke it because FileNotFoundError: [Errno 2] No usable temporary directory found in ['/tmp', '/var/tmp', '/usr/tmp','/app']. I've had to set the environment variable DD_REMOTE_CONFIGURATION_ENABLED: 'false' to make it work again.
#10004 fixes behavior which was erroneously introduced here. CC @harm-nedap; disabling the configuration manually should be unnecessary after the linked PR is ~merged~ part of a release (thank you for calling out this behavior and directing others to the fix, though!)
Thanks for linking that PR, I had not seen it yet :)!