perf: reduce Span._finish_ns overhead in tracer and profiling #13720
+875
−262
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove Hooks class usage from BaseContextProvider and Tracer - Replace hook-based callbacks with core event dispatcher system - Optimize _update_active logic to reduce redundant finished() checks - Cache span._local_root property lookup in profiling link_span - Streamline span activation flow in DefaultContextProvider - Convert SimpleMovingAverage to Cython for C-speed mathematical operations - Add encoder caching to reduce redundant _trace_id_64bits property lookups Testing against a locally running version of PyPA warehouse with a single gunicorn worker with tracing and profiling enabled we saw an improvement of: - a 4.9% improvement in requests per second (20.08 → 21.06 RPS) - 4.6% reduction in response time (49.79ms → 47.48ms) - elimination of 44,028 function calls (-7.2%)
|
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 273 ± 3 ms. The average import time from base is: 275 ± 2 ms. The import time difference between this PR and base is: -2.5 ± 0.1 ms. Import time breakdownThe following import paths have grown:
|
BenchmarksBenchmark execution time: 2025-06-20 20:32:31 Comparing candidate commit 50dc57c in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 561 metrics, 3 unstable metrics. |
P403n1x87
reviewed
Jun 19, 2025
We make the native extension cache more fine-grained by generating
hashes for each single native integration that we build. We then store
each generated artifact in the cache according to this FS structure
```
.ext_cache/ # Example:
└─ <module_name>/ # "ddtrace.profiling.collector"
└─ <source_hash>/ # <sha256 hex digest>
└─ <binary> # "collector.cpython-310-darwin.so"
```
We implement a custom command to allow retrieving the source to target
mapping from `setup.py` directly, instead of coding the same information
elsewhere, which would increase the maintenance cost of the whole build
process.
## Checklist
- [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
## 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
fixes error when we try to concat the string 'truncated' to bytes ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: Quinna Halim <quinna.halim@datadoghq.com> Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
We do a minor setup.py cleanup, focusing on typing correctness. ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
Add telemetry support for the legacy ATO SDK. This will also be tested using system tests DataDog/system-tests#4806 APPSEC-58051 ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
This PR is a workaround for this [bug report](#13700) where LiteLLM users are running into an unhandled exception caused by trying to append a chunk choice to the list of streamed chunks at that choice index. The root of the issue comes down to where the user is setting the parameter for number of choices in the streamed response. If the `n` parameter was set as a kwarg, then there would be no issue. However, if the parameter was set elsewhere (e.g. in the user's proxy config file), then the `_streamed_chunks` list would incorrectly expect 1 choice which would lead to an indexing error. I was able to reproduce the issue with the following config and client request: Config File ``` model_list: - model_name: gpt-3.5-turbo litellm_params: model: openai/gpt-3.5-turbo api_key: "os.environ/OPENAI_API_KEY" n: 2 temperature: 0.2 ``` Client Request ``` import os import litellm import asyncio from litellm import acompletion litellm.api_key = os.environ["OPENAI_API_KEY"] async def acompletion_proxy(): messages = [{ "content": "What color is the sky?","role": "user"}] response = await acompletion(model="gpt-3.5-turbo", messages=messages, api_base="http://0.0.0.0:4000/", stream=True) async for item in response: print(item) if __name__ == "__main__": asyncio.run(acompletion_proxy()) ``` This lead to the following error: ``` Traceback (most recent call last): File "/Users/nicole.cybul/Documents/ML Observability/scripts/integrations/_simple_litellm_script.py", line 14, in <module> for item in response: File "/Users/nicole.cybul/Documents/ML Observability/scripts/.venv/lib/python3.11/site-packages/ddtrace/contrib/internal/litellm/utils.py", line 63, in __iter__ _loop_handler(chunk, self._streamed_chunks) File "/Users/nicole.cybul/Documents/ML Observability/scripts/.venv/lib/python3.11/site-packages/ddtrace/contrib/internal/litellm/utils.py", line 131, in _loop_handler streamed_chunks[choice.index].append(choice) ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^ IndexError: list index out of range ``` The fix in this PR uses a `defaultdict(list)` which should be robust against these types of indexing issues. With this new fix, the same request leads to a successful response and this [trace](https://dd.datad0g.com/llm/traces?query=%40ml_app%3Anicole-test%20%40event_type%3Aspan%20%40parent_id%3Aundefined&agg_m=count&agg_m_source=base&agg_t=count&fromUser=true&llmPanels=%5B%7B%22t%22%3A%22sampleDetailPanel%22%2C%22rEID%22%3A%22AwAAAZeEzsIo5PnrZAAAABhBWmVFenNJb0FBRDZPcUs4ZV94bEFBQUEAAAAkZjE5Nzg0Y2UtZTFlNy00YzI2LTk5MWQtMjg3YmJlNGM2ZTllAAAAIg%22%7D%5D&spanId=15697199987942968683&start=1750278947467&end=1750279847467&paused=false) in the product. ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
We include the rust extensions in the computation of the hash for native extensions. This allows us to cache the compilation result of rust builds and reuse them in CI. ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
…setup.py (#13732) With the recent changes that have introduced some incremental build support for CMake, there doesn't seem to be a reason to maintain this extra feature, so we can revert the change and simplify setup.py a bit. - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
brettlangdon
force-pushed
the
brettlangdon/perf.optimize.span.finish
branch
from
ee0f0a2 to
19270da
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Testing against a locally running version of PyPA warehouse with a single gunicorn worker with tracing and profiling enabled we saw an improvement of:
Checklist
Reviewer Checklist