Repository navigation
benchmarking: run the observability ladder as one test - #1071
Merged
Max Smythe (maxsmythe) merged 3 commits intoAug 22, 2026
Merged
Conversation
Jeff Luo (JeffLuoo)
requested review from
Da Huang (git286) and
Max Smythe (maxsmythe)
August 19, 2026 19:21
Max Smythe (maxsmythe)
requested changes
Aug 21, 2026
Max Smythe (maxsmythe)
left a comment
Max Smythe (maxsmythe)
left a comment
Collaborator
There was a problem hiding this comment.
Looks good! Main feedback is to try to separate test and shape definitions from runner.py code to help establish a pattern.
I expect there will be multiple new shapes/tests in the future.
| # Optional `flags` is a list of arbitrary strings appended to runner.py's | ||
| # argv; runner.py forwards them to locust verbatim. | ||
| # | ||
| # `--ladder users:duration[:trace_probability],...` in `flags` turns the test |
Collaborator
There was a problem hiding this comment.
how does trace probability set in the ladder interact with trace probability set via flag?
Collaborator
Author
There was a problem hiding this comment.
--trace-probability gives the rate before the first step that names one. A step that names a rate changes it when the step starts, and that rate stays until another step changes it. Thus a ladder with no rate in any step keeps the flag from start to end.
Also make it clear in a comment.
|
|
||
| # Loaded next to the test file for a --ladder run. It holds no user class, so | ||
| # locust must never get it on its own. | ||
| LADDER_SHAPE = str(Path(__file__).resolve().parent / "shapes" / "ladder_shape.py") |
Collaborator
There was a problem hiding this comment.
To help maintain extensibility, can we just require the tests.yaml author to explicitly mix in the ladder_shape.py file? I believe file takes a comma-separated list of paths (perhaps it should have been "files")
This way users can mix and match test classes and load shapes and runner.py doesn't need to be as coupled to the test suites.
Collaborator
Author
There was a problem hiding this comment.
Done - Decoupled runner.py completely from ladder_shape.py
| f" duration: {args.duration}", | ||
| f" users: {args.users}", | ||
| f" uses_boomer: {needs_boomer(args.file)}", | ||
| f" ladder: {ladder_spec(args.locust_extra) or '(none)'}", |
Collaborator
There was a problem hiding this comment.
Instead of making special flags per-test, maybe we can make the "extra flags" output more legible (e.g. not all in one line, but multiple indented lines)
Collaborator
Author
There was a problem hiding this comment.
Done. Removed the dedicated ladder log line and added pair_flags() to format all extra flags across multiple indented lines with flags and values grouped together:
==== Run config ====
name: observability_s1_user_sweep
...
extra flags:
--ladder 5:3m,10:3m,15:3m
--trace-probability 0.1
--min-wait-time 1.0
--max-wait-time 1.0
====================
| with_boomer = needs_boomer(args.file) | ||
| ladder = ladder_spec(args.locust_extra) | ||
|
|
||
| # A ladder holds a step's user count for the step's duration and then |
Collaborator
There was a problem hiding this comment.
I think if we want to evaluate test-specific or shape-specific arguments, we should do that where the test or shape is defined to try to maintain separation-of-concerns
Collaborator
Author
There was a problem hiding this comment.
Updated
Removed ladder_spec() and all custom ladder inspection from runner.py.
Jeff Luo (JeffLuoo)
added a commit
to JeffLuoo/substrate
that referenced
this pull request
Aug 21, 2026
Review of agent-substrate#1071: a test and a shape must not be known to runner.py, thus a new shape needs no change there. `file` in tests.yaml now takes a comma-separated list, and the four observability tests name the shape themselves. runner.py gives the value to locust with no change, thus it holds no path of a shape and no test of the --ladder flag. A list of files breaks two values that read one path. needs_boomer took the basename of the whole list, thus a Python test with a shape gave boomer, which is wrong. The --user-class of boomer took the stem of the whole list, thus it became the name of the shape and boomer could not start. Add test_file(), which removes each path in the shapes directory and returns what stays. The result does not change with the order of the paths. The two conditions on the ladder in runner.py go away: * locust ignores -t when a shape is present and prints that it ignores it, thus runner.py can give -t for each run. * /boomer-config is now on for each headless run that uses boomer, and not for a ladder only. The shape cannot give a flag to boomer, thus this wiring cannot move to the shape. It is not specific to a ladder: it gives boomer each value that changes while a run continues. The banner of the run gives one flag and its value for each line, in place of the line for the ladder and one line for all the flags. Record how --trace-probability and a step of the ladder interact, in tests.yaml and in the shape: the flag gives the rate before the first step that names one, and a step that names a rate holds it until another step changes it.
Jeff Luo (JeffLuoo)
force-pushed
the
obs-ladder-automation
branch
from
August 21, 2026 14:53
25c58ad to
988240e
Compare
Jeff Luo (JeffLuoo)
added a commit
to JeffLuoo/substrate
that referenced
this pull request
Aug 21, 2026
Review of agent-substrate#1071: a test and a shape must not be known to runner.py, thus a new shape needs no change there. `file` in tests.yaml now takes a comma-separated list, and the four observability tests name the shape themselves. runner.py gives the value to locust with no change, thus it holds no path of a shape and no test of the --ladder flag. A list of files breaks two values that read one path. needs_boomer took the basename of the whole list, thus a Python test with a shape gave boomer, which is wrong. The --user-class of boomer took the stem of the whole list, thus it became the name of the shape and boomer could not start. Add test_file(), which removes each path in the shapes directory and returns what stays. The result does not change with the order of the paths. The two conditions on the ladder in runner.py go away: * locust ignores -t when a shape is present and prints that it ignores it, thus runner.py can give -t for each run. * /boomer-config is now on for each headless run that uses boomer, and not for a ladder only. The shape cannot give a flag to boomer, thus this wiring cannot move to the shape. It is not specific to a ladder: it gives boomer each value that changes while a run continues. The banner of the run gives one flag and its value for each line, in place of the line for the ladder and one line for all the flags. Record how --trace-probability and a step of the ladder interact, in tests.yaml and in the shape: the flag gives the rate before the first step that names one, and a step that names a rate holds it until another step changes it.
Jeff Luo (JeffLuoo)
added a commit
to JeffLuoo/substrate
that referenced
this pull request
Aug 21, 2026
A run of the sample-rate sweep on a cluster showed that the sweep does nothing. The three steps of that scenario hold 10 users and change the sample rate only. The shape gave each rate to the master at the correct time, and boomer kept the rate of the first step for the whole run: 15:00:27 Ladder step 1/3: trace_probability=0.0 15:00:27 Shape test updating to 10 users at 5.00 spawn rate 15:03:28 Ladder step 2/3: trace_probability=0.1 15:06:27 Ladder step 3/3: trace_probability=1.0 rates that boomer applied: 0 The cause is in the shape worker of locust, which sends a spawn message only when the number of users or the spawn rate changes. SubscribeSpawn reads /boomer-config at a spawn message, thus a step that holds the number of users gives this worker no message and no new value. The run continues and gives a table of three steps at one rate, which a reader cannot tell from a correct one. Add StartPoll, which reads the endpoint on an interval also, and --config-poll-interval to set it. The default is 10s. Only a change goes to the log, because a poll of each few seconds for the length of a soak would hide the run. A failed poll is not fatal, unlike a failed fetch at a spawn message: the worker holds the last good value, and one failed poll of a long run is not a reason to lose the run. The end of the run stops a fetch that is in progress, and that is not a failed fetch. Follow-up to agent-substrate#1071.
The scenario ladder was a table in observability.md that an operator followed by hand. Each step was a separate run, thus each step also carried a deploy, a set of cold caches, and a new baseline for the reader to align. The sample-rate sweep needed a new run for each rate, because the rate reached the boomer worker one time, at its start. Add shapes/ladder_shape.py and put the ladder in tests.yaml. `--ladder users:duration[:trace_probability],...` makes the steps of a sweep one run. The shape holds each step for its duration and then goes to the next one, thus it owns the end of the run and runner.py forwards no `-t` for a ladder. A step that names a probability changes the sample rate as it starts: the value goes to the parsed options of the master, thus a Python user class reads it from the sampler of common.trace and a boomer worker reads it from /boomer-config at the spawn message that the step change makes. A headless master has no Flask app, thus it had no /boomer-config and runner.py gave boomer the values one time through --config-json. That is sufficient for a run that holds one set of values, and not for a ladder. Serve the same payload from a plain HTTP server on the loopback interface when --boomer-config-port is set, which runner.py sets for a ladder along with the same port as boomer's --master-web-port. The four tests run on glutton, and thus on boomer. maxsmythe called this out on agent-substrate#749: Python and gRPC hold each other at a high number of users, so a ladder on SleepUser would put the latency of the load generator in the measurement. The user sweep stops at 15 users, and not at the 30 of the earlier table, because the pool of the run is 10 workers; a step above that measures the queue of the scheduler. The idle floor sends no request on purpose, thus locust writes no measurement row for it and runner.py counted the run as a failure. Add --allow-empty-stats for that one case, rather than accepting an empty result for every test. Follow-up to agent-substrate#749.
Review of agent-substrate#1071: a test and a shape must not be known to runner.py, thus a new shape needs no change there. `file` in tests.yaml now takes a comma-separated list, and the four observability tests name the shape themselves. runner.py gives the value to locust with no change, thus it holds no path of a shape and no test of the --ladder flag. A list of files breaks two values that read one path. needs_boomer took the basename of the whole list, thus a Python test with a shape gave boomer, which is wrong. The --user-class of boomer took the stem of the whole list, thus it became the name of the shape and boomer could not start. Add test_file(), which removes each path in the shapes directory and returns what stays. The result does not change with the order of the paths. The two conditions on the ladder in runner.py go away: * locust ignores -t when a shape is present and prints that it ignores it, thus runner.py can give -t for each run. * /boomer-config is now on for each headless run that uses boomer, and not for a ladder only. The shape cannot give a flag to boomer, thus this wiring cannot move to the shape. It is not specific to a ladder: it gives boomer each value that changes while a run continues. The banner of the run gives one flag and its value for each line, in place of the line for the ladder and one line for all the flags. Record how --trace-probability and a step of the ladder interact, in tests.yaml and in the shape: the flag gives the rate before the first step that names one, and a step that names a rate holds it until another step changes it.
A run of the sample-rate sweep on a cluster showed that the sweep does nothing. The three steps of that scenario hold 10 users and change the sample rate only. The shape gave each rate to the master at the correct time, and boomer kept the rate of the first step for the whole run: 15:00:27 Ladder step 1/3: trace_probability=0.0 15:00:27 Shape test updating to 10 users at 5.00 spawn rate 15:03:28 Ladder step 2/3: trace_probability=0.1 15:06:27 Ladder step 3/3: trace_probability=1.0 rates that boomer applied: 0 The cause is in the shape worker of locust, which sends a spawn message only when the number of users or the spawn rate changes. SubscribeSpawn reads /boomer-config at a spawn message, thus a step that holds the number of users gives this worker no message and no new value. The run continues and gives a table of three steps at one rate, which a reader cannot tell from a correct one. Add StartPoll, which reads the endpoint on an interval also, and --config-poll-interval to set it. The default is 10s. Only a change goes to the log, because a poll of each few seconds for the length of a soak would hide the run. A failed poll is not fatal, unlike a failed fetch at a spawn message: the worker holds the last good value, and one failed poll of a long run is not a reason to lose the run. The end of the run stops a fetch that is in progress, and that is not a failed fetch. Follow-up to agent-substrate#1071.
Jeff Luo (JeffLuoo)
force-pushed
the
obs-ladder-automation
branch
from
August 21, 2026 20:04
69fd65d to
881498b
Compare
Max Smythe (maxsmythe)
merged commit Aug 22, 2026
4bc650a
into
agent-substrate:main
10 of 11 checks passed
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
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.
Follow-up to #749, for comment of adding the ladder design in benchmarking for observability.