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.

Sign up for GitHub

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

Migrate opencensus bridge to errors.Join #4599

Merged
merged 3 commits into from
Oct 11, 2023
Merged

Migrate opencensus bridge to errors.Join #4599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea0fc10
migrate opencensus bridge to errors.Join
dashpole Oct 9, 2023
02bfbf7
Merge branch 'main' into oc_errors_join
hanyuancheung Oct 11, 2023
3901413
Merge branch 'main' into oc_errors_join
pellared Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 23 additions & 35 deletions bridge/opencensus/internal/ocmetric/metric.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,8 @@
)

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")
Expand All @@ -38,14 +35,14 @@
// 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{
Expand All @@ -55,11 +52,10 @@
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.
Expand Down Expand Up @@ -97,17 +93,17 @@
// 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)

Check warning on line 100 in bridge/opencensus/internal/ocmetric/metric.go

View check run for this annotation

Codecov / codecov/patch

bridge/opencensus/internal/ocmetric/metric.go#L100 

Added line #L100 was not covered by tests
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]{
Expand All @@ -118,37 +114,33 @@
})
}
}
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)

Check warning on line 128 in bridge/opencensus/internal/ocmetric/metric.go

View check run for this annotation

Codecov / codecov/patch

bridge/opencensus/internal/ocmetric/metric.go#L128 

Added line #L128 was not covered by tests
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
Expand All @@ -163,11 +155,7 @@
})
}
}
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.
Expand Down
12 changes: 6 additions & 6 deletions bridge/opencensus/internal/ocmetric/metric_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ func TestConvertMetrics(t *testing.T) {
},
},
},
expectedErr: errConversion,
expectedErr: errNegativeDistributionCount,
}, {
desc: "histogram with negative bucket count",
input: []*ocmetricdata.Metric{
Expand All @@ -488,7 +488,7 @@ func TestConvertMetrics(t *testing.T) {
},
},
},
expectedErr: errConversion,
expectedErr: errNegativeBucketCount,
}, {
desc: "histogram with non-histogram datapoint type",
input: []*ocmetricdata.Metric{
Expand All @@ -509,7 +509,7 @@ func TestConvertMetrics(t *testing.T) {
},
},
},
expectedErr: errConversion,
expectedErr: errMismatchedValueTypes,
}, {
desc: "sum with non-sum datapoint type",
input: []*ocmetricdata.Metric{
Expand All @@ -530,7 +530,7 @@ func TestConvertMetrics(t *testing.T) {
},
},
},
expectedErr: errConversion,
expectedErr: errMismatchedValueTypes,
}, {
desc: "gauge with non-gauge datapoint type",
input: []*ocmetricdata.Metric{
Expand All @@ -551,7 +551,7 @@ func TestConvertMetrics(t *testing.T) {
},
},
},
expectedErr: errConversion,
expectedErr: errMismatchedValueTypes,
}, {
desc: "unsupported Gauge Distribution type",
input: []*ocmetricdata.Metric{
Expand All @@ -564,7 +564,7 @@ func TestConvertMetrics(t *testing.T) {
},
},
},
expectedErr: errConversion,
expectedErr: errAggregationType,
},
} {
t.Run(tc.desc, func(t *testing.T) {
Expand Down
Loading