Skip to content
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

[receiver/elasticsearch]: add missing datapoints for operation count and operation time metrics #16170

Conversation

aboguszewski-sumo
Copy link
Member

@aboguszewski-sumo aboguszewski-sumo commented Nov 8, 2022

Description:
All missing datapoints for metrics elasticsearch.index.operation.count and elasticsearch.index.operation.time have been added. This PR also replaces the following PRs: #14871 #14921 #14924
The rationale for this PR is explained here: #14924 (comment)

Link to tracking Issue: #14635

Testing:
Unit and integration tests

Documentation:
mdatagen

@djaglowski
Copy link
Member

I initially thought this should be a bug fix because it appeared that we had missed a data point or two on an existing metric. However, I do not think adding 30+ data points to a metric is in the spirit of "fixing a mistake".

I do think that adding these is fine, but we should declare it an enhancement and use a feature gate.

@aboguszewski-sumo
Copy link
Member Author

To be honest, all of these datapoints aren't necessary for me (only some arbitrarily chosen ones), but operations metrics are probably the most "basic" metrics there can be, so it might be a good idea to at least send these with a full set of datapoint.
Because this PR is not a heavy blocker, I'll wait with changes for #14875 to get merged, as some code related to feature gates will be common for both PRs.

@aboguszewski-sumo aboguszewski-sumo force-pushed the elastic-add-missing-operations branch from dbf7f96 to 72cdb63 Compare November 18, 2022 09:13
@aboguszewski-sumo
Copy link
Member Author

@djaglowski I added feature gates.

@djaglowski djaglowski merged commit 3c15679 into open-telemetry:main Nov 18, 2022
JaredTan95 pushed a commit to openinsight-proj/opentelemetry-collector-contrib that referenced this pull request Nov 21, 2022
…and operation time metrics (open-telemetry#16170)

* feat: add missing operation count datapoints with total aggregation

* feat: add missing operation count datapoints with primary shards aggregation

* feat: add missing operation time datapoints with total shard aggregation

* feat: add missing operation time datapoints with primary shards aggregations
shalper2 pushed a commit to shalper2/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2022
…and operation time metrics (open-telemetry#16170)

* feat: add missing operation count datapoints with total aggregation

* feat: add missing operation count datapoints with primary shards aggregation

* feat: add missing operation time datapoints with total shard aggregation

* feat: add missing operation time datapoints with primary shards aggregations
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants