Skip to content

refactor(internal): allow an injectable sleep in the retry utility - #19425

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
mainfrom
pavlo.khrebto/adjust-retry-logic
Aug 3, 2026
Merged

refactor(internal): allow an injectable sleep in the retry utility#19425
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
mainfrom
pavlo.khrebto/adjust-retry-logic

Conversation

@pavlokhrebto

@pavlokhrebto pavlokhrebto commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Description

Adds an optional sleep_func parameter to ddtrace/internal/utils/retry.py, used for the initial wait and for each backoff between attempts. It defaults to time.sleep, so every existing caller behaves identically — the change is purely additive.

The motivation is background-thread callers: time.sleep cannot be interrupted, so a service shutting down mid-backoff has to wait out the remaining delay. Passing an interruptible wait such as threading.Event.wait lets the caller abandon the backoff immediately.

Split out of #19331 at review request. The Feature Flagging agentless poller is the first consumer and uses it to make shutdown prompt instead of waiting out up to ~30s of backoff.

Testing

Four tests added to tests/internal/test_utils_retry.py:

  • sleep_func receives the initial wait and every backoff delay, in order.
  • No backoff wait happens when the first attempt is accepted.
  • The default remains bound to time.sleep for callers that do not pass it.
  • An event-style wait can stop the remaining attempts (the interruptible-shutdown case).

tests/internal/test_utils_retry.py passes in full (9 tests).

Risks

None expected. The parameter is optional and defaults to the previous behavior, and no existing caller passes it.

Additional Notes

No release note: this is an internal utility with no customer-visible behavior change, so changelog/no-changelog is applied.

@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Codeowners resolved as

ddtrace/internal/utils/retry.py                                         @DataDog/apm-core-python
tests/internal/test_utils_retry.py                                      @DataDog/apm-core-python

@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Circular import analysis

⚠️ Existing circular imports

There are 3 circular imports that already exist on the base branch and have not been changed by this PR.

ddtrace.trace -> ddtrace._trace.tracer -> ddtrace.internal.debug -> ddtrace.trace
ddtrace -> ddtrace.trace -> ddtrace._trace.tracer -> ddtrace.internal.debug -> ddtrace
ddtrace -> ddtrace.trace -> ddtrace._trace.tracer -> ddtrace.internal.debug -> ddtrace.internal.runtime.runtime_metrics -> ddtrace

@datadog-official

datadog-official Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🔄 Datadog auto-retried 1 job - 1 passed on retry View in Datadog

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 20d09f7 | Docs | Datadog PR Page | Give us feedback!

@pavlokhrebto pavlokhrebto added the changelog/no-changelog A changelog entry is not required for this PR. label Jul 31, 2026
@pavlokhrebto
pavlokhrebto marked this pull request as ready for review July 31, 2026 10:25
@pavlokhrebto
pavlokhrebto requested a review from a team as a code owner July 31, 2026 10:25

@brettlangdon brettlangdon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit about one of the tests, but otherwise lgtm

Comment thread tests/internal/test_utils_retry.py Outdated
Comment thread tests/internal/test_utils_retry.py Outdated
@P403n1x87

Copy link
Copy Markdown
Collaborator

Interesting idea. Perhaps we can evolve the decorator to return a callable object that can be cancel()led and/or nudge()d. One thing to note is that we cannot use threading primitives from the threading library, so we would have to make a native implementation of events (we already have them in _threads.cpp but not exported to Python)

@pavlokhrebto

Copy link
Copy Markdown
Contributor Author

Interesting idea. Perhaps we can evolve the decorator to return a callable object that can be cancel()led and/or nudge()d. One thing to note is that we cannot use threading primitives from the threading library, so we would have to make a native implementation of events (we already have them in _threads.cpp but not exported to Python)

@P403n1x87 ok for a follow-up PR?

@pr-commenter

pr-commenter Bot commented Jul 31, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-07-31 14:50:26

Comparing candidate commit 20d09f7 in PR branch pavlo.khrebto/adjust-retry-logic with baseline commit 293f869 in branch main.

Found 0 performance improvements and 7 performance regressions! Performance is the same for 610 metrics, 9 unstable metrics.

scenario:httppropagationinject-ids_only

  • 🟥 execution_time [+1.553µs; +1.751µs] or [+7.110%; +8.018%]

scenario:iastaspects-repr_aspect

  • 🟥 execution_time [+40.277µs; +49.901µs] or [+10.271%; +12.725%]

scenario:iastaspects-translate_aspect

  • 🟥 execution_time [+54.059µs; +60.135µs] or [+10.945%; +12.175%]

scenario:iastaspectsospath-ospathbasename_aspect

  • 🟥 execution_time [+97.660µs; +105.281µs] or [+23.201%; +25.012%]

scenario:span-start

  • 🟥 execution_time [+1.572ms; +1.789ms] or [+9.751%; +11.094%]

scenario:telemetryaddmetric-1-count-metric-1-times

  • 🟥 execution_time [+206.090ns; +239.914ns] or [+9.797%; +11.405%]

scenario:tracer-small

  • 🟥 execution_time [+32.832µs; +35.203µs] or [+10.088%; +10.817%]

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit 59fe269 into main Aug 3, 2026
1372 of 1375 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the pavlo.khrebto/adjust-retry-logic branch August 3, 2026 10:52
brettlangdon pushed a commit that referenced this pull request Aug 3, 2026
…19425)

## Description

Adds an optional `sleep_func` parameter to `ddtrace/internal/utils/retry.py`, used for the initial wait and for each backoff between attempts. It defaults to `time.sleep`, so every existing caller behaves identically — the change is purely additive.

The motivation is background-thread callers: `time.sleep` cannot be interrupted, so a service shutting down mid-backoff has to wait out the remaining delay. Passing an interruptible wait such as `threading.Event.wait` lets the caller abandon the backoff immediately.

Split out of #19331 at review request. The Feature Flagging agentless poller is the first consumer and uses it to make shutdown prompt instead of waiting out up to ~30s of backoff.

## Testing

Four tests added to `tests/internal/test_utils_retry.py`:

- `sleep_func` receives the initial wait and every backoff delay, in order.
- No backoff wait happens when the first attempt is accepted.
- The default remains bound to `time.sleep` for callers that do not pass it.
- An event-style wait can stop the remaining attempts (the interruptible-shutdown case).

`tests/internal/test_utils_retry.py` passes in full (9 tests).

## Risks

None expected. The parameter is optional and defaults to the previous behavior, and no existing caller passes it.

## Additional Notes

No release note: this is an internal utility with no customer-visible behavior change, so `changelog/no-changelog` is applied.


Co-authored-by: pavlo.khrebto <pavlo.khrebto@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants