Record the total time spent waiting for the GVL as a metric#5569
Record the total time spent waiting for the GVL as a metric#5569
Conversation
|
Thank you for updating Change log entry section 👏 Visited at: 2026-04-09 14:13:34 UTC |
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: a125c7d | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback! |
5aee7cb to
472c7ee
Compare
BenchmarksBenchmark execution time: 2026-04-09 14:16:50 Comparing candidate commit 472c7ee in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 1 unstable metrics.
|
ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c
Outdated
Show resolved
Hide resolved
| metrics = { | ||
| gvl_waiting_time_ns_total: worker_stats.delete(:gvl_waiting_time_ns_total), | ||
| } |
There was a problem hiding this comment.
So it looks to me the metric can be nil in e.g. old Rubies or when DD_PROFILING_GVL_ENABLED is set to false.
I believe in the backend this maps to JsonMetricsComputer.java and at a quick glance I'm not sure it likes null. Maybe better to not emit it at all if it's nil?
(Or try submitting to staging / poking at the backend code / asking in #profiling-backend what's the best option here)
| ) | ||
| expect(JSON.parse(flush.metrics, symbolize_names: true)).to eq(gvl_waiting_time_ns_total: nil) | ||
| end | ||
|
|
||
| context "when metrics include a numeric gvl_waiting_time_ns_total" do | ||
| let(:metrics) { {gvl_waiting_time_ns_total: 12_345} } | ||
|
|
||
| it "stores them as metrics.json payload" do | ||
| expect(JSON.parse(flush.metrics, symbolize_names: true)).to eq(gvl_waiting_time_ns_total: 12_345) | ||
| end |
There was a problem hiding this comment.
Minor: This suspiciously looks like "AI doing verbose tests" haha, I would recommend just changing the have_attributes above to have a metrics: '{"gvl_waiting_time_ns_total": 12345}' or something like that? Same coverage, less code ;)
What does this PR do?
Record the total time spent waiting for the GVL as a metric.
See #3433.
Motivation:
That issue.
Change log entry
Yes. Added metric for total time spent waiting for the GVL.
Additional Notes:
Yes, I'm experimenting with Cursor.
How to test the change?