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.
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
Add OpenTelemetry performance benchmark spec #748
Add OpenTelemetry performance benchmark spec #748
Changes from 16 commits
4dc4c41
08bbf04
bc7b3a3
dd1a61f
6c709a7
c4aab13
051bfda
803c65b
0235ae8
87a730a
1dbebd5
3c8607c
3425356
17d1aa0
4908384
8a2d548
a9fd74f
cb2db52
eefc9d4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to read this whole section before I understood that it describes the measurement that the experiment should take and report.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added "Measurement" to related section header to highlight the subject, let me know if further clarification needed, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: in languages with a JIT (like Java), you'll definitely want to have the benchmark system warm up a bit before actually making measurements. This is notoriously difficult to get right. Also, with essentially identical input data, it's very possible that the Java JIT will optimize away significant portions of the code that might not be optimized during realistic workloads.
I would definitely add a description of what information you're hoping to extract by making this measurement like this. It will also probably vary greatly depending on what sort of hardware you're running the benchmark on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note: with zero-duration spans, the BatchSpanProcessor is likely to drop a large number of them, due to extremely high volume (at least, this is how it works in the Java implementation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more note: The BatchSpanProcessor is highly configurable, and the throughput of this setup will vary a lot depending on how it's configured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default configuration of BSP and OTLP in the implementating SDK is suggested to run SDK. This spec is not targeting to shows perf number for different configurations and helping users to choose the best.
You are right the spans are very close to "zero duration". As the spec is meant to measure pure SDK performance with out any user logic, this would be expected.
Threads number is mentioned here, but the spec requires measuring single core and all cores performance, the the SDK and benchmark author need to define the thread number which does best fit for each scenario.
The spec asks to implement benchmark for each SDK implementation, so when a user wants to adopt one SDK, like opentelemetry-java, the user could run the benchmark program on the target platform to get an idea of the baseline performance cost of SDK with given throughput. Like if the user specifies 10,000 events/second and gets 20% CPU report over all cores from this benchmark, the user could triage whether this is affordable overhead of SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkwatson any more questions or further clarification needed? Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was asking to have the motivation and purpose added to the document itself, not just in a PR comment that will be lost forever. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, add the summary of above comment to a new paragraph to the beginning of this doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkwatson let me know if more motivation and purpose background need to be added to the doc.