Skip to content

Commit

Permalink
chore(celery): fix flaky 'test_beat_scheduler_tracing' (#5717)
Browse files Browse the repository at this point in the history
This change fixes the 'test_beat_scheduler_tracing' flakiness, which was
due to the assertions expecting celery's task scheduler to run an
'expected' number of times.

The main theme is removing magic numbers. For instance "we don't expect
it to run more than 3 extra times" was removed so the logic is "as long
as it runs at least as many times as we want".

## 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/contributing.html#Release-Note-Guidelines)
are followed.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] PR description includes explicit acknowledgement/acceptance of the
performance implications of this PR as reported in the benchmarks PR
comment.

## 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.
  • Loading branch information
tabgok authored May 2, 2023
1 parent 3f01553 commit bfe0695
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions tests/contrib/celery/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -717,8 +717,9 @@ def fn_task():
sleep(run_time_seconds + 0.3)
beat_service.stop()

actual_run_count = beat_service.service.get_scheduler().schedule["mytestschedule"].total_run_count
traces = self.pop_traces()
assert len(traces) >= 30 # tick() runs a large, unpredictable amount of times
assert len(traces) >= actual_run_count
assert traces[0][0].name == "celery.beat.tick"

# the following code verifies a trace structure in which every root span is either "celery.beat.tick" or
Expand All @@ -742,15 +743,14 @@ def fn_task():
assert trace[2].name == "celery.apply"
# the number of task runs that beat schedules in this test is unpredictable
# luckily this test doesn't care about the specific number of runs as long as it's
# within a fairly wide range
max_extra_run_count = 3
assert target_task_run_count + max_extra_run_count >= deep_traces_count >= target_task_run_count
# sufficiently large to cover the number of invocations we expect
assert deep_traces_count >= target_task_run_count
assert deep_traces_count == spans_counter["celery.beat.apply_entry"]
assert deep_traces_count == spans_counter["celery.apply"]
# beat_service.stop() can happen any time during the beat thread's execution.
# When by chance it happens between apply_entry() and run(), the run() span will be
# omitted, resulting in one fewer span for run() than the other functions
assert target_task_run_count >= spans_counter["celery.run"] >= target_task_run_count - 1
assert actual_run_count >= spans_counter["celery.run"] >= actual_run_count - 1


class CeleryDistributedTracingIntegrationTask(CeleryBaseTestCase):
Expand Down

0 comments on commit bfe0695

Please sign in to comment.