- 
                Notifications
    You must be signed in to change notification settings 
- Fork 468
fix(ci_visibility): sys.monitoring deinstrumentation [backport 3.15] #15022
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: 3.15
Are you sure you want to change the base?
Conversation
| 
  | 
| Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 282 ± 6 ms. The average import time from base is: 284 ± 7 ms. The import time difference between this PR and base is: -1.6 ± 0.3 ms. Import time breakdownThe following import paths have shrunk: 
             | 
| Performance SLOsComparing candidate backport-14859-to-3.15 (9762e64) with baseline 3.15 (5a5365e) 📈 Performance Regressions (1 suite)📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 4.464µs (SLO: <10.000µs 📉 -55.4%) vs baseline: +1.4% Memory: ✅ 37.709MB (SLO: <39.000MB -3.3%) vs baseline: +4.9% ✅ ospathbasename_noaspectTime: ✅ 1.091µs (SLO: <10.000µs 📉 -89.1%) vs baseline: -0.1% Memory: ✅ 37.690MB (SLO: <39.000MB -3.4%) vs baseline: +4.8% ✅ ospathjoin_aspectTime: ✅ 6.126µs (SLO: <10.000µs 📉 -38.7%) vs baseline: +0.6% Memory: ✅ 37.670MB (SLO: <39.000MB -3.4%) vs baseline: +4.8% ✅ ospathjoin_noaspectTime: ✅ 2.283µs (SLO: <10.000µs 📉 -77.2%) vs baseline: -0.5% Memory: ✅ 37.670MB (SLO: <39.000MB -3.4%) vs baseline: +4.8% ✅ ospathnormcase_aspectTime: ✅ 3.973µs (SLO: <10.000µs 📉 -60.3%) vs baseline: 📈 +13.0% Memory: ✅ 37.631MB (SLO: <39.000MB -3.5%) vs baseline: +4.7% ✅ ospathnormcase_noaspectTime: ✅ 0.571µs (SLO: <10.000µs 📉 -94.3%) vs baseline: -1.4% Memory: ✅ 37.670MB (SLO: <39.000MB -3.4%) vs baseline: +4.7% ✅ ospathsplit_aspectTime: ✅ 5.009µs (SLO: <10.000µs 📉 -49.9%) vs baseline: +0.6% Memory: ✅ 37.650MB (SLO: <39.000MB -3.5%) vs baseline: +4.9% ✅ ospathsplit_noaspectTime: ✅ 1.588µs (SLO: <10.000µs 📉 -84.1%) vs baseline: +0.2% Memory: ✅ 37.650MB (SLO: <39.000MB -3.5%) vs baseline: +5.0% ✅ ospathsplitdrive_aspectTime: ✅ 3.737µs (SLO: <10.000µs 📉 -62.6%) vs baseline: -0.1% Memory: ✅ 37.690MB (SLO: <39.000MB -3.4%) vs baseline: +4.9% ✅ ospathsplitdrive_noaspectTime: ✅ 0.713µs (SLO: <10.000µs 📉 -92.9%) vs baseline: +1.0% Memory: ✅ 37.690MB (SLO: <39.000MB -3.4%) vs baseline: +4.9% ✅ ospathsplitext_aspectTime: ✅ 5.325µs (SLO: <10.000µs 📉 -46.8%) vs baseline: 📈 +15.5% Memory: ✅ 37.650MB (SLO: <39.000MB -3.5%) vs baseline: +4.9% ✅ ospathsplitext_noaspectTime: ✅ 1.377µs (SLO: <10.000µs 📉 -86.2%) vs baseline: -0.1% Memory: ✅ 37.709MB (SLO: <39.000MB -3.3%) vs baseline: +5.1% 🟡 Near SLO Breach (3 suites)🟡 djangosimple - 30/30✅ appsecTime: ✅ 20.460ms (SLO: <22.300ms -8.3%) vs baseline: -0.5% Memory: ✅ 65.203MB (SLO: <67.000MB -2.7%) vs baseline: +4.8% ✅ exception-replay-enabledTime: ✅ 1.349ms (SLO: <1.450ms -6.9%) vs baseline: ~same Memory: ✅ 64.253MB (SLO: <67.000MB -4.1%) vs baseline: +4.8% ✅ iastTime: ✅ 20.503ms (SLO: <22.250ms -7.8%) vs baseline: ~same Memory: ✅ 65.220MB (SLO: <67.000MB -2.7%) vs baseline: +4.8% ✅ profilerTime: ✅ 15.229ms (SLO: <16.550ms -8.0%) vs baseline: +0.2% Memory: ✅ 53.671MB (SLO: <54.500MB 🟡 -1.5%) vs baseline: +5.0% ✅ resource-renamingTime: ✅ 20.615ms (SLO: <21.750ms -5.2%) vs baseline: +0.5% Memory: ✅ 65.228MB (SLO: <67.000MB -2.6%) vs baseline: +4.8% ✅ span-code-originTime: ✅ 26.243ms (SLO: <28.200ms -6.9%) vs baseline: +0.2% Memory: ✅ 67.329MB (SLO: <69.500MB -3.1%) vs baseline: +4.6% ✅ tracerTime: ✅ 20.533ms (SLO: <21.750ms -5.6%) vs baseline: +0.1% Memory: ✅ 65.248MB (SLO: <67.000MB -2.6%) vs baseline: +4.9% ✅ tracer-and-profilerTime: ✅ 22.112ms (SLO: <23.500ms -5.9%) vs baseline: ~same Memory: ✅ 66.368MB (SLO: <67.500MB 🟡 -1.7%) vs baseline: +5.0% ✅ tracer-dont-create-db-spansTime: ✅ 19.289ms (SLO: <21.500ms 📉 -10.3%) vs baseline: -0.5% Memory: ✅ 65.201MB (SLO: <66.000MB 🟡 -1.2%) vs baseline: +4.8% ✅ tracer-minimalTime: ✅ 16.694ms (SLO: <17.500ms -4.6%) vs baseline: -0.3% Memory: ✅ 65.114MB (SLO: <66.000MB 🟡 -1.3%) vs baseline: +4.7% ✅ tracer-nativeTime: ✅ 20.445ms (SLO: <21.750ms -6.0%) vs baseline: ~same Memory: ✅ 71.074MB (SLO: <72.500MB 🟡 -2.0%) vs baseline: +4.9% ✅ tracer-no-cachesTime: ✅ 18.479ms (SLO: <19.650ms -6.0%) vs baseline: ~same Memory: ✅ 65.230MB (SLO: <67.000MB -2.6%) vs baseline: +4.8% ✅ tracer-no-databasesTime: ✅ 18.881ms (SLO: <20.100ms -6.1%) vs baseline: +0.2% Memory: ✅ 64.838MB (SLO: <67.000MB -3.2%) vs baseline: +4.8% ✅ tracer-no-middlewareTime: ✅ 20.128ms (SLO: <21.500ms -6.4%) vs baseline: -0.4% Memory: ✅ 65.211MB (SLO: <67.000MB -2.7%) vs baseline: +4.8% ✅ tracer-no-templatesTime: ✅ 20.320ms (SLO: <22.000ms -7.6%) vs baseline: -0.2% Memory: ✅ 65.249MB (SLO: <67.000MB -2.6%) vs baseline: +4.8% 🟡 flasksqli - 6/6✅ appsec-enabledTime: ✅ 3.941ms (SLO: <4.200ms -6.2%) vs baseline: ~same Memory: ✅ 63.229MB (SLO: <66.000MB -4.2%) vs baseline: +4.7% ✅ iast-enabledTime: ✅ 2.456ms (SLO: <2.800ms 📉 -12.3%) vs baseline: +0.6% Memory: ✅ 58.866MB (SLO: <60.000MB 🟡 -1.9%) vs baseline: +5.0% ✅ tracer-enabledTime: ✅ 2.081ms (SLO: <2.250ms -7.5%) vs baseline: -0.2% Memory: ✅ 51.995MB (SLO: <54.500MB -4.6%) vs baseline: +4.9% 🟡 otelspan - 22/22✅ add-eventTime: ✅ 45.190ms (SLO: <47.150ms -4.2%) vs baseline: ~same Memory: ✅ 45.110MB (SLO: <47.000MB -4.0%) vs baseline: +4.7% ✅ add-metricsTime: ✅ 321.522ms (SLO: <344.800ms -6.8%) vs baseline: +0.9% Memory: ✅ 553.390MB (SLO: <562.000MB 🟡 -1.5%) vs baseline: +5.0% ✅ add-tagsTime: ✅ 292.137ms (SLO: <314.000ms -7.0%) vs baseline: +0.3% Memory: ✅ 554.000MB (SLO: <563.500MB 🟡 -1.7%) vs baseline: +4.9% ✅ get-contextTime: ✅ 84.775ms (SLO: <92.350ms -8.2%) vs baseline: +3.1% Memory: ✅ 40.187MB (SLO: <46.500MB 📉 -13.6%) vs baseline: +4.9% ✅ is-recordingTime: ✅ 43.111ms (SLO: <44.500ms -3.1%) vs baseline: +0.8% Memory: ✅ 44.551MB (SLO: <47.500MB -6.2%) vs baseline: +4.8% ✅ record-exceptionTime: ✅ 61.965ms (SLO: <67.650ms -8.4%) vs baseline: +0.2% Memory: ✅ 40.480MB (SLO: <47.000MB 📉 -13.9%) vs baseline: +4.8% ✅ set-statusTime: ✅ 48.823ms (SLO: <50.400ms -3.1%) vs baseline: +0.6% Memory: ✅ 44.567MB (SLO: <47.000MB -5.2%) vs baseline: +4.8% ✅ startTime: ✅ 42.311ms (SLO: <43.450ms -2.6%) vs baseline: +0.4% Memory: ✅ 44.523MB (SLO: <47.000MB -5.3%) vs baseline: +4.9% ✅ start-finishTime: ✅ 83.275ms (SLO: <88.000ms -5.4%) vs baseline: +0.6% Memory: ✅ 34.583MB (SLO: <46.500MB 📉 -25.6%) vs baseline: +4.8% ✅ start-finish-telemetryTime: ✅ 84.686ms (SLO: <89.000ms -4.8%) vs baseline: +0.2% Memory: ✅ 34.524MB (SLO: <46.500MB 📉 -25.8%) vs baseline: +4.7% ✅ update-nameTime: ✅ 44.513ms (SLO: <45.150ms 🟡 -1.4%) vs baseline: +0.7% Memory: ✅ 44.756MB (SLO: <47.000MB -4.8%) vs baseline: +4.9% 
 | 
## Description Fixes a coverage tracking performance issue by leveraging de-instrumentation after line events, and re-instrumentation between coverage collection contexts on Python 3.12+. **Problem:** The coverage tracking wasn't using `sys.monitoring` API's `DISABLE` for `LINE` events once the line coverage was tracked. **Solution:** Return `sys.monitoring.DISABLE` once a line is tracked, and call `sys.monitoring.restart_events()` when entering new coverage contexts to re-enable monitoring. ## Testing - New tests covering sequential contexts, nested contexts, dynamic imports, and nested import chains - Tests verify that coverage is complete and consistent across multiple context switches ## Risks Low - only affects Python 3.12+ coverage, uses `sys.monitoring.DISABLE` and `sys.monitoring.restart_events()` [API](https://docs.python.org/3/library/sys.monitoring.html#disabling-events), extensively tested. ## Additional Notes ### Performance Gain Example: The best performance gains for this PR happen when recursive code or loops are used heavily in the tested code, for example a recursive implementation of a fibonacci sequence calculator: ``` # fibonacci.py def fibonacci(n): if n <= 1: return n return fibonacci(n - 1) + fibonacci(n - 2) ``` Then running just this test: ``` # test_fibonacci.py from fibonacci import fibonacci def test_fibonacci(): assert fibonacci(35) == 9227465 ``` Yields the following results: No coverage: ``` 1 passed in 0.98s ``` current coverage (main): ``` 1 passed in 24.11s ``` new coverage (this branch): ``` 1 passed in 1.01s ``` (cherry picked from commit 93aff64)
53978b3    to
    9762e64      
    Compare
  
    
Backport 93aff64 from #14859 to 3.15.
Description
Fixes a coverage tracking performance issue by leveraging de-instrumentation after line events, and re-instrumentation between coverage collection contexts on Python 3.12+.
Problem: The coverage tracking wasn't using
sys.monitoringAPI'sDISABLEforLINEevents once the line coverage was tracked.Solution: Return
sys.monitoring.DISABLEonce a line is tracked, and callsys.monitoring.restart_events()when entering new coverage contexts to re-enable monitoring.Testing
Risks
Low - only affects Python 3.12+ coverage, uses
sys.monitoring.DISABLEandsys.monitoring.restart_events()API, extensively tested.However, if other tool was using this API at the same time, when we call
sys.monitoring.restart_events(), we would be re-enabling their disabled events as well.Additional Notes
Performance Gain Example:
The best performance gains for this PR happen when recursive code or loops are used heavily in the tested code, for example a recursive implementation of a fibonacci sequence calculator:
Then running just this test:
Yields the following results:
No coverage:
current coverage (main):
new coverage (this branch):