multiple exemplars per sample #34
Draft
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
Signed-off-by: György Krajcsovits <[email protected]>
aknuds1
reviewed
Apr 3, 2024
Comment on lines
+34
to
+35
| In the current solution we have a circular buffer for all exemplars. The exemplars are organized into two lists. One list links all exemplars from oldest received to newest so that we can make space when maximum number of exemplars is hit on insert. The other link | ||
| is per time series, linking exemplars from oldest to newest. Also there is a pointer to the latest exemplar per time series to be able to check for duplicates, out of order. |
There was a problem hiding this comment.
Do you mean "the other list" rather than "the other link"? Would think this refers to the second list.
Suggested change
| In the current solution we have a circular buffer for all exemplars. The exemplars are organized into two lists. One list links all exemplars from oldest received to newest so that we can make space when maximum number of exemplars is hit on insert. The other link | |
| is per time series, linking exemplars from oldest to newest. Also there is a pointer to the latest exemplar per time series to be able to check for duplicates, out of order. | |
| In the current solution we have a circular buffer for all exemplars. The exemplars are organized into two lists. One list links all exemplars from oldest received to newest so that we can make space when maximum number of exemplars is hit on insert. The other list | |
| is per time series, listing exemplars from oldest to newest. Also there is a pointer to the latest exemplar per time series to be able to check for duplicates, out of order. |
aknuds1
reviewed
Apr 3, 2024
|
|
||
| * There is no interface to pass multiple exemplars to head.AppendExemplar, so multiple calls need to be made, increasing the call count and lock contention. | ||
| * If a native histogram has 10 exemplars per scrape/sample and all are received in the same remote write request, we currently try appending all in a loop. We consider them out of order if all are out of order, but if there's a duplicate of the last stored exemplar or some are newer than the last stored exemplar, then we just ignore the old exemplars. | ||
| * However the logic above fails if exemplars are received in multiple remote write requests, so the logic is a workaround at best. |
There was a problem hiding this comment.
[Nit]
Suggested change
| * However the logic above fails if exemplars are received in multiple remote write requests, so the logic is a workaround at best. | |
| * However, the logic above fails if exemplars are received in multiple remote write requests, so the logic is a workaround at best. |
aknuds1
reviewed
Apr 10, 2024
|
|
||
| Currently the remote write protocol (1.0 and 2.0) processes exemplars individually, which means that if a native histogram has multiple exemplars, they may arrive in separate remote write requests, making it hard to decide if the exemplar is out of order it belonged to a previous set of exemplars and should be considered a duplicate. | ||
|
|
||
| Currently there is no mapping of the "set of exemplars" per sample concept that is in Open Telemetry specification. |
There was a problem hiding this comment.
Nit for consistency :)
Suggested change
| Currently there is no mapping of the "set of exemplars" per sample concept that is in Open Telemetry specification. | |
| Currently there is no mapping of the "set of exemplars" per sample concept that is in OpenTelemetry specification. |
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.
No description provided.