Tightening rate verification for RateTests #131114
Conversation
|
r: @dnhatn |
elasticsearchmachine
added
the
needs:triage
pabloem
added
:StorageEngine/TSDB
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
| case DOUBLE, COUNTER_DOUBLE -> dataRows.stream().mapToDouble(v -> (Double) v).min().orElse(0.0); | ||
| default -> throw new IllegalStateException("Unexpected value: " + fieldTypedData.type()); | ||
| }; | ||
| // If the minrate is greater than 0, we need to adjust the maxrate accordingly |
There was a problem hiding this comment.
Interesting.. I would've thought that counters are INTEGER or LONG, and only COUNTER_INTEGER and COUNTER_LONG can be input to rate. Also, I haven't thought about negative values - though OTLP has the notion of up/down counters:
https://opentelemetry.io/docs/specs/otel/metrics/supplementary-guidelines/#instrument-selection
Maybe we should restrict the current rate implementation to non-negative counter values and check for that? @dnhatn for thoughts.
There was a problem hiding this comment.
Maybe we should restrict the current rate implementation to non-negative counter values and check for that?
I think the current rate implementation does not work correctly with up/down counters.
There was a problem hiding this comment.
Hm there's no check for negative values, though.. Shall we add a check and error out, if a negative counter value is detected?
There was a problem hiding this comment.
hi team! how would these address changes to my PR? what should we address here?
There was a problem hiding this comment.
I don't think we need to address the issue here.
4 participants
No description provided.