Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report timing information if try_wait_until times out #4265

Merged
merged 4 commits into from
Jan 8, 2025

Conversation

p-datadog
Copy link
Member

What does this PR do?

Adds timing information to the exception message if try_wait_until fails.

Motivation:

There are flaky tests that fail with this timeout but do not provide much information to determine whether a longer wait time is appropriate (example: https://app.circleci.com/pipelines/github/DataDog/dd-trace-rb/18420/workflows/05d20861-e600-4a0a-a1a1-5d39e964a755/jobs/648349)

Change log entry
None

Additional Notes:
Example message produced:

        Wait time exhausted! Requested: none, waited: 0.00 seconds, 50 attempts with backoff 0.1

How to test the change?

Not applicable

@p-datadog p-datadog requested a review from a team as a code owner January 8, 2025 05:31
@github-actions github-actions bot added the dev/testing Involves testing processes (e.g. RSpec) label Jan 8, 2025
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Jan 8, 2025

Datadog Report

Branch report: report-wait-until
Commit report: f2ad94f
Test service: dd-trace-rb

✅ 0 Failed, 24606 Passed, 1515 Skipped, 5m 29.35s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

  • Server internal tracer traces the looping scheduled wait - rspec - Last Failure

    Expand for error
     Wait time exhausted! Requested: 10 seconds, waited: 10.02 seconds, 100 attempts with backoff 0.1
     
     Failure/Error: raise("Wait time exhausted! Requested: #{spec}, waited: #{actual}")
     
     RuntimeError:
       Wait time exhausted! Requested: 10 seconds, waited: 10.02 seconds, 100 attempts with backoff 0.1
     ./spec/support/synchronization_helpers.rb:110:in \`try_wait_until'
     ./spec/support/synchronization_helpers.rb:26:in \`expect_in_fork'
     ./spec/datadog/tracing/contrib/sidekiq/support/helper.rb:52:in \`expect_in_sidekiq_server'
     ./spec/datadog/tracing/contrib/sidekiq/server_internal_tracer/scheduled_poller_spec.rb:49:in \`block (2 levels) in <top (required)>'
     ...
    

@pr-commenter
Copy link

pr-commenter bot commented Jan 8, 2025

Benchmarks

Benchmark execution time: 2025-01-08 14:57:40

Comparing candidate commit f2ad94f in PR branch report-wait-until with baseline commit b86820e in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 30 metrics, 2 unstable metrics.

scenario:tracing - Propagation - Trace Context

  • 🟩 throughput [+4544.272op/s; +4653.901op/s] or [+13.362%; +13.684%]

Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@p-datadog p-datadog merged commit e3ea38f into master Jan 8, 2025
335 of 337 checks passed
@p-datadog p-datadog deleted the report-wait-until branch January 8, 2025 15:07
@github-actions github-actions bot added this to the 2.9.0 milestone Jan 8, 2025
p-datadog pushed a commit to p-datadog/dd-trace-rb that referenced this pull request Jan 9, 2025
* master:
  DEBUG-3210 DI: change logging to be appropriate for customer inspection (DataDog#4266)
  Report timing information if try_wait_until times out (DataDog#4265)
  Move simplecov patch to an overlay in our tree instead of using a forked simplecov repo (DataDog#4263)
  DEBUG-3251 dependency inject logger into DI component (DataDog#4262)
  DEBUG-3182 move Rails utils to core (DataDog#4261)
  add supported versions workflow (DataDog#4210)
  DEBUG-3305 remove dependency on benchmark (DataDog#4259)
  Fix case & grammar in issue template (DataDog#4244)
  [🤖] Update Latest Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/12614773826
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/testing Involves testing processes (e.g. RSpec)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants