From 960e6ab4255a51b2914ceb4a3daddd5d244f3dc1 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Wed, 11 Oct 2023 03:34:00 -0400 Subject: [PATCH] migrate opencensus bridge to errors.Join (#4599) --- bridge/opencensus/internal/ocmetric/metric.go | 58 ++++++++----------- .../internal/ocmetric/metric_test.go | 12 ++-- 2 files changed, 29 insertions(+), 41 deletions(-) diff --git a/bridge/opencensus/internal/ocmetric/metric.go b/bridge/opencensus/internal/ocmetric/metric.go index 3869d318cfb..d656e5d2871 100644 --- a/bridge/opencensus/internal/ocmetric/metric.go +++ b/bridge/opencensus/internal/ocmetric/metric.go @@ -25,11 +25,8 @@ import ( ) var ( - errConversion = errors.New("converting from OpenCensus to OpenTelemetry") errAggregationType = errors.New("unsupported OpenCensus aggregation type") errMismatchedValueTypes = errors.New("wrong value type for data point") - errNumberDataPoint = errors.New("converting a number data point") - errHistogramDataPoint = errors.New("converting a histogram data point") errNegativeDistributionCount = errors.New("distribution count is negative") errNegativeBucketCount = errors.New("distribution bucket count is negative") errMismatchedAttributeKeyValues = errors.New("mismatched number of attribute keys and values") @@ -38,14 +35,14 @@ var ( // ConvertMetrics converts metric data from OpenCensus to OpenTelemetry. func ConvertMetrics(ocmetrics []*ocmetricdata.Metric) ([]metricdata.Metrics, error) { otelMetrics := make([]metricdata.Metrics, 0, len(ocmetrics)) - var errInfo []string + var err error for _, ocm := range ocmetrics { if ocm == nil { continue } - agg, err := convertAggregation(ocm) - if err != nil { - errInfo = append(errInfo, err.Error()) + agg, aggregationErr := convertAggregation(ocm) + if aggregationErr != nil { + err = errors.Join(err, fmt.Errorf("error converting metric %v: %w", ocm.Descriptor.Name, aggregationErr)) continue } otelMetrics = append(otelMetrics, metricdata.Metrics{ @@ -55,11 +52,10 @@ func ConvertMetrics(ocmetrics []*ocmetricdata.Metric) ([]metricdata.Metrics, err Data: agg, }) } - var aggregatedError error - if len(errInfo) > 0 { - aggregatedError = fmt.Errorf("%w: %q", errConversion, errInfo) + if err != nil { + return otelMetrics, fmt.Errorf("error converting from OpenCensus to OpenTelemetry: %w", err) } - return otelMetrics, aggregatedError + return otelMetrics, nil } // convertAggregation produces an aggregation based on the OpenCensus Metric. @@ -97,17 +93,17 @@ func convertSum[N int64 | float64](labelKeys []ocmetricdata.LabelKey, ts []*ocme // convertNumberDataPoints converts OpenCensus TimeSeries to OpenTelemetry DataPoints. func convertNumberDataPoints[N int64 | float64](labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.TimeSeries) ([]metricdata.DataPoint[N], error) { var points []metricdata.DataPoint[N] - var errInfo []string + var err error for _, t := range ts { - attrs, err := convertAttrs(labelKeys, t.LabelValues) - if err != nil { - errInfo = append(errInfo, err.Error()) + attrs, attrsErr := convertAttrs(labelKeys, t.LabelValues) + if attrsErr != nil { + err = errors.Join(err, attrsErr) continue } for _, p := range t.Points { v, ok := p.Value.(N) if !ok { - errInfo = append(errInfo, fmt.Sprintf("%v: %q", errMismatchedValueTypes, p.Value)) + err = errors.Join(err, fmt.Errorf("%w: %q", errMismatchedValueTypes, p.Value)) continue } points = append(points, metricdata.DataPoint[N]{ @@ -118,37 +114,33 @@ func convertNumberDataPoints[N int64 | float64](labelKeys []ocmetricdata.LabelKe }) } } - var aggregatedError error - if len(errInfo) > 0 { - aggregatedError = fmt.Errorf("%w: %v", errNumberDataPoint, errInfo) - } - return points, aggregatedError + return points, err } // convertHistogram converts OpenCensus Distribution timeseries to an // OpenTelemetry Histogram aggregation. func convertHistogram(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.TimeSeries) (metricdata.Histogram[float64], error) { points := make([]metricdata.HistogramDataPoint[float64], 0, len(ts)) - var errInfo []string + var err error for _, t := range ts { - attrs, err := convertAttrs(labelKeys, t.LabelValues) - if err != nil { - errInfo = append(errInfo, err.Error()) + attrs, attrsErr := convertAttrs(labelKeys, t.LabelValues) + if attrsErr != nil { + err = errors.Join(err, attrsErr) continue } for _, p := range t.Points { dist, ok := p.Value.(*ocmetricdata.Distribution) if !ok { - errInfo = append(errInfo, fmt.Sprintf("%v: %d", errMismatchedValueTypes, p.Value)) + err = errors.Join(err, fmt.Errorf("%w: %d", errMismatchedValueTypes, p.Value)) continue } - bucketCounts, err := convertBucketCounts(dist.Buckets) - if err != nil { - errInfo = append(errInfo, err.Error()) + bucketCounts, bucketErr := convertBucketCounts(dist.Buckets) + if bucketErr != nil { + err = errors.Join(err, bucketErr) continue } if dist.Count < 0 { - errInfo = append(errInfo, fmt.Sprintf("%v: %d", errNegativeDistributionCount, dist.Count)) + err = errors.Join(err, fmt.Errorf("%w: %d", errNegativeDistributionCount, dist.Count)) continue } // TODO: handle exemplars @@ -163,11 +155,7 @@ func convertHistogram(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.Time }) } } - var aggregatedError error - if len(errInfo) > 0 { - aggregatedError = fmt.Errorf("%w: %v", errHistogramDataPoint, errInfo) - } - return metricdata.Histogram[float64]{DataPoints: points, Temporality: metricdata.CumulativeTemporality}, aggregatedError + return metricdata.Histogram[float64]{DataPoints: points, Temporality: metricdata.CumulativeTemporality}, err } // convertBucketCounts converts from OpenCensus bucket counts to slice of uint64. diff --git a/bridge/opencensus/internal/ocmetric/metric_test.go b/bridge/opencensus/internal/ocmetric/metric_test.go index 0cbb217f293..76acbc80089 100644 --- a/bridge/opencensus/internal/ocmetric/metric_test.go +++ b/bridge/opencensus/internal/ocmetric/metric_test.go @@ -461,7 +461,7 @@ func TestConvertMetrics(t *testing.T) { }, }, }, - expectedErr: errConversion, + expectedErr: errNegativeDistributionCount, }, { desc: "histogram with negative bucket count", input: []*ocmetricdata.Metric{ @@ -488,7 +488,7 @@ func TestConvertMetrics(t *testing.T) { }, }, }, - expectedErr: errConversion, + expectedErr: errNegativeBucketCount, }, { desc: "histogram with non-histogram datapoint type", input: []*ocmetricdata.Metric{ @@ -509,7 +509,7 @@ func TestConvertMetrics(t *testing.T) { }, }, }, - expectedErr: errConversion, + expectedErr: errMismatchedValueTypes, }, { desc: "sum with non-sum datapoint type", input: []*ocmetricdata.Metric{ @@ -530,7 +530,7 @@ func TestConvertMetrics(t *testing.T) { }, }, }, - expectedErr: errConversion, + expectedErr: errMismatchedValueTypes, }, { desc: "gauge with non-gauge datapoint type", input: []*ocmetricdata.Metric{ @@ -551,7 +551,7 @@ func TestConvertMetrics(t *testing.T) { }, }, }, - expectedErr: errConversion, + expectedErr: errMismatchedValueTypes, }, { desc: "unsupported Gauge Distribution type", input: []*ocmetricdata.Metric{ @@ -564,7 +564,7 @@ func TestConvertMetrics(t *testing.T) { }, }, }, - expectedErr: errConversion, + expectedErr: errAggregationType, }, } { t.Run(tc.desc, func(t *testing.T) {