[receiver/elasticsearch]: add flush time metric on index level #14924
Conversation
aboguszewski-sumo
force-pushed
the
elastic-flush-time
branch
from
a39f4d0
to
ad8e5a3
Compare
| @@ -0,0 +1,16 @@ | |||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | |||
| change_type: enhancement | |||
There was a problem hiding this comment.
Since we were already collecting this but not emitting it, and because there is already a metric for this in the data model, I think this should be considered a bug fix.
There was a problem hiding this comment.
It was omitted on purpose by me - I reused the operation attribute when adding search metrics, so it's natural that all unused operations type are collected, but not emitted.
There was a problem hiding this comment.
I see, but this PR does not add a new metric. It adds a new data point to an existing metric. The result is that the existing metric will have a different "total" than before.
If we think adding this new data point gives us an accurate total, then this is a bug fix. Otherwise, if we think the "total" is still only part of the picture, then we could look to complete the total by adding additional data points.
There was a problem hiding this comment.
To sum up: do you think that we should add all missing data points at once?
For example, #14871 is a very similar pull request to this one. If we want to add missing data points at once, I will close both PRs and send another one, that adds all missing data points.
There was a problem hiding this comment.
If there are multiple missing data points for the same metric, I think it's appropriate to add them in one PR.
|
Closing this for now, this PR is going to be replaced by another one, see #14924 (comment) for more info. |
Description:
A metric with flush time is now emitted on index level. It was already present in
metadata.yaml, but not emitted.Link to tracking Issue: #14635
Testing:
Unit and integration.
Documentation:
mdatagen