Skip to content

Commit cf8de28

Browse files
authored
fix(profiling): add (one of?) the correct classes to lock profiling exporting (#6605)
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: sanchda <[email protected]>
1 parent bf1b3c2 commit cf8de28

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
@@ -13,6 +13,7 @@ from ddtrace.internal import packages
1313
from ddtrace.internal._encoding import ListStringTable as _StringTable
1414
from ddtrace.internal.compat import ensure_str
1515
from ddtrace.internal.utils import config
16+
from ddtrace.profiling.collector import threading
1617
from ddtrace.profiling import event
1718
from ddtrace.profiling import exporter
1819
from ddtrace.profiling import recorder
@@ -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)