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

DEBUG-3175 DI snapshot/rate limit benchmark #4207

Merged
merged 5 commits into from
Dec 18, 2024
Merged

Conversation

p-datadog
Copy link
Member

@p-datadog p-datadog commented Dec 9, 2024

What does this PR do?

Adds dynamic instrumentation benchmarks verifying that the rate at which probes can capture, serialize and submit snapshots to the agent are below the rate limits.

It also makes the probe notifier worker queue capacity configurable via an internal setting. With default DI settings, DI will permit no more than 30 snapshots/second on average across all probes which is well short of the 5000/second/probe that this benchmark is aiming to achieve. The benchmark raises the limit (and makes flushes more frequent) to demonstrate theoretical performance of DI implementation, however more work is needed to permit higher limits by default - flushing logic must be carefully written to not starve the application of CPU. The default limits are very conservative in this regard.

Motivation:

We want to ensure that the rate limits we set are achievable, meaning they are not set so high that DI code can starve customer applications of CPU and still not hit rate limits.

Change log entry

None.

Additional Notes:

The benchmarks perform as many invocations of target methods as each probe type would permit (5000/second for basic probes, 1/second for enriched probes). A rate of greater than 1 instruction/second in the benchmark means the rate limit was not reached by the test code and thus DI performance is within expectations.

How to test the change?

N/A

@p-datadog p-datadog requested a review from a team as a code owner December 9, 2024 14:08
Copy link

github-actions bot commented Dec 9, 2024

👋 Hey @p-datadog, please fill "Change log entry" section in the pull request description.

If changes need to be present in CHANGELOG.md you can state it this way

**Change log entry**

Yes. A brief summary to be placed into the CHANGELOG.md

(possible answers Yes/Yep/Yeah)

Or you can opt out like that

**Change log entry**

None.

(possible answers No/Nope/None)

Visited at: 2024-12-09 21:35:47 UTC

@p-datadog p-datadog force-pushed the di-rate-limit-benchmark branch from 2671908 to b5045c8 Compare December 9, 2024 14:22
@pr-commenter
Copy link

pr-commenter bot commented Dec 9, 2024

Benchmarks

Benchmark execution time: 2024-12-18 06:15:12

Comparing candidate commit fda26e8 in PR branch di-rate-limit-benchmark with baseline commit c07a8d4 in branch master.

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

scenario:profiler - sample timeline=false

  • 🟥 throughput [-0.550op/s; -0.541op/s] or [-8.340%; -8.216%]

@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.77%. Comparing base (c07a8d4) to head (fda26e8).
Report is 104 commits behind head on master.

Files with missing lines Patch % Lines
spec/datadog/di/validate_benchmarks_spec.rb 88.88% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4207   +/-   ##
=======================================
  Coverage   97.77%   97.77%           
=======================================
  Files        1357     1357           
  Lines       81973    81983   +10     
  Branches     4168     4170    +2     
=======================================
+ Hits        80145    80162   +17     
+ Misses       1828     1821    -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions github-actions bot added the dev/testing Involves testing processes (e.g. RSpec) label Dec 9, 2024
@p-datadog p-datadog force-pushed the di-rate-limit-benchmark branch from ca27dae to 4f443b9 Compare December 9, 2024 15:14
@p-datadog p-datadog marked this pull request as draft December 9, 2024 15:20
@p-datadog p-datadog marked this pull request as ready for review December 9, 2024 21:31
@p-datadog p-datadog changed the title DEBUG-3175 DI rate limit benchmark DEBUG-3175 DI snapshot/rate limit benchmark Dec 9, 2024
@p-datadog p-datadog added the dev/internal Other internal work that does not need to be included in the changelog label Dec 9, 2024
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Dec 12, 2024

Datadog Report

Branch report: di-rate-limit-benchmark
Commit report: fda26e8
Test service: dd-trace-rb

✅ 0 Failed, 22237 Passed, 1458 Skipped, 5m 58.82s Total Time

@p-datadog p-datadog force-pushed the di-rate-limit-benchmark branch from 55c04e4 to fda26e8 Compare December 18, 2024 05:51
@p-datadog p-datadog merged commit 7fd1feb into master Dec 18, 2024
340 of 341 checks passed
@p-datadog p-datadog deleted the di-rate-limit-benchmark branch December 18, 2024 14:04
@github-actions github-actions bot added this to the 2.9.0 milestone Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/internal Other internal work that does not need to be included in the changelog dev/testing Involves testing processes (e.g. RSpec)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants