From 5ea349e6fd8002e83e9b3ac0c90e5ab9ad0b4bd1 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Tue, 7 Jan 2025 09:49:20 -0500 Subject: [PATCH 1/2] ci: set CMAKE_BUILD_PARALLEL_LEVEL for mac builds (#11863) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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) --- .github/workflows/build_python_3.yml | 4 ++++ pyproject.toml | 6 ------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build_python_3.yml b/.github/workflows/build_python_3.yml index fac67e45f82..a3d6066dc0c 100644 --- a/.github/workflows/build_python_3.yml +++ b/.github/workflows/build_python_3.yml @@ -132,6 +132,10 @@ jobs: CIBW_BEFORE_ALL_WINDOWS: rustup target add i686-pc-windows-msvc CIBW_BEFORE_ALL_MACOS: rustup target add aarch64-apple-darwin CIBW_ENVIRONMENT_LINUX: PATH=$HOME/.cargo/bin:$PATH CMAKE_BUILD_PARALLEL_LEVEL=24 + # SYSTEM_VERSION_COMPAT is a workaround for versioning issue, a.k.a. + # `platform.mac_ver()` reports incorrect MacOS version at 11.0 + # See: https://stackoverflow.com/a/65402241 + CIBW_ENVIRONMENT_MACOS: CMAKE_BUILD_PARALLEL_LEVEL=24 SYSTEM_VERSION_COMPAT=0 CIBW_REPAIR_WHEEL_COMMAND_LINUX: | mkdir ./tempwheelhouse && unzip -l {wheel} | grep '\.so' && diff --git a/pyproject.toml b/pyproject.toml index a42954445ff..df5fbdcdbb2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -204,12 +204,6 @@ test-command = ["python {project}/tests/smoke_test.py"] # Skip trying to test arm64 builds on Intel Macs test-skip = "*-macosx_universal2:arm64" -[tool.cibuildwheel.macos.environment] -# Workaround for Macos 11.0 versioning issue, a.k.a. -# `platform.mac_ver()` reports incorrect MacOS version at 11.0 -# See: https://stackoverflow.com/a/65402241 -SYSTEM_VERSION_COMPAT = "0" - [tool.ruff] exclude = [ ".riot", From 268d3f41ccc154191315ffe0a2b4076a8fa8b3c4 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Tue, 7 Jan 2025 09:49:36 -0500 Subject: [PATCH 2/2] chore(profiling): memory profiler tests with libdd exporter (#11714) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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) --- tests/profiling_v2/collector/test_memalloc.py | 116 ++++++++++++++++-- 1 file changed, 109 insertions(+), 7 deletions(-) diff --git a/tests/profiling_v2/collector/test_memalloc.py b/tests/profiling_v2/collector/test_memalloc.py index 2787507bdce..881a64a162c 100644 --- a/tests/profiling_v2/collector/test_memalloc.py +++ b/tests/profiling_v2/collector/test_memalloc.py @@ -1,7 +1,10 @@ import os +import threading -from ddtrace.profiling import Profiler -from ddtrace.settings.profiling import config +import pytest + +from ddtrace.internal.datadog.profiling import ddup +from ddtrace.profiling.collector import memalloc from tests.profiling.collector import pprof_utils @@ -9,12 +12,22 @@ def _allocate_1k(): return [object() for _ in range(1000)] -def test_heap_samples_collected(tmp_path, monkeypatch): +_ALLOC_LINE_NUMBER = _allocate_1k.__code__.co_firstlineno + 1 + + +# This test is marked as subprocess as it changes default heap sample size +@pytest.mark.subprocess( + env=dict(DD_PROFILING_HEAP_SAMPLE_SIZE="1024", DD_PROFILING_OUTPUT_PPROF="/tmp/test_heap_samples_collected") +) +def test_heap_samples_collected(): + import os + + from ddtrace.profiling import Profiler + from tests.profiling.collector import pprof_utils + from tests.profiling_v2.collector.test_memalloc import _allocate_1k + # Test for https://github.com/DataDog/dd-trace-py/issues/11069 - test_name = "test_heap" - pprof_prefix = str(tmp_path / test_name) - monkeypatch.setattr(config, "output_pprof", pprof_prefix) - monkeypatch.setattr(config.heap, "sample_size", 1024) + pprof_prefix = os.environ["DD_PROFILING_OUTPUT_PPROF"] output_filename = pprof_prefix + "." + str(os.getpid()) p = Profiler() @@ -25,3 +38,92 @@ def test_heap_samples_collected(tmp_path, monkeypatch): profile = pprof_utils.parse_profile(output_filename) samples = pprof_utils.get_samples_with_value_type(profile, "heap-space") assert len(samples) > 0 + + +def test_memory_collector(tmp_path): + test_name = "test_memory_collector" + pprof_prefix = str(tmp_path / test_name) + output_filename = pprof_prefix + "." + str(os.getpid()) + + ddup.config( + service=test_name, + version="test", + env="test", + output_filename=pprof_prefix, + ) + ddup.start() + + mc = memalloc.MemoryCollector(None) + with mc: + _allocate_1k() + mc.periodic() + + ddup.upload() + + profile = pprof_utils.parse_profile(output_filename) + # Gets samples with alloc-space > 0 + samples = pprof_utils.get_samples_with_value_type(profile, "alloc-space") + + assert len(samples) > 0 + + alloc_samples_idx = pprof_utils.get_sample_type_index(profile, "alloc-samples") + for sample in samples: + # We also want to check 'alloc-samples' is > 0. + assert sample.value[alloc_samples_idx] > 0 + + # We also want to assert that there's a sample that's coming from _allocate_1k() + # And also assert that it's actually coming from _allocate_1k() + pprof_utils.assert_profile_has_sample( + profile, + samples, + expected_sample=pprof_utils.StackEvent( + thread_name="MainThread", + thread_id=threading.main_thread().ident, + locations=[ + pprof_utils.StackLocation( + function_name="_allocate_1k", filename="test_memalloc.py", line_no=_ALLOC_LINE_NUMBER + ) + ], + ), + ) + + +def test_memory_collector_ignore_profiler(tmp_path): + test_name = "test_memory_collector_ignore_profiler" + pprof_prefix = str(tmp_path / test_name) + output_filename = pprof_prefix + "." + str(os.getpid()) + + ddup.config( + service=test_name, + version="test", + env="test", + output_filename=pprof_prefix, + ) + ddup.start() + + mc = memalloc.MemoryCollector(None, ignore_profiler=True) + quit_thread = threading.Event() + + with mc: + + def alloc(): + _allocate_1k() + quit_thread.wait() + + alloc_thread = threading.Thread(name="allocator", target=alloc) + alloc_thread._ddtrace_profiling_ignore = True + alloc_thread.start() + + mc.periodic() + + # We need to wait for the data collection to happen so it gets the `_ddtrace_profiling_ignore` Thread attribute from + # the global thread list. + quit_thread.set() + alloc_thread.join() + + ddup.upload() + + try: + pprof_utils.parse_profile(output_filename) + except AssertionError as e: + assert "No samples found" in str(e)