[test-only] simulations use tritium metric registry #358
Conversation
This file contains 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
iamdanfox
commented
Feb 18, 2020
iamdanfox
commented
Feb 18, 2020
iamdanfox
commented
Feb 18, 2020
| one_endpoint_dies_on_each_server[ROUND_ROBIN].txt: success=67.6% client_mean=PT0.6S server_cpu=PT5M6S client_received=510/510 server_resps=510 codes={200=345, 500=165} | ||
| simplest_possible_case[CONCURRENCY_LIMITER].txt: success=100.0% client_mean=PT0.7998S server_cpu=PT13M19.8S client_received=1000/1000 server_resps=1000 codes={200=1000} | ||
| simplest_possible_case[ROUND_ROBIN].txt: success=100.0% client_mean=PT0.7998S server_cpu=PT13M19.8S client_received=1000/1000 server_resps=1000 codes={200=1000} | ||
| slow_503s_then_revert[CONCURRENCY_LIMITER].txt: success=100.0% client_mean=PT0.129949S server_cpu=PT6M29.847333275S client_received=3000/3000 server_resps=3130 codes={200=3000} |
There was a problem hiding this comment.
this is an example where our RetryingChannel kicked in and resulted in servers producing responding 3130 responses, but clients just perceived 3000 green responses (as the retries were eventually successful).
iamdanfox
changed the title
carterkozak
reviewed
Feb 18, 2020
simulation/src/main/java/com/palantir/dialogue/core/SimulationMetricsReporter.java
Outdated
Show resolved
Hide resolved
carterkozak
reviewed
Feb 18, 2020
| () -> { | ||
| long micros = | ||
| TimeUnit.MICROSECONDS.convert(simulation.clock().read() - start, TimeUnit.NANOSECONDS); | ||
| histogram.update(micros); |
There was a problem hiding this comment.
can we use a timer instead?
There was a problem hiding this comment.
Have updated to use the vanilla InstrumentedChannel!
iamdanfox
force-pushed
the
dfox/response-bodies
branch
from
7663dc6 to
a63dd26
Compare
iamdanfox
force-pushed
the
dfox/response-bodies
branch
from
a63dd26 to
6008dd7
Compare
carterkozak
changed the title
iamdanfox
force-pushed
the
dfox/response-bodies
branch
from
f847202 to
c19b20d
Compare
|
Ok I think this is ready to go :) |
carterkozak
approved these changes
Feb 18, 2020
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.
Before this PR
My simulation was keeping track of a map of metrics like an animal.
After this PR
==COMMIT_MSG==
Simulation metrics are now tracked in a tritium TaggedMetricRegistry implementation, which is hooked up to the simulation clock.
==COMMIT_MSG==
This PR also uses this new metricregistry to better record the global number of responses returned and also verify that all of them were closed. Graphs will also display tags nicely, where previously I was doing string concatenation!
As a bonus, gradle compilation is now much snappier.
Possible downsides?