Skip to content

Commit aca2f36

Browse files
fix(profiling): add (one of?) the correct classes to lock profiling exporting [backport 1.16] (#6621)
Backport cf8de28 from #6605 to 1.16. In testing libdatadog and from recent customer questions, I learned that lock profiling isn't working the way it should. Conceptually, profiling works by adding events into a dictionary-of-lists which is keyed by the name of the inserted class. A static list of class names is checked later in order to export events which have been a-priori qualified as "lock" events. Reviewers may wonder why this wasn't spotted by tests. The tests check for the presence of instrumentation, not the result of profiling. This will probably increase overhead at time of export since we'll have more events to emit. ## Checklist - [X] Change(s) are motivated and described in the PR description. - [X] Testing strategy is described if automated tests are not included in the PR. - [X] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [X] Change is maintainable (easy to change, telemetry, documentation). - [X] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [X] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [X] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: David Sanchez <[email protected]>
1 parent 15e5269 commit aca2f36

File tree

1 file changed

+4
-1
lines changed

1 file changed

+4
-1
lines changed

ddtrace/profiling/exporter/pprof.pyx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ from ddtrace.profiling import recorder
1818
from ddtrace.profiling.collector import _lock
1919
from ddtrace.profiling.collector import memalloc
2020
from ddtrace.profiling.collector import stack_event
21+
from ddtrace.profiling.collector import threading
2122
from ddtrace.profiling.exporter import _packages
2223

2324

@@ -99,7 +100,7 @@ _pb_version = _protobuf_version()
99100
for v in [(4, 21), (3, 19), (3, 12)]:
100101
if _pb_version >= v:
101102
import sys
102-
103+
103104
pprof_module = "ddtrace.profiling.exporter.pprof_%s%s_pb2" % v
104105
__import__(pprof_module)
105106
pprof_pb2 = sys.modules[pprof_module]
@@ -687,6 +688,8 @@ class PprofExporter(exporter.Exporter):
687688
for event_class, convert_fn in (
688689
(_lock.LockAcquireEvent, converter.convert_lock_acquire_event),
689690
(_lock.LockReleaseEvent, converter.convert_lock_release_event),
691+
(threading.ThreadingLockAcquireEvent, converter.convert_lock_acquire_event),
692+
(threading.ThreadingLockReleaseEvent, converter.convert_lock_release_event),
690693
):
691694
lock_events = events.get(event_class, []) # type: ignore[call-overload]
692695
sampling_sum_pct = sum(event.sampling_pct for event in lock_events)

0 commit comments

Comments
 (0)