Add prom bench, coroutinze prom #3018
Open
+288
−43
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
travisdowns
force-pushed
the
td-upstream-metrics-bench
branch
from
af3732d to
52f9abc
Compare
Introduce metrics_perf benchmark. This is a precursor to work in upcoming changes to: - Couroutinize metrics endpoint - Improve the perf of the prometheus endpoint It tests a few flavors of metrics configurations that stress different parts of the core metrics response loop.
There was no existing test coverage of the feature of adding a label to all series in the seastar-side prometheus config. Add such a test case to the prom http test.
Coroutinize and de-boilerplate up the prometheus metrics writing path. We do not couroutinze the innermost loop, as this is using ss::async and currently relies on that. This is a precursor to performance improvements on this path. This change is more or less performance neutral: there is a slight "fixed cost" win for the body writing, but this doesn't matter much in the larger context since the per-series overhead dominates and there is no change in that case (for this change).
travisdowns
force-pushed
the
td-upstream-metrics-bench
branch
from
52f9abc to
eff3dc1
Compare
|
Please fix spelling, especially "prom", I'm allergic. |
avikivity
reviewed
Oct 3, 2025
| #include <seastar/testing/perf_tests.hh> | ||
| #include <seastar/util/tmp_file.hh> | ||
|
|
||
| #include <boost/range/irange.hpp> |
There was a problem hiding this comment.
For new code, let's avoid boost::ranges, std::views::subrange is the replacement.
avikivity
reviewed
Oct 3, 2025
| using namespace seastar; | ||
| using namespace seastar::metrics; | ||
|
|
||
| remove_existing_metrics(); |
There was a problem hiding this comment.
Not an expert on fixtures, but wouldn't that poison global state? So if a later test dependent on standard metrics, it would fail.
|
Looks good. The results are normalized per-metric? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Three primary changes related to prometheus endpoint:
These are precursors to a perf improvement I want to make.
Here is the benchmark output before/after the coroutine change:
before:
after
As expected the differences are ~nil, below the noise floor, as the coroutines are only introduced around a relatively long-running function.