-
Notifications
You must be signed in to change notification settings - Fork 273
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
Benchmarking changes of the wire protocol #110
Comments
When establishing and creating the OTel Profiling protocol, @petethepig invested noticeable time and effort in benchmarks - see petethepig/opentelemetry-collector#1. He also documented changes and potential options with https://docs.google.com/spreadsheets/d/1Q-6MlegV8xLYdz5WD5iPxQU2tsfodX1-CDV1WeGzyQ0/edit?gid=1732807979#gid=1732807979. It might be worth considering building on this existing work. |
A simpler approach that @christos68k and I have been testing with previously is to build two profiling agents with two protocols that you want to compare, then running them at the same time on the same machine while applying some heavy workload and recording the sum of all message sizes. Sampling won't interrupt exactly the same traces in both agents, but if you run it for an hour or so it should statistically give you a pretty good estimate. From previous experience of looking at differential flamegraphs of two agents running on the same machine, I'd expect the error to be in the realm of 0.5 - 1% with that approach. It's arguably more difficult to reproduce for other reviewers than with @petethepig's approach or the one that you are describing in this issue here. |
#120 is a PoC for the ideas outlines in the issue description. |
When making changes of the wire protocol, we should take into account the effect on CPU usage, memory usage and network bandwidth. For this we need some tooling for doing (nearly) reproducible benchmarks.
Roughly, my thoughts are
The recorded data can be replayed multiple times, e.g. with and without a protocol implementation change, to allow comparisons of the change's effects.
The text was updated successfully, but these errors were encountered: