chore(tracing): emit five CPython GC collection and pause runtime metrics (PROF-15855) - #19937
chore(tracing): emit five CPython GC collection and pause runtime metrics (PROF-15855)#19937vlad-scherbich wants to merge 8 commits into
Conversation
Fleet-scan STW impact from DogStatsD without waiting on a GC profiler.
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 11f00bc | Docs | View more details | Give us feedback! |
Codeowners resolved asResolved from the full PR diff against |
Circular import analysis
|
Dependency direction analysis
|
vlad-scherbich
changed the title
vlad-scherbich
changed the title
BenchmarksBenchmark execution time: 2026-08-29 17:37:45 Comparing candidate commit 11f00bc in PR branch Found 0 performance improvements and 3 performance regressions! Performance is the same for 374 metrics, 9 unstable metrics, 5 flaky benchmarks without significant changes.
|
|
@codex review |
vlad-scherbich
changed the title
vlad-scherbich
changed the title
There was a problem hiding this comment.
Pull request overview
Adds new CPython GC stop-the-world (STW) pause-time metrics and per-generation collection-count deltas to the existing runtime metrics pipeline, enabling fleet-wide GC impact visibility via DogStatsD when runtime metrics are enabled.
Changes:
- Introduces a process-wide
gc.callbacks-based pause monitor with refcounted install and snapshot/reset semantics. - Extends GC runtime metrics to emit per-generation
gc.get_stats()collection deltas plus pause total/max (ns) per flush interval. - Updates runtime-metrics shutdown to stop collectors, adds/updates tests, and includes a release note.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
ddtrace/internal/runtime/gc_monitor.py |
New process-wide GC pause observer built on gc.callbacks, with snapshot/reset and listener support. |
ddtrace/internal/runtime/metric_collectors.py |
Extends GC runtime collector to emit collection deltas and pause metrics; integrates the pause monitor. |
ddtrace/internal/runtime/runtime_metrics.py |
Adds a collector stop() cascade and calls it during RuntimeWorker.disable(). |
ddtrace/internal/runtime/constants.py |
Defines new metric names and expands GC_RUNTIME_METRICS. |
tests/tracer/runtime/test_gc_monitor.py |
New unit tests for monitor acquire/release, snapshot behavior, and listener plumbing. |
tests/tracer/runtime/test_metric_collectors.py |
Updates GC collector tests to stop the collector and adds assertions for the new metrics. |
releasenotes/notes/runtime-python-gc-pause-metrics-c3e8a1b2d4f6.yaml |
Documents the new runtime metrics feature. |
Suppressed comments (1)
ddtrace/internal/runtime/metric_collectors.py:83
- collect_fn snapshots the pause window after calling gc.get_stats()/allocating lists. Any GC triggered by those allocations can be counted as "pause" for the interval, and stop() can race with flush by changing _monitor between the check and the method call. Snapshot the monitor first using a local variable, then do the other reads/allocations.
gc_mod: ModuleType = self.modules["gc"]
counts: tuple[int, int, int] = gc_mod.get_count()
collections: list[int] = _read_gc_collections(gc_mod)
d_collections: list[int] = _delta(collections, self._prev_collections)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7172716e8f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Clear in-flight starts on uninstall, unregister the collector fork hook, and snapshot primitives before allocating so a reentrant GC cannot drop or mismatch a pause.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
ddtrace/internal/runtime/gc_monitor.py:104
- GCPauseMonitor.acquire() registers a forksafe hook (self.reset), but release() never unregisters it when the refcount drops to 0. This leaves a global fork hook installed even after the monitor is fully released, which contradicts the refcounted install semantics and adds unnecessary fork-time work.
if self._refcount == 0:
try:
gc.callbacks.remove(self._on_gc)
except ValueError:
pass
tests/tracer/runtime/test_metric_collectors.py:228
- Type annotation for collected uses tuple[str, str], but GCRuntimeMetricCollector.collect() returns numeric values (ints) for these metrics. Keeping the annotation accurate avoids confusing type checkers/readers.
This issue also appears on line 247 of the same file.
collected: Optional[list[tuple[str, str]]] = collector.collect(GC_RUNTIME_METRICS)
tests/tracer/runtime/test_metric_collectors.py:247
- Type annotation for collected uses tuple[str, str], but GCRuntimeMetricCollector.collect() returns numeric values (ints) for these metrics. Keeping the annotation accurate avoids confusing type checkers/readers.
collected: Optional[list[tuple[str, str]]] = collector.collect(GC_RUNTIME_METRICS)
A child that inherits _MONITOR_LOCK held across fork can deadlock the next gc_pause_monitor() call.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
ddtrace/internal/runtime/gc_monitor.py:106
- GCPauseMonitor.acquire() registers self.reset as a forksafe hook, but release() never unregisters it when the refcount reaches 0. This means each standalone GCPauseMonitor() instance (e.g., in tests) is kept alive by forksafe._registry, and the registry grows over time (extra hooks run on every fork). Unregister the hook when the last acquire is released.
def acquire(self) -> None:
with self._lock:
self._refcount += 1
if self._refcount == 1:
if self._on_gc not in gc.callbacks:
gc.callbacks.append(self._on_gc)
if not self._fork_registered:
forksafe.register(self.reset)
self._fork_registered = True
def release(self) -> None:
with self._lock:
if self._refcount <= 0:
return
self._refcount -= 1
if self._refcount == 0:
try:
gc.callbacks.remove(self._on_gc)
except ValueError:
pass
# Drop in-flight starts so a later re-acquire cannot pair a
# new stop with a stale timestamp from before uninstall.
self._start_ns = [0] * GEN_COUNT
self._clear_window()
tests/tracer/runtime/test_metric_collectors.py:228
- Type annotation mismatch: ValueCollector.collect() returns pairs where the value is numeric for runtime metrics collectors, so annotating this as tuple[str, str] is misleading and can break type-checking of tests. Annotate this as tuple[str, int] here (these GC metrics are ints).
This issue also appears on line 247 of the same file.
collected: Optional[list[tuple[str, str]]] = collector.collect(GC_RUNTIME_METRICS)
tests/tracer/runtime/test_metric_collectors.py:247
- Type annotation mismatch: this GC collector returns numeric values; annotate the collected metrics as tuple[str, int] instead of tuple[str, str] to keep the test type-correct.
collected: Optional[list[tuple[str, str]]] = collector.collect(GC_RUNTIME_METRICS)
Description
With
DD_RUNTIME_METRICS_ENABLED(default off), each flush adds five DogStatsD metrics:runtime.python.gc.collections.gen{0,1,2}(gc.get_stats()collections) andruntime.python.gc.pause.time/runtime.python.gc.pause.max(STW pause ns).GCPauseMonitoris one refcountedgc.callbackssubscriber; snapshot on flush. Existingruntime.python.gc.count.gen{0,1,2}stay allocation counters.RuntimeCollectorsIterable.stop()uninstalls the callback.Testing
Unit tests drive
gc.collect()through the monitor and collector to pin callback lifetime, the five names, and a zeroed window on the next flush.Risks
+5 series per existing
runtime.python.gc.count.gen0tag combo. Default-off; Datadog-internal already has the flag on.Additional Notes
KR0.5.1 fleet-scan metrics.
add_listeneris unused here for profiling — this PR does not emit STW timeline events.DD_PROFILING_GC_ENABLED)vlad/profiling-gc-pause-eventsOn main: T Kowalski #19190 sampled GC frames (
DD_PROFILING_STACK_GC_ENABLED) — not pause duration, not Henrik. Next PR (stacked here) adds exactgc.callbacksevents for the Henrik STW timeline; do not P1 until after this fleet scan. Cut names (pause.time.gen*,collected.*,uncollectable.*) live onvlad/runtime-gc-metrics-futureonly.