-
Notifications
You must be signed in to change notification settings - Fork 480
fix(profiling): don't patch gevent when profiling is off #16425
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2884eec5f9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Patch gevent only when profiling is enabled | ||
| if config.enabled: | ||
| if (is_stack := config.stack.enabled): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define
is_stack when profiling disabled
Wrapping the is_stack initialization inside if config.enabled means is_stack (and _gevent_tracer) are never defined when profiling is disabled. However get_task() unconditionally reads is_stack and _gevent_tracer later in this module. If a user starts profiling programmatically (e.g., Profiler().start()) without setting dd.profiling.enabled, config.enabled stays false and calling get_task() will raise NameError. This is a regression from the previous unconditional initialization and can break lock/stack collectors in that scenario.
Useful? React with 👍 / 👎.
|
Performance SLOsComparing candidate taegyunkim/patch-gevent-when-profiling-enabled (2884eec) with baseline main (1859292) 📈 Performance Regressions (3 suites)📈 iastaspects - 118/118✅ add_aspectTime: ✅ 104.357µs (SLO: <130.000µs 📉 -19.7%) vs baseline: +2.7% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.5% ✅ add_inplace_aspectTime: ✅ 102.132µs (SLO: <130.000µs 📉 -21.4%) vs baseline: -0.3% Memory: ✅ 43.096MB (SLO: <46.000MB -6.3%) vs baseline: +5.0% ✅ add_inplace_noaspectTime: ✅ 28.290µs (SLO: <40.000µs 📉 -29.3%) vs baseline: +0.2% Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +4.8% ✅ add_noaspectTime: ✅ 48.811µs (SLO: <70.000µs 📉 -30.3%) vs baseline: -0.5% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +5.0% ✅ bytearray_aspectTime: ✅ 251.907µs (SLO: <400.000µs 📉 -37.0%) vs baseline: +0.3% Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +5.2% ✅ bytearray_extend_aspectTime: ✅ 629.039µs (SLO: <800.000µs 📉 -21.4%) vs baseline: +0.4% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ bytearray_extend_noaspectTime: ✅ 263.010µs (SLO: <400.000µs 📉 -34.2%) vs baseline: -0.7% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.7% ✅ bytearray_noaspectTime: ✅ 140.955µs (SLO: <300.000µs 📉 -53.0%) vs baseline: +0.7% Memory: ✅ 43.136MB (SLO: <46.000MB -6.2%) vs baseline: +5.2% ✅ bytes_aspectTime: ✅ 216.594µs (SLO: <300.000µs 📉 -27.8%) vs baseline: -1.2% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.6% ✅ bytes_noaspectTime: ✅ 134.578µs (SLO: <200.000µs 📉 -32.7%) vs baseline: +0.2% Memory: ✅ 43.273MB (SLO: <46.000MB -5.9%) vs baseline: +5.2% ✅ bytesio_aspectTime: ✅ 3.926ms (SLO: <5.000ms 📉 -21.5%) vs baseline: -0.3% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% ✅ bytesio_noaspectTime: ✅ 313.619µs (SLO: <420.000µs 📉 -25.3%) vs baseline: -1.0% Memory: ✅ 43.136MB (SLO: <46.000MB -6.2%) vs baseline: +5.2% ✅ capitalize_aspectTime: ✅ 89.567µs (SLO: <300.000µs 📉 -70.1%) vs baseline: +0.9% Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +5.0% ✅ capitalize_noaspectTime: ✅ 257.716µs (SLO: <300.000µs 📉 -14.1%) vs baseline: +0.2% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.7% ✅ casefold_aspectTime: ✅ 92.515µs (SLO: <500.000µs 📉 -81.5%) vs baseline: +3.5% Memory: ✅ 43.096MB (SLO: <46.000MB -6.3%) vs baseline: +5.1% ✅ casefold_noaspectTime: ✅ 315.233µs (SLO: <500.000µs 📉 -37.0%) vs baseline: +0.5% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% ✅ decode_aspectTime: ✅ 88.024µs (SLO: <100.000µs 📉 -12.0%) vs baseline: +0.3% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.9% ✅ decode_noaspectTime: ✅ 153.771µs (SLO: <210.000µs 📉 -26.8%) vs baseline: ~same Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +4.6% ✅ encode_aspectTime: ✅ 85.581µs (SLO: <200.000µs 📉 -57.2%) vs baseline: +0.4% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.5% ✅ encode_noaspectTime: ✅ 142.484µs (SLO: <200.000µs 📉 -28.8%) vs baseline: +0.6% Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.9% ✅ format_aspectTime: ✅ 14.625ms (SLO: <19.200ms 📉 -23.8%) vs baseline: +0.3% Memory: ✅ 43.137MB (SLO: <46.000MB -6.2%) vs baseline: +4.9% ✅ format_map_aspectTime: ✅ 16.405ms (SLO: <21.500ms 📉 -23.7%) vs baseline: ~same Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.8% ✅ format_map_noaspectTime: ✅ 369.923µs (SLO: <500.000µs 📉 -26.0%) vs baseline: +1.1% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +5.0% ✅ format_noaspectTime: ✅ 308.100µs (SLO: <500.000µs 📉 -38.4%) vs baseline: +0.4% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +4.8% ✅ index_aspectTime: ✅ 123.385µs (SLO: <300.000µs 📉 -58.9%) vs baseline: -0.6% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +4.7% ✅ index_noaspectTime: ✅ 40.299µs (SLO: <300.000µs 📉 -86.6%) vs baseline: -0.2% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.8% ✅ join_aspectTime: ✅ 215.744µs (SLO: <300.000µs 📉 -28.1%) vs baseline: +0.6% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.8% ✅ join_noaspectTime: ✅ 145.235µs (SLO: <300.000µs 📉 -51.6%) vs baseline: -0.9% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.7% ✅ ljust_aspectTime: ✅ 490.391µs (SLO: <700.000µs 📉 -29.9%) vs baseline: +0.5% Memory: ✅ 43.087MB (SLO: <46.000MB -6.3%) vs baseline: +4.9% ✅ ljust_noaspectTime: ✅ 255.223µs (SLO: <300.000µs 📉 -14.9%) vs baseline: ~same Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +5.0% ✅ lower_aspectTime: ✅ 300.506µs (SLO: <500.000µs 📉 -39.9%) vs baseline: +0.2% Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +5.1% ✅ lower_noaspectTime: ✅ 235.267µs (SLO: <300.000µs 📉 -21.6%) vs baseline: -0.4% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ lstrip_aspectTime: ✅ 0.341ms (SLO: <3.000ms 📉 -88.6%) vs baseline: 📈 +23.2% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.8% ✅ lstrip_noaspectTime: ✅ 0.179ms (SLO: <3.000ms 📉 -94.0%) vs baseline: +0.8% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.8% ✅ modulo_aspectTime: ✅ 14.328ms (SLO: <18.750ms 📉 -23.6%) vs baseline: ~same Memory: ✅ 43.116MB (SLO: <46.000MB -6.3%) vs baseline: +4.6% ✅ modulo_aspect_for_bytearray_bytearrayTime: ✅ 14.763ms (SLO: <19.350ms 📉 -23.7%) vs baseline: +0.2% Memory: ✅ 43.166MB (SLO: <46.000MB -6.2%) vs baseline: +4.9% ✅ modulo_aspect_for_bytesTime: ✅ 14.406ms (SLO: <18.900ms 📉 -23.8%) vs baseline: ~same Memory: ✅ 43.116MB (SLO: <46.000MB -6.3%) vs baseline: +4.9% ✅ modulo_aspect_for_bytes_bytearrayTime: ✅ 14.572ms (SLO: <19.150ms 📉 -23.9%) vs baseline: ~same Memory: ✅ 43.216MB (SLO: <46.000MB -6.1%) vs baseline: +4.6% ✅ modulo_noaspectTime: ✅ 0.361ms (SLO: <3.000ms 📉 -88.0%) vs baseline: +0.2% Memory: ✅ 43.116MB (SLO: <46.000MB -6.3%) vs baseline: +5.0% ✅ replace_aspectTime: ✅ 18.344ms (SLO: <24.000ms 📉 -23.6%) vs baseline: +0.2% Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +4.8% ✅ replace_noaspectTime: ✅ 291.997µs (SLO: <300.000µs -2.7%) vs baseline: +4.5% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +5.0% ✅ repr_aspectTime: ✅ 319.135µs (SLO: <420.000µs 📉 -24.0%) vs baseline: -0.7% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +5.0% ✅ repr_noaspectTime: ✅ 46.628µs (SLO: <90.000µs 📉 -48.2%) vs baseline: ~same Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +4.6% ✅ rstrip_aspectTime: ✅ 372.542µs (SLO: <500.000µs 📉 -25.5%) vs baseline: ~same Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +4.9% ✅ rstrip_noaspectTime: ✅ 181.315µs (SLO: <300.000µs 📉 -39.6%) vs baseline: +0.2% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.6% ✅ slice_aspectTime: ✅ 182.752µs (SLO: <300.000µs 📉 -39.1%) vs baseline: +0.4% Memory: ✅ 43.136MB (SLO: <46.000MB -6.2%) vs baseline: +4.8% ✅ slice_noaspectTime: ✅ 54.348µs (SLO: <90.000µs 📉 -39.6%) vs baseline: +1.1% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.7% ✅ stringio_aspectTime: ✅ 3.947ms (SLO: <5.000ms 📉 -21.1%) vs baseline: -0.5% Memory: ✅ 43.195MB (SLO: <46.000MB -6.1%) vs baseline: +4.6% ✅ stringio_noaspectTime: ✅ 345.258µs (SLO: <500.000µs 📉 -30.9%) vs baseline: -0.5% Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.7% ✅ strip_aspectTime: ✅ 277.638µs (SLO: <350.000µs 📉 -20.7%) vs baseline: -0.5% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +4.8% ✅ strip_noaspectTime: ✅ 176.773µs (SLO: <240.000µs 📉 -26.3%) vs baseline: +0.3% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.5% ✅ swapcase_aspectTime: ✅ 339.787µs (SLO: <500.000µs 📉 -32.0%) vs baseline: +1.1% Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +4.8% ✅ swapcase_noaspectTime: ✅ 273.534µs (SLO: <400.000µs 📉 -31.6%) vs baseline: +0.4% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +5.0% ✅ title_aspectTime: ✅ 329.069µs (SLO: <500.000µs 📉 -34.2%) vs baseline: +1.1% Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.7% ✅ title_noaspectTime: ✅ 261.452µs (SLO: <400.000µs 📉 -34.6%) vs baseline: +0.1% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.9% ✅ translate_aspectTime: ✅ 544.923µs (SLO: <700.000µs 📉 -22.2%) vs baseline: 📈 +10.5% Memory: ✅ 43.155MB (SLO: <46.000MB -6.2%) vs baseline: +5.0% ✅ translate_noaspectTime: ✅ 423.480µs (SLO: <500.000µs 📉 -15.3%) vs baseline: -2.1% Memory: ✅ 43.352MB (SLO: <46.000MB -5.8%) vs baseline: +5.7% ✅ upper_aspectTime: ✅ 296.665µs (SLO: <500.000µs 📉 -40.7%) vs baseline: -0.7% Memory: ✅ 43.155MB (SLO: <46.000MB -6.2%) vs baseline: +5.3% ✅ upper_noaspectTime: ✅ 236.187µs (SLO: <400.000µs 📉 -41.0%) vs baseline: ~same Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% 📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 488.395µs (SLO: <700.000µs 📉 -30.2%) vs baseline: 📈 +14.4% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.9% ✅ ospathbasename_noaspectTime: ✅ 434.173µs (SLO: <700.000µs 📉 -38.0%) vs baseline: +0.6% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.9% ✅ ospathjoin_aspectTime: ✅ 614.870µs (SLO: <700.000µs 📉 -12.2%) vs baseline: +0.1% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.9% ✅ ospathjoin_noaspectTime: ✅ 622.581µs (SLO: <700.000µs 📉 -11.1%) vs baseline: ~same Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.9% ✅ ospathnormcase_aspectTime: ✅ 352.649µs (SLO: <700.000µs 📉 -49.6%) vs baseline: -0.5% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ ospathnormcase_noaspectTime: ✅ 359.258µs (SLO: <700.000µs 📉 -48.7%) vs baseline: -1.2% Memory: ✅ 43.057MB (SLO: <46.000MB -6.4%) vs baseline: +4.9% ✅ ospathsplit_aspectTime: ✅ 487.072µs (SLO: <700.000µs 📉 -30.4%) vs baseline: -0.8% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ ospathsplit_noaspectTime: ✅ 496.001µs (SLO: <700.000µs 📉 -29.1%) vs baseline: -0.2% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +5.0% ✅ ospathsplitdrive_aspectTime: ✅ 375.082µs (SLO: <700.000µs 📉 -46.4%) vs baseline: ~same Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +5.0% ✅ ospathsplitdrive_noaspectTime: ✅ 72.258µs (SLO: <700.000µs 📉 -89.7%) vs baseline: -1.0% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.9% ✅ ospathsplitext_aspectTime: ✅ 460.303µs (SLO: <700.000µs 📉 -34.2%) vs baseline: -0.1% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.9% ✅ ospathsplitext_noaspectTime: ✅ 470.352µs (SLO: <700.000µs 📉 -32.8%) vs baseline: +0.5% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.9% 📈 telemetryaddmetric - 30/30✅ 1-count-metric-1-timesTime: ✅ 3.406µs (SLO: <20.000µs 📉 -83.0%) vs baseline: 📈 +16.1% Memory: ✅ 35.547MB (SLO: <38.000MB -6.5%) vs baseline: +4.7% ✅ 1-count-metrics-100-timesTime: ✅ 204.128µs (SLO: <220.000µs -7.2%) vs baseline: +1.3% Memory: ✅ 35.625MB (SLO: <38.000MB -6.2%) vs baseline: +5.1% ✅ 1-distribution-metric-1-timesTime: ✅ 3.256µs (SLO: <20.000µs 📉 -83.7%) vs baseline: -0.4% Memory: ✅ 35.586MB (SLO: <38.000MB -6.4%) vs baseline: +4.8% ✅ 1-distribution-metrics-100-timesTime: ✅ 214.524µs (SLO: <230.000µs -6.7%) vs baseline: -1.2% Memory: ✅ 35.645MB (SLO: <38.000MB -6.2%) vs baseline: +4.9% ✅ 1-gauge-metric-1-timesTime: ✅ 2.202µs (SLO: <20.000µs 📉 -89.0%) vs baseline: +0.3% Memory: ✅ 35.547MB (SLO: <38.000MB -6.5%) vs baseline: +4.6% ✅ 1-gauge-metrics-100-timesTime: ✅ 136.455µs (SLO: <150.000µs -9.0%) vs baseline: -0.4% Memory: ✅ 35.527MB (SLO: <38.000MB -6.5%) vs baseline: +4.6% ✅ 1-rate-metric-1-timesTime: ✅ 3.075µs (SLO: <20.000µs 📉 -84.6%) vs baseline: -0.9% Memory: ✅ 35.606MB (SLO: <38.000MB -6.3%) vs baseline: +4.7% ✅ 1-rate-metrics-100-timesTime: ✅ 217.376µs (SLO: <250.000µs 📉 -13.0%) vs baseline: +0.4% Memory: ✅ 35.665MB (SLO: <38.000MB -6.1%) vs baseline: +5.2% ✅ 100-count-metrics-100-timesTime: ✅ 20.220ms (SLO: <22.000ms -8.1%) vs baseline: +0.8% Memory: ✅ 35.606MB (SLO: <38.000MB -6.3%) vs baseline: +4.7% ✅ 100-distribution-metrics-100-timesTime: ✅ 2.257ms (SLO: <2.550ms 📉 -11.5%) vs baseline: -0.5% Memory: ✅ 35.586MB (SLO: <38.000MB -6.4%) vs baseline: +4.7% ✅ 100-gauge-metrics-100-timesTime: ✅ 1.398ms (SLO: <1.550ms -9.8%) vs baseline: +0.1% Memory: ✅ 35.645MB (SLO: <38.000MB -6.2%) vs baseline: +4.9% ✅ 100-rate-metrics-100-timesTime: ✅ 2.191ms (SLO: <2.550ms 📉 -14.1%) vs baseline: -0.7% Memory: ✅ 35.488MB (SLO: <38.000MB -6.6%) vs baseline: +4.5% ✅ flush-1-metricTime: ✅ 4.469µs (SLO: <20.000µs 📉 -77.7%) vs baseline: +0.2% Memory: ✅ 35.566MB (SLO: <38.000MB -6.4%) vs baseline: +4.7% ✅ flush-100-metricsTime: ✅ 173.609µs (SLO: <250.000µs 📉 -30.6%) vs baseline: ~same Memory: ✅ 35.665MB (SLO: <38.000MB -6.1%) vs baseline: +4.7% ✅ flush-1000-metricsTime: ✅ 2.171ms (SLO: <2.500ms 📉 -13.1%) vs baseline: -0.2% Memory: ✅ 36.353MB (SLO: <38.750MB -6.2%) vs baseline: +4.7% 🟡 Near SLO Breach (1 suite)🟡 tracer - 6/6✅ largeTime: ✅ 31.294ms (SLO: <32.950ms -5.0%) vs baseline: -0.3% Memory: ✅ 36.864MB (SLO: <39.250MB -6.1%) vs baseline: +4.9% ✅ mediumTime: ✅ 3.092ms (SLO: <3.200ms -3.4%) vs baseline: ~same Memory: ✅ 35.232MB (SLO: <38.750MB -9.1%) vs baseline: +4.5% ✅ smallTime: ✅ 362.764µs (SLO: <370.000µs 🟡 -2.0%) vs baseline: +3.6% Memory: ✅ 35.566MB (SLO: <38.750MB -8.2%) vs baseline: +5.7%
|
Codeowners resolved as |
Description
While investigating root cause of SCP-1077, noticed that just having
from ddtrace.profiling import Profilerresulted in patching the gevent module and installing our own gevent tracer. We need to avoid that when profiler is not enabled.Regression test fails on 09f948a
Testing
Risks
Additional Notes