Skip to content

Commit

Permalink
ci: mark flaky tests in the profile suite (#9170)
Browse files Browse the repository at this point in the history
This change temporarily skips unreliable tests in the `profile` suite.
Recent failures of these tests on the main branch:


https://app.circleci.com/pipelines/github/DataDog/dd-trace-py/60782/workflows/078778dd-deb8-4c4d-bb3c-1325370eba89/jobs/3812548

https://app.circleci.com/pipelines/github/DataDog/dd-trace-py/60798/workflows/7a11bd02-5a05-412e-9e07-c98dd0dd85ea/jobs/3813849

https://app.circleci.com/pipelines/github/DataDog/dd-trace-py/60814/workflows/80c5392d-0991-401f-8119-c290a15141b9/jobs/3814905

## 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] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [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))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has 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)
  • Loading branch information
emmettbutler authored May 6, 2024
1 parent 43ed11f commit 242e2b0
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 17 deletions.
1 change: 1 addition & 0 deletions tests/profiling/collector/test_memalloc.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ def test_memory_collector():
assert count_object > 0


@flaky(1719591602)
@pytest.mark.parametrize(
"ignore_profiler",
(True, False),
Expand Down
36 changes: 19 additions & 17 deletions tests/profiling/collector/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from ddtrace.profiling import recorder
from ddtrace.profiling.collector import threading as collector_threading
from tests.utils import flaky

from . import test_collector

Expand Down Expand Up @@ -67,13 +68,13 @@ def test_lock_acquire_events():
assert len(r.events[collector_threading.ThreadingLockAcquireEvent]) == 1
assert len(r.events[collector_threading.ThreadingLockReleaseEvent]) == 0
event = r.events[collector_threading.ThreadingLockAcquireEvent][0]
assert event.lock_name == "test_threading.py:65"
assert event.lock_name == "test_threading.py:66"
assert event.thread_id == _thread.get_ident()
assert event.wait_time_ns >= 0
# It's called through pytest so I'm sure it's gonna be that long, right?
assert len(event.frames) > 3
assert event.nframes > 3
assert event.frames[0] == (__file__.replace(".pyc", ".py"), 66, "test_lock_acquire_events", "")
assert event.frames[0] == (__file__.replace(".pyc", ".py"), 67, "test_lock_acquire_events", "")
assert event.sampling_pct == 100


Expand All @@ -91,13 +92,13 @@ def lockfunc(self):
assert len(r.events[collector_threading.ThreadingLockAcquireEvent]) == 1
assert len(r.events[collector_threading.ThreadingLockReleaseEvent]) == 0
event = r.events[collector_threading.ThreadingLockAcquireEvent][0]
assert event.lock_name == "test_threading.py:86"
assert event.lock_name == "test_threading.py:87"
assert event.thread_id == _thread.get_ident()
assert event.wait_time_ns >= 0
# It's called through pytest so I'm sure it's gonna be that long, right?
assert len(event.frames) > 3
assert event.nframes > 3
assert event.frames[0] == (__file__.replace(".pyc", ".py"), 87, "lockfunc", "Foobar")
assert event.frames[0] == (__file__.replace(".pyc", ".py"), 88, "lockfunc", "Foobar")
assert event.sampling_pct == 100


Expand All @@ -118,14 +119,14 @@ def test_lock_events_tracer(tracer):
events = r.reset()
# The tracer might use locks, so we need to look into every event to assert we got ours
for event_type in (collector_threading.ThreadingLockAcquireEvent, collector_threading.ThreadingLockReleaseEvent):
assert {"test_threading.py:109", "test_threading.py:112"}.issubset({e.lock_name for e in events[event_type]})
assert {"test_threading.py:110", "test_threading.py:113"}.issubset({e.lock_name for e in events[event_type]})
for event in events[event_type]:
if event.name == "test_threading.py:85":
if event.name == "test_threading.py:86":
assert event.trace_id is None
assert event.span_id is None
assert event.trace_resource_container is None
assert event.trace_type is None
elif event.name == "test_threading.py:88":
elif event.name == "test_threading.py:89":
assert event.trace_id == trace_id
assert event.span_id == span_id
assert event.trace_resource_container[0] == t.resource
Expand All @@ -151,14 +152,14 @@ def test_lock_events_tracer_late_finish(tracer):
events = r.reset()
# The tracer might use locks, so we need to look into every event to assert we got ours
for event_type in (collector_threading.ThreadingLockAcquireEvent, collector_threading.ThreadingLockReleaseEvent):
assert {"test_threading.py:140", "test_threading.py:143"}.issubset({e.lock_name for e in events[event_type]})
assert {"test_threading.py:141", "test_threading.py:144"}.issubset({e.lock_name for e in events[event_type]})
for event in events[event_type]:
if event.name == "test_threading.py:116":
if event.name == "test_threading.py:117":
assert event.trace_id is None
assert event.span_id is None
assert event.trace_resource_container is None
assert event.trace_type is None
elif event.name == "test_threading.py:119":
elif event.name == "test_threading.py:120":
assert event.trace_id == trace_id
assert event.span_id == span_id
assert event.trace_resource_container[0] == span.resource
Expand All @@ -183,14 +184,14 @@ def test_resource_not_collected(monkeypatch, tracer):
events = r.reset()
# The tracer might use locks, so we need to look into every event to assert we got ours
for event_type in (collector_threading.ThreadingLockAcquireEvent, collector_threading.ThreadingLockReleaseEvent):
assert {"test_threading.py:174", "test_threading.py:177"}.issubset({e.lock_name for e in events[event_type]})
assert {"test_threading.py:175", "test_threading.py:178"}.issubset({e.lock_name for e in events[event_type]})
for event in events[event_type]:
if event.name == "test_threading.py:150":
if event.name == "test_threading.py:151":
assert event.trace_id is None
assert event.span_id is None
assert event.trace_resource_container is None
assert event.trace_type is None
elif event.name == "test_threading.py:153":
elif event.name == "test_threading.py:154":
assert event.trace_id == trace_id
assert event.span_id == span_id
assert event.trace_resource_container is None
Expand All @@ -206,13 +207,13 @@ def test_lock_release_events():
assert len(r.events[collector_threading.ThreadingLockAcquireEvent]) == 1
assert len(r.events[collector_threading.ThreadingLockReleaseEvent]) == 1
event = r.events[collector_threading.ThreadingLockReleaseEvent][0]
assert event.lock_name == "test_threading.py:203"
assert event.lock_name == "test_threading.py:204"
assert event.thread_id == _thread.get_ident()
assert event.locked_for_ns >= 0
# It's called through pytest so I'm sure it's gonna be that long, right?
assert len(event.frames) > 3
assert event.nframes > 3
assert event.frames[0] == (__file__.replace(".pyc", ".py"), 205, "test_lock_release_events", "")
assert event.frames[0] == (__file__.replace(".pyc", ".py"), 206, "test_lock_release_events", "")
assert event.sampling_pct == 100


Expand Down Expand Up @@ -246,7 +247,7 @@ def play_with_lock():
assert len(r.events[collector_threading.ThreadingLockReleaseEvent]) >= 1

for event in r.events[collector_threading.ThreadingLockAcquireEvent]:
if event.lock_name == "test_threading.py:236":
if event.lock_name == "test_threading.py:237":
assert event.wait_time_ns >= 0
assert event.task_id == t.ident
assert event.task_name == "foobar"
Expand All @@ -260,7 +261,7 @@ def play_with_lock():
pytest.fail("Lock event not found")

for event in r.events[collector_threading.ThreadingLockReleaseEvent]:
if event.lock_name == "test_threading.py:236":
if event.lock_name == "test_threading.py:237":
assert event.locked_for_ns >= 0
assert event.task_id == t.ident
assert event.task_name == "foobar"
Expand Down Expand Up @@ -315,6 +316,7 @@ def test_lock_acquire_release_speed(benchmark):
benchmark(_lock_acquire_release, threading.Lock())


@flaky(1719591602)
@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="only works on linux")
@pytest.mark.subprocess(
ddtrace_run=True,
Expand Down
2 changes: 2 additions & 0 deletions tests/profiling/test_uwsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import pytest

from tests.contrib.uwsgi import run_uwsgi
from tests.utils import flaky

from . import utils

Expand Down Expand Up @@ -49,6 +50,7 @@ def test_uwsgi_threads_disabled(uwsgi):
assert THREADS_MSG in stdout


@flaky(1719591602)
def test_uwsgi_threads_number_set(uwsgi):
proc = uwsgi("--threads", "1")
try:
Expand Down

0 comments on commit 242e2b0

Please sign in to comment.