-
Notifications
You must be signed in to change notification settings - Fork 469
fix(tornado): fix tracer.wrap() support for coroutines and Futures #15305
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
base: main
Are you sure you want to change the base?
Conversation
6a22ddc to
c0fa851
Compare
|
|
c0fa851 to
88fd5d3
Compare
88fd5d3 to
951e12a
Compare
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 259 ± 7 ms. The average import time from base is: 263 ± 6 ms. The import time difference between this PR and base is: -3.5 ± 0.3 ms. Import time breakdownThe following import paths have shrunk:
|
Performance SLOsComparing candidate munir/remove-tornado-stuff (1909383) with baseline main (53726a1) 📈 Performance Regressions (1 suite)📈 telemetryaddmetric - 30/30✅ 1-count-metric-1-timesTime: ✅ 3.280µs (SLO: <20.000µs 📉 -83.6%) vs baseline: 📈 +12.0% Memory: ✅ 34.642MB (SLO: <35.500MB -2.4%) vs baseline: +5.2% ✅ 1-count-metrics-100-timesTime: ✅ 200.473µs (SLO: <220.000µs -8.9%) vs baseline: ~same Memory: ✅ 34.583MB (SLO: <35.500MB -2.6%) vs baseline: +4.9% ✅ 1-distribution-metric-1-timesTime: ✅ 3.253µs (SLO: <20.000µs 📉 -83.7%) vs baseline: ~same Memory: ✅ 34.623MB (SLO: <35.500MB -2.5%) vs baseline: +4.9% ✅ 1-distribution-metrics-100-timesTime: ✅ 213.143µs (SLO: <230.000µs -7.3%) vs baseline: -1.1% Memory: ✅ 34.623MB (SLO: <35.500MB -2.5%) vs baseline: +4.9% ✅ 1-gauge-metric-1-timesTime: ✅ 2.154µs (SLO: <20.000µs 📉 -89.2%) vs baseline: -1.2% Memory: ✅ 34.603MB (SLO: <35.500MB -2.5%) vs baseline: +4.6% ✅ 1-gauge-metrics-100-timesTime: ✅ 136.677µs (SLO: <150.000µs -8.9%) vs baseline: -0.7% Memory: ✅ 34.603MB (SLO: <35.500MB -2.5%) vs baseline: +4.5% ✅ 1-rate-metric-1-timesTime: ✅ 3.131µs (SLO: <20.000µs 📉 -84.3%) vs baseline: +2.3% Memory: ✅ 34.642MB (SLO: <35.500MB -2.4%) vs baseline: +5.0% ✅ 1-rate-metrics-100-timesTime: ✅ 213.751µs (SLO: <250.000µs 📉 -14.5%) vs baseline: -0.3% Memory: ✅ 34.603MB (SLO: <35.500MB -2.5%) vs baseline: +4.7% ✅ 100-count-metrics-100-timesTime: ✅ 20.499ms (SLO: <22.000ms -6.8%) vs baseline: +1.3% Memory: ✅ 34.642MB (SLO: <35.500MB -2.4%) vs baseline: +5.0% ✅ 100-distribution-metrics-100-timesTime: ✅ 2.238ms (SLO: <2.300ms -2.7%) vs baseline: -0.7% Memory: ✅ 34.564MB (SLO: <35.500MB -2.6%) vs baseline: +4.9% ✅ 100-gauge-metrics-100-timesTime: ✅ 1.407ms (SLO: <1.550ms -9.2%) vs baseline: -0.5% Memory: ✅ 34.583MB (SLO: <35.500MB -2.6%) vs baseline: +4.6% ✅ 100-rate-metrics-100-timesTime: ✅ 2.203ms (SLO: <2.550ms 📉 -13.6%) vs baseline: -0.2% Memory: ✅ 34.603MB (SLO: <35.500MB -2.5%) vs baseline: +5.0% ✅ flush-1-metricTime: ✅ 4.537µs (SLO: <20.000µs 📉 -77.3%) vs baseline: +0.9% Memory: ✅ 34.564MB (SLO: <35.500MB -2.6%) vs baseline: +4.6% ✅ flush-100-metricsTime: ✅ 173.429µs (SLO: <250.000µs 📉 -30.6%) vs baseline: -0.6% Memory: ✅ 34.564MB (SLO: <35.500MB -2.6%) vs baseline: +5.0% ✅ flush-1000-metricsTime: ✅ 2.124ms (SLO: <2.500ms 📉 -15.0%) vs baseline: +0.5% Memory: ✅ 35.389MB (SLO: <36.500MB -3.0%) vs baseline: +4.7% 🟡 Near SLO Breach (1 suite)🟡 iastpropagation - 8/8✅ no-propagationTime: ✅ 48.611µs (SLO: <60.000µs 📉 -19.0%) vs baseline: ~same Memory: ✅ 39.459MB (SLO: <40.500MB -2.6%) vs baseline: +4.8% ✅ propagation_enabledTime: ✅ 167.260µs (SLO: <190.000µs 📉 -12.0%) vs baseline: ~same Memory: ✅ 39.538MB (SLO: <40.000MB 🟡 -1.2%) vs baseline: +5.0% ✅ propagation_enabled_100Time: ✅ 1.857ms (SLO: <2.300ms 📉 -19.3%) vs baseline: +0.1% Memory: ✅ 39.459MB (SLO: <40.000MB 🟡 -1.4%) vs baseline: +4.8% ✅ propagation_enabled_1000Time: ✅ 32.604ms (SLO: <34.550ms -5.6%) vs baseline: +0.9% Memory: ✅ 39.499MB (SLO: <40.000MB 🟡 -1.3%) vs baseline: +4.8%
|
ce01e46 to
177d120
Compare
177d120 to
17c0219
Compare
03628cf to
1909383
Compare
Description
Fixed
tracer.wrap()support for Tornado coroutines and functions that return Futures. Previously,tracer.wrap()could apply incorrect durations to spans for asynchronous operations, as the span might finish before the asynchronous task completed.The fix introduces a custom
wrap_executorthat leverages ddtrace's contextvars-based context management (viatracer.context_provider) to properly handle Tornado Futures. The executor attaches a callback to Futures that finishes the span only when the Future completes, ensuring accurate span durations and correct context propagation.Additionally,
tornado.gen.sleepis no longer supported for Tornado >=6.1 (asyncio-based); a warning will be logged if it is used. Users should migrate toasyncio.sleep.Testing
All existing Tornado integration tests pass. Updated tests to use
asyncio.sleepinstead oftornado.gen.sleepand verified that span durations correctly reflect asynchronous operation completion.Risks
Low. Tornado v6.1+ is asyncio-based and has been stable since 2020. The changes improve trace accuracy and context propagation for async operations.
Additional Notes
tracer.wrap())