From 55b9ef35b79c2463d13c0f31429620931c7a5c40 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Mon, 9 Oct 2023 20:26:35 +0000 Subject: [PATCH 1/3] support summaries in the OpenCensus bridge --- CHANGELOG.md | 1 + bridge/opencensus/doc.go | 1 - bridge/opencensus/internal/ocmetric/metric.go | 65 +++++- .../internal/ocmetric/metric_test.go | 208 +++++++++++++++++- 4 files changed, 269 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3826a72584c..fa295bb3097 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `Version` function in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`. (#4660) - Add `Version` function in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#4660) - Add Summary, SummaryDataPoint, and QuantileValue to `go.opentelemetry.io/sdk/metric/metricdata`. (#4622) +- Add support for Summary metrics in `go.opentelemetry.io/otel/bridge/opencensus`. (#4668) ### Deprecated diff --git a/bridge/opencensus/doc.go b/bridge/opencensus/doc.go index ed2a4cfd935..7a4ed70fa17 100644 --- a/bridge/opencensus/doc.go +++ b/bridge/opencensus/doc.go @@ -56,7 +56,6 @@ // implemented, and An error will be sent to the OpenTelemetry ErrorHandler. // // There are known limitations to the metric bridge: -// - Summary-typed metrics are dropped // - GaugeDistribution-typed metrics are dropped // - Histogram's SumOfSquaredDeviation field is dropped // - Exemplars on Histograms are dropped diff --git a/bridge/opencensus/internal/ocmetric/metric.go b/bridge/opencensus/internal/ocmetric/metric.go index d656e5d2871..1d8f4d4c8f0 100644 --- a/bridge/opencensus/internal/ocmetric/metric.go +++ b/bridge/opencensus/internal/ocmetric/metric.go @@ -17,6 +17,7 @@ package internal // import "go.opentelemetry.io/otel/bridge/opencensus/internal/ import ( "errors" "fmt" + "sort" ocmetricdata "go.opencensus.io/metric/metricdata" @@ -27,7 +28,7 @@ import ( var ( errAggregationType = errors.New("unsupported OpenCensus aggregation type") errMismatchedValueTypes = errors.New("wrong value type for data point") - errNegativeDistributionCount = errors.New("distribution count is negative") + errNegativeCount = errors.New("distribution or summary count is negative") errNegativeBucketCount = errors.New("distribution bucket count is negative") errMismatchedAttributeKeyValues = errors.New("mismatched number of attribute keys and values") ) @@ -72,7 +73,8 @@ func convertAggregation(metric *ocmetricdata.Metric) (metricdata.Aggregation, er return convertSum[float64](labelKeys, metric.TimeSeries) case ocmetricdata.TypeCumulativeDistribution: return convertHistogram(labelKeys, metric.TimeSeries) - // TODO: Support summaries, once it is in the OTel data types. + case ocmetricdata.TypeSummary: + return convertSummary(labelKeys, metric.TimeSeries) } return nil, fmt.Errorf("%w: %q", errAggregationType, metric.Descriptor.Type) } @@ -140,7 +142,7 @@ func convertHistogram(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.Time continue } if dist.Count < 0 { - err = errors.Join(err, fmt.Errorf("%w: %d", errNegativeDistributionCount, dist.Count)) + err = errors.Join(err, fmt.Errorf("%w: %d", errNegativeCount, dist.Count)) continue } // TODO: handle exemplars @@ -170,6 +172,63 @@ func convertBucketCounts(buckets []ocmetricdata.Bucket) ([]uint64, error) { return bucketCounts, nil } +// convertSummary converts OpenCensus Summary timeseries to an +// OpenTelemetry Summary. +func convertSummary(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.TimeSeries) (metricdata.Summary, error) { + points := make([]metricdata.SummaryDataPoint, 0, len(ts)) + var err error + for _, t := range ts { + attrs, attrErr := convertAttrs(labelKeys, t.LabelValues) + if attrErr != nil { + err = errors.Join(err, attrErr) + continue + } + for _, p := range t.Points { + summary, ok := p.Value.(*ocmetricdata.Summary) + if !ok { + err = errors.Join(err, fmt.Errorf("%w: %d", errMismatchedValueTypes, p.Value)) + continue + } + if summary.Count < 0 { + err = errors.Join(err, fmt.Errorf("%w: %d", errNegativeCount, summary.Count)) + continue + } + point := metricdata.SummaryDataPoint{ + Attributes: attrs, + StartTime: t.StartTime, + Time: p.Time, + Count: uint64(summary.Count), + QuantileValues: convertQuantiles(summary.Snapshot), + Sum: summary.Sum, + } + points = append(points, point) + } + } + return metricdata.Summary{DataPoints: points}, err +} + +// convertQuantiles converts an OpenCensus summary snapshot to +// OpenTelemetry quantiles. +func convertQuantiles(snapshot ocmetricdata.Snapshot) []metricdata.QuantileValue { + quantileValues := make([]metricdata.QuantileValue, 0, len(snapshot.Percentiles)) + for quantile, value := range snapshot.Percentiles { + quantileValues = append(quantileValues, metricdata.QuantileValue{ + Quantile: quantile, + Value: value, + }) + } + sort.Sort(byQuantile(quantileValues)) + return quantileValues +} + +// byQuantile implements sort.Interface for []metricdata.QuantileValue +// based on the Quantile field. +type byQuantile []metricdata.QuantileValue + +func (a byQuantile) Len() int { return len(a) } +func (a byQuantile) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a byQuantile) Less(i, j int) bool { return a[i].Quantile < a[j].Quantile } + // convertAttrs converts from OpenCensus attribute keys and values to an // OpenTelemetry attribute Set. func convertAttrs(keys []ocmetricdata.LabelKey, values []ocmetricdata.LabelValue) (attribute.Set, error) { diff --git a/bridge/opencensus/internal/ocmetric/metric_test.go b/bridge/opencensus/internal/ocmetric/metric_test.go index a3258c6b1e7..6a904c50619 100644 --- a/bridge/opencensus/internal/ocmetric/metric_test.go +++ b/bridge/opencensus/internal/ocmetric/metric_test.go @@ -41,7 +41,7 @@ func TestConvertMetrics(t *testing.T) { expected: []metricdata.Metrics{}, }, { - desc: "normal Histogram, gauges, and sums", + desc: "normal Histogram, summary, gauges, and sums", input: []*ocmetricdata.Metric{ { Descriptor: ocmetricdata.Descriptor{ @@ -206,6 +206,54 @@ func TestConvertMetrics(t *testing.T) { }, }, }, + }, { + Descriptor: ocmetricdata.Descriptor{ + Name: "foo.com/summary-a", + Description: "a testing summary", + Unit: ocmetricdata.UnitMilliseconds, + Type: ocmetricdata.TypeSummary, + LabelKeys: []ocmetricdata.LabelKey{ + {Key: "g"}, + {Key: "h"}, + }, + }, + TimeSeries: []*ocmetricdata.TimeSeries{ + { + LabelValues: []ocmetricdata.LabelValue{ + { + Value: "ding", + Present: true, + }, { + Value: "dong", + Present: true, + }, + }, + Points: []ocmetricdata.Point{ + ocmetricdata.NewSummaryPoint(endTime1, &ocmetricdata.Summary{ + Count: 10, + Sum: 13.2, + HasCountAndSum: true, + Snapshot: ocmetricdata.Snapshot{ + Percentiles: map[float64]float64{ + 0.5: 1.0, + 0.0: 0.1, + 1.0: 10.4, + }, + }, + }), + ocmetricdata.NewSummaryPoint(endTime2, &ocmetricdata.Summary{ + Count: 12, + Snapshot: ocmetricdata.Snapshot{ + Percentiles: map[float64]float64{ + 0.0: 0.2, + 0.5: 1.1, + 1.0: 10.5, + }, + }, + }), + }, + }, + }, }, }, expected: []metricdata.Metrics{ @@ -367,6 +415,64 @@ func TestConvertMetrics(t *testing.T) { }, }, }, + }, { + Name: "foo.com/summary-a", + Description: "a testing summary", + Unit: "ms", + Data: metricdata.Summary{ + DataPoints: []metricdata.SummaryDataPoint{ + { + Attributes: attribute.NewSet(attribute.KeyValue{ + Key: attribute.Key("g"), + Value: attribute.StringValue("ding"), + }, attribute.KeyValue{ + Key: attribute.Key("h"), + Value: attribute.StringValue("dong"), + }), + Time: endTime1, + Count: 10, + Sum: 13.2, + QuantileValues: []metricdata.QuantileValue{ + { + Quantile: 0.0, + Value: 0.1, + }, + { + Quantile: 0.5, + Value: 1.0, + }, + { + Quantile: 1.0, + Value: 10.4, + }, + }, + }, { + Attributes: attribute.NewSet(attribute.KeyValue{ + Key: attribute.Key("g"), + Value: attribute.StringValue("ding"), + }, attribute.KeyValue{ + Key: attribute.Key("h"), + Value: attribute.StringValue("dong"), + }), + Time: endTime2, + Count: 12, + QuantileValues: []metricdata.QuantileValue{ + { + Quantile: 0.0, + Value: 0.2, + }, + { + Quantile: 0.5, + Value: 1.1, + }, + { + Quantile: 1.0, + Value: 10.5, + }, + }, + }, + }, + }, }, }, }, @@ -464,7 +570,7 @@ func TestConvertMetrics(t *testing.T) { }, }, }, - expectedErr: errNegativeDistributionCount, + expectedErr: errNegativeCount, }, { desc: "histogram with negative bucket count", @@ -516,6 +622,82 @@ func TestConvertMetrics(t *testing.T) { }, expectedErr: errMismatchedValueTypes, }, + { + desc: "summary with mismatched attributes", + input: []*ocmetricdata.Metric{ + { + Descriptor: ocmetricdata.Descriptor{ + Name: "foo.com/summary-mismatched", + Description: "a mismatched summary", + Unit: ocmetricdata.UnitMilliseconds, + Type: ocmetricdata.TypeSummary, + LabelKeys: []ocmetricdata.LabelKey{ + {Key: "g"}, + }, + }, + TimeSeries: []*ocmetricdata.TimeSeries{ + { + LabelValues: []ocmetricdata.LabelValue{ + { + Value: "ding", + Present: true, + }, { + Value: "dong", + Present: true, + }, + }, + Points: []ocmetricdata.Point{ + ocmetricdata.NewSummaryPoint(endTime1, &ocmetricdata.Summary{ + Count: 10, + Sum: 13.2, + HasCountAndSum: true, + Snapshot: ocmetricdata.Snapshot{ + Percentiles: map[float64]float64{ + 0.0: 0.1, + 0.5: 1.0, + 1.0: 10.4, + }, + }, + }), + }, + }, + }, + }, + }, + expectedErr: errMismatchedAttributeKeyValues, + }, + { + desc: "summary with negative count", + input: []*ocmetricdata.Metric{ + { + Descriptor: ocmetricdata.Descriptor{ + Name: "foo.com/summary-negative", + Description: "a negative count summary", + Unit: ocmetricdata.UnitMilliseconds, + Type: ocmetricdata.TypeSummary, + }, + TimeSeries: []*ocmetricdata.TimeSeries{ + { + Points: []ocmetricdata.Point{ + ocmetricdata.NewSummaryPoint(endTime1, &ocmetricdata.Summary{ + Count: -10, + Sum: 13.2, + HasCountAndSum: true, + Snapshot: ocmetricdata.Snapshot{ + Percentiles: map[float64]float64{ + 0.0: 0.1, + 0.5: 1.0, + 1.0: 10.4, + }, + }, + }), + }, + }, + }, + }, + }, + expectedErr: errNegativeCount, + }, { desc: "sum with non-sum datapoint type", input: []*ocmetricdata.Metric{ @@ -560,6 +742,28 @@ func TestConvertMetrics(t *testing.T) { }, expectedErr: errMismatchedValueTypes, }, + { + desc: "summary with non-summary datapoint type", + input: []*ocmetricdata.Metric{ + { + Descriptor: ocmetricdata.Descriptor{ + Name: "foo.com/bad-point", + Description: "a bad type", + Unit: ocmetricdata.UnitDimensionless, + Type: ocmetricdata.TypeSummary, + }, + TimeSeries: []*ocmetricdata.TimeSeries{ + { + Points: []ocmetricdata.Point{ + ocmetricdata.NewDistributionPoint(endTime1, &ocmetricdata.Distribution{}), + }, + StartTime: startTime, + }, + }, + }, + }, + expectedErr: errMismatchedValueTypes, + }, { desc: "unsupported Gauge Distribution type", input: []*ocmetricdata.Metric{ From e68f3530dad0bc5b7d6f5407ebadc5d8b6f4262c Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Wed, 25 Oct 2023 16:47:01 +0000 Subject: [PATCH 2/3] fix summaryDataPoint comparrison --- sdk/metric/metricdata/metricdatatest/comparisons.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metric/metricdata/metricdatatest/comparisons.go b/sdk/metric/metricdata/metricdatatest/comparisons.go index c6407aca8f8..a25c930676f 100644 --- a/sdk/metric/metricdata/metricdatatest/comparisons.go +++ b/sdk/metric/metricdata/metricdatatest/comparisons.go @@ -472,7 +472,7 @@ func equalSummaryDataPoint(a, b metricdata.SummaryDataPoint, cfg config) (reason } r := compareDiff(diffSlices( a.QuantileValues, - a.QuantileValues, + b.QuantileValues, func(a, b metricdata.QuantileValue) bool { r := equalQuantileValue(a, b, cfg) return len(r) == 0 From 3ee8de1ef542a21d72a2ffcb8ed75f20d7368b33 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Wed, 25 Oct 2023 16:47:14 +0000 Subject: [PATCH 3/3] divide quantiles by 100 --- bridge/opencensus/internal/ocmetric/metric.go | 4 +++- bridge/opencensus/internal/ocmetric/metric_test.go | 14 +++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/bridge/opencensus/internal/ocmetric/metric.go b/bridge/opencensus/internal/ocmetric/metric.go index 1d8f4d4c8f0..8ac70457e47 100644 --- a/bridge/opencensus/internal/ocmetric/metric.go +++ b/bridge/opencensus/internal/ocmetric/metric.go @@ -213,7 +213,9 @@ func convertQuantiles(snapshot ocmetricdata.Snapshot) []metricdata.QuantileValue quantileValues := make([]metricdata.QuantileValue, 0, len(snapshot.Percentiles)) for quantile, value := range snapshot.Percentiles { quantileValues = append(quantileValues, metricdata.QuantileValue{ - Quantile: quantile, + // OpenCensus quantiles are range (0-100.0], but OpenTelemetry + // quantiles are range [0.0, 1.0]. + Quantile: quantile / 100.0, Value: value, }) } diff --git a/bridge/opencensus/internal/ocmetric/metric_test.go b/bridge/opencensus/internal/ocmetric/metric_test.go index 6a904c50619..7b7e4a251f0 100644 --- a/bridge/opencensus/internal/ocmetric/metric_test.go +++ b/bridge/opencensus/internal/ocmetric/metric_test.go @@ -235,9 +235,9 @@ func TestConvertMetrics(t *testing.T) { HasCountAndSum: true, Snapshot: ocmetricdata.Snapshot{ Percentiles: map[float64]float64{ - 0.5: 1.0, - 0.0: 0.1, - 1.0: 10.4, + 50.0: 1.0, + 0.0: 0.1, + 100.0: 10.4, }, }, }), @@ -245,9 +245,9 @@ func TestConvertMetrics(t *testing.T) { Count: 12, Snapshot: ocmetricdata.Snapshot{ Percentiles: map[float64]float64{ - 0.0: 0.2, - 0.5: 1.1, - 1.0: 10.5, + 0.0: 0.2, + 50.0: 1.1, + 100.0: 10.5, }, }, }), @@ -782,7 +782,7 @@ func TestConvertMetrics(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { output, err := ConvertMetrics(tc.input) if !errors.Is(err, tc.expectedErr) { - t.Errorf("convertAggregation(%+v) = err(%v), want err(%v)", tc.input, err, tc.expectedErr) + t.Errorf("ConvertMetrics(%+v) = err(%v), want err(%v)", tc.input, err, tc.expectedErr) } metricdatatest.AssertEqual[metricdata.ScopeMetrics](t, metricdata.ScopeMetrics{Metrics: tc.expected},