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
Drop histogram metrics with no non-zero bucket values #533
Conversation
@@ -657,7 +657,7 @@ func (m *metricMapper) histogramToTimeSeries( | |||
point pmetric.HistogramDataPoint, | |||
projectID string, | |||
) []*monitoringpb.TimeSeries { | |||
if point.Flags().NoRecordedValue() || !point.HasSum() { | |||
if point.Flags().NoRecordedValue() || !point.HasSum() || point.ExplicitBounds().Len() == 0 { |
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.
this change affected the _expected
test fixture:
diff --git a/exporter/collector/integrationtest/testdata/fixtures/metrics/prometheus_empty_buckets_expected.json b/exporter/collector/integrationtest/testdata/fixtures/metrics/prometheus_empty_buckets_expected.json
index 6633318..96c3f83 100644
--- a/exporter/collector/integrationtest/testdata/fixtures/metrics/prometheus_empty_buckets_expected.json
+++ b/exporter/collector/integrationtest/testdata/fixtures/metrics/prometheus_empty_buckets_expected.json
@@ -3,44 +3,6 @@
{
"name": "projects/fakeprojectid",
"timeSeries": [
- {
- "metric": {
- "type": "workload.googleapis.com/kv_prober_write_latency",
- "labels": {
- "service_instance_id": "0.0.0.0:2112",
- "service_name": "otel-collector"
- }
- },
- "resource": {
- "type": "generic_task",
- "labels": {
- "job": "otel-collector",
- "location": "global",
- "namespace": "",
- "task_id": "0.0.0.0:2112"
- }
- },
- "metricKind": "CUMULATIVE",
- "valueType": "DISTRIBUTION",
- "points": [
- {
- "interval": {
- "endTime": "1970-01-01T00:00:00Z",
- "startTime": "1970-01-01T00:00:00Z"
- },
- "value": {
- "distributionValue": {
- "bucketOptions": {
- "explicitBuckets": {}
- },
- "bucketCounts": [
- "0"
- ]
- }
- }
- }
- ]
- },
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.
Thats interesting. How were integration tests passing before if there was an invalid histogram?
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.
Sorry, this wasn't a test that existed before. I added it, and did a before and after with the logic change to see that the (new) fixture was updated.
ecadec9
to
e31b082
Compare
e31b082
to
7b261a7
Compare
Fixes #525 by dropping metric points that have no bucket bounds, consistent with GMP.
Testdata was generated with the following sample metric:
served on a local endpoint, scraped with the collector's prometheus receiver, and written with the file exporter.