Skip to content

Commit 4591885

Browse files
authored
chore(profiling): remove duplicate tests and unneeded test functions (#15403)
## Description This PR updates tests to be less verbose, and to successfully pass even if we have different allocation counts between snapshots. This PR removes: - 6 deprecated test functions that were using the old `test_snapshot()` API - The `test_snapshot()` method itself from `MemoryCollector` - The `MemorySample` NamedTuple class that was only used by `test_snapshot()` These tests are safely redundant because their functionality is now comprehensively covered by newer tests that use the `snapshot_and_parse_pprof()` API, which validates against actual pprof profiles (the format we actually export). ### 1. `test_heap_profiler_sampling_accuracy()` **Why it's safe to remove:** - **Covered by:** `test_memory_collector_allocation_accuracy_with_tracemalloc()` - **Coverage:** The replacement test validates sampling accuracy using tracemalloc as ground truth, testing allocation counts and size accuracy across multiple sample intervals (256, 512, 1024 bytes) - **Improvements:** The new test uses the pprof-based approach and validates both size and count accuracy with statistical assertions ### 2. `test_iter_events()` **Why it's safe to remove:** - **Covered by:** - `test_memory_collector()` - validates allocation capture and sample structure from pprof profiles - `test_memory_collector_allocation_accuracy_with_tracemalloc()` - validates allocation counts and accuracy - **Coverage:** Both replacement tests verify that allocations are captured with correct stack traces, thread IDs, and counts from actual pprof exports ### 3. `test_iter_events_dropped()` **Why it's safe to remove:** - **Covered by:** - `test_memory_collector()` - validates that allocations are captured - `test_memory_collector_allocation_accuracy_with_tracemalloc()` - validates allocation counts meet expected thresholds - **Coverage:** These tests verify that samples aren't improperly dropped and allocation counts are accurate ### 4. `test_iter_events_not_started()` **Why it's safe to remove:** - **Edge case behavior changed:** The new `snapshot_and_parse_pprof()` API throws an error if the collector hasn't been started, rather than returning an empty tuple - **Not safety-critical:** This tested an edge case that would be caught immediately in any real usage - **Better failure mode:** An exception is more obvious than silently returning empty data ### 5. `test_iter_events_multi_thread()` **Why it's safe to remove:** - **Covered by:** - `test_memory_collector_thread_lifecycle()` - validates allocations across multiple threads that are created and destroyed dynamically - `test_memory_collector_buffer_pool_exhaustion()` - validates concurrent thread allocations with deep stack traces - **Coverage:** Both tests verify multi-threaded allocation tracking, with the newer tests being more comprehensive (testing thread lifecycle and concurrent stress scenarios) ### 6. `test_heap()` and `_test_heap_impl()` helper **Why it's safe to remove:** - **Covered by:** - `test_memory_collector_allocation_tracking_across_snapshots()` - validates heap (live) vs freed samples with deletions and garbage collection - `test_memory_collector_python_interface_with_allocation_tracking()` - validates multiple snapshot cycles with allocations and frees - **Coverage:** The replacement tests validate the same behavior (tracking live vs freed allocations) but using the pprof-based approach and with more comprehensive scenarios ## Testing <!-- Describe your testing strategy or note what tests are included --> ## Risks <!-- Note any risks associated with this change, or "None" if no risks --> ## Additional Notes <!-- Any other information that would be helpful for reviewers -->
1 parent fc99201 commit 4591885

File tree

2 files changed

+41
-383
lines changed

2 files changed

+41
-383
lines changed

ddtrace/profiling/collector/memalloc.py

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,11 @@
44
import threading
55
from types import TracebackType
66
from typing import Any
7-
from typing import List
8-
from typing import NamedTuple
97
from typing import Optional
108
from typing import Set
11-
from typing import Tuple
129
from typing import Type
1310
from typing import cast
1411

15-
from ddtrace.profiling.event import DDFrame
16-
1712

1813
try:
1914
from ddtrace.profiling.collector import _memalloc
@@ -30,17 +25,6 @@
3025
LOG = logging.getLogger(__name__)
3126

3227

33-
class MemorySample(NamedTuple):
34-
frames: List[DDFrame]
35-
size: int
36-
count: ( # pyright: ignore[reportIncompatibleMethodOverride] (count is a method of tuple)
37-
int # type: ignore[assignment]
38-
)
39-
in_use_size: int
40-
alloc_size: int
41-
thread_id: int
42-
43-
4428
class MemoryCollector:
4529
"""Memory allocation collector."""
4630

@@ -101,7 +85,7 @@ def _get_thread_id_ignore_set(self) -> Set[int]:
10185
if getattr(thread, "_ddtrace_profiling_ignore", False) and thread.ident is not None
10286
}
10387

104-
def snapshot(self) -> Tuple[MemorySample, ...]:
88+
def snapshot(self) -> None:
10589
thread_id_ignore_set = self._get_thread_id_ignore_set()
10690

10791
try:
@@ -111,7 +95,7 @@ def snapshot(self) -> Tuple[MemorySample, ...]:
11195
except (RuntimeError, ValueError):
11296
# DEV: This can happen if either _memalloc has not been started or has been stopped.
11397
LOG.debug("Unable to collect heap events from process %d", os.getpid(), exc_info=True)
114-
return tuple()
98+
return
11599

116100
for event in events:
117101
(frames, thread_id), in_use_size, alloc_size, count = event
@@ -138,31 +122,6 @@ def snapshot(self) -> Tuple[MemorySample, ...]:
138122
# re-initialization.
139123
LOG.debug("Invalid state detected in memalloc module, suppressing profile")
140124

141-
return tuple()
142-
143-
def test_snapshot(self) -> Tuple[MemorySample, ...]:
144-
thread_id_ignore_set = self._get_thread_id_ignore_set()
145-
146-
try:
147-
if _memalloc is None:
148-
raise ValueError("Memalloc is not initialized")
149-
events = _memalloc.heap()
150-
except (RuntimeError, ValueError):
151-
# DEV: This can happen if either _memalloc has not been started or has been stopped.
152-
LOG.debug("Unable to collect heap events from process %d", os.getpid(), exc_info=True)
153-
return tuple()
154-
155-
samples: List[MemorySample] = []
156-
for event in events:
157-
(frames, thread_id), in_use_size, alloc_size, count = event
158-
159-
if not self.ignore_profiler or thread_id not in thread_id_ignore_set:
160-
size = in_use_size if in_use_size > 0 else alloc_size
161-
162-
samples.append(MemorySample(frames, size, count, in_use_size, alloc_size, thread_id))
163-
164-
return tuple(samples)
165-
166125
def snapshot_and_parse_pprof(self, output_filename: str) -> Any:
167126
"""Export samples to profile, upload, and parse the pprof profile.
168127
@@ -193,6 +152,3 @@ def snapshot_and_parse_pprof(self, output_filename: str) -> Any:
193152
)
194153

195154
return pprof_utils.parse_newest_profile(output_filename)
196-
197-
def collect(self) -> Tuple[MemorySample, ...]:
198-
return tuple()

0 commit comments

Comments
 (0)