From 3b5aee0c7a168f7ba57a87e864b4f65d794eef8b Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Fri, 15 Mar 2024 10:29:37 -0700 Subject: [PATCH] Revert "[chore] Refactor mdatagen unmarshaling to use less custom Unmarshalers" (#9773) Reverts open-telemetry/opentelemetry-collector#9760 Trying to make contrib happy. We will be back with a unit test covering contrib's usage of mdatagen. --- cmd/mdatagen/loader_test.go | 4 +-- cmd/mdatagen/metricdata.go | 52 ++++++++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/cmd/mdatagen/loader_test.go b/cmd/mdatagen/loader_test.go index b9040e23380..677288258d2 100644 --- a/cmd/mdatagen/loader_test.go +++ b/cmd/mdatagen/loader_test.go @@ -249,7 +249,7 @@ func TestLoadMetadata(t *testing.T) { }, { name: "testdata/unknown_value_type.yaml", - wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[system.cpu.time]': 1 error(s) decoding:\n\n* error decoding 'sum': invalid value_type: \"unknown\"", + wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[system.cpu.time]': 1 error(s) decoding:\n\n* error decoding 'sum': 1 error(s) decoding:\n\n* error decoding 'value_type': invalid value_type: \"unknown\"", }, { name: "testdata/no_aggregation.yaml", @@ -259,7 +259,7 @@ func TestLoadMetadata(t *testing.T) { { name: "testdata/invalid_aggregation.yaml", want: metadata{}, - wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[default.metric]': 1 error(s) decoding:\n\n* error decoding 'sum': invalid aggregation: \"invalidaggregation\"", + wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[default.metric]': 1 error(s) decoding:\n\n* error decoding 'sum': 1 error(s) decoding:\n\n* error decoding 'aggregation_temporality': invalid aggregation: \"invalidaggregation\"", }, { name: "testdata/invalid_type_attr.yaml", diff --git a/cmd/mdatagen/metricdata.go b/cmd/mdatagen/metricdata.go index d70e5324084..5bf77985cc9 100644 --- a/cmd/mdatagen/metricdata.go +++ b/cmd/mdatagen/metricdata.go @@ -28,20 +28,18 @@ type MetricData interface { type AggregationTemporality struct { // Aggregation describes if the aggregator reports delta changes // since last report time, or cumulative changes since a fixed start time. - Aggregation pmetric.AggregationTemporality `mapstructure:"aggregation_temporality"` + Aggregation pmetric.AggregationTemporality } -func (agg *AggregationTemporality) Unmarshal(parser *confmap.Conf) error { - if !parser.IsSet("aggregation_temporality") { - return errors.New("missing required field: `aggregation_temporality`") - } - switch vt := parser.Get("aggregation_temporality"); vt { +// UnmarshalText implements the encoding.TextUnmarshaler interface. +func (agg *AggregationTemporality) UnmarshalText(text []byte) error { + switch vtStr := string(text); vtStr { case "cumulative": agg.Aggregation = pmetric.AggregationTemporalityCumulative case "delta": agg.Aggregation = pmetric.AggregationTemporalityDelta default: - return fmt.Errorf("invalid aggregation: %q", vt) + return fmt.Errorf("invalid aggregation: %q", vtStr) } return nil } @@ -75,31 +73,36 @@ func (mit MetricInputType) String() string { // MetricValueType defines the metric number type. type MetricValueType struct { // ValueType is type of the metric number, options are "double", "int". - ValueType pmetric.NumberDataPointValueType `mapstructure:"value_type"` + ValueType pmetric.NumberDataPointValueType } func (mvt *MetricValueType) Unmarshal(parser *confmap.Conf) error { if !parser.IsSet("value_type") { return errors.New("missing required field: `value_type`") } - switch vt := parser.Get("value_type"); vt { + return nil +} + +// UnmarshalText implements the encoding.TextUnmarshaler interface. +func (mvt *MetricValueType) UnmarshalText(text []byte) error { + switch vtStr := string(text); vtStr { case "int": mvt.ValueType = pmetric.NumberDataPointValueTypeInt case "double": mvt.ValueType = pmetric.NumberDataPointValueTypeDouble default: - return fmt.Errorf("invalid value_type: %q", vt) + return fmt.Errorf("invalid value_type: %q", vtStr) } return nil } // Type returns name of the datapoint type. -func (mvt *MetricValueType) String() string { +func (mvt MetricValueType) String() string { return mvt.ValueType.String() } // BasicType returns name of a golang basic type for the datapoint type. -func (mvt *MetricValueType) BasicType() string { +func (mvt MetricValueType) BasicType() string { switch mvt.ValueType { case pmetric.NumberDataPointValueTypeInt: return "int64" @@ -113,10 +116,18 @@ func (mvt *MetricValueType) BasicType() string { } type gauge struct { - MetricValueType `mapstructure:",squash"` + MetricValueType `mapstructure:"value_type"` MetricInputType `mapstructure:",squash"` } +// Unmarshal is a custom unmarshaler for gauge. Needed mostly to avoid MetricValueType.Unmarshal inheritance. +func (d *gauge) Unmarshal(parser *confmap.Conf) error { + if err := d.MetricValueType.Unmarshal(parser); err != nil { + return err + } + return parser.Unmarshal(d, confmap.WithIgnoreUnused()) +} + func (d gauge) Type() string { return "Gauge" } @@ -130,12 +141,23 @@ func (d gauge) HasAggregated() bool { } type sum struct { - AggregationTemporality `mapstructure:",squash"` + AggregationTemporality `mapstructure:"aggregation_temporality"` Mono `mapstructure:",squash"` - MetricValueType `mapstructure:",squash"` + MetricValueType `mapstructure:"value_type"` MetricInputType `mapstructure:",squash"` } +// Unmarshal is a custom unmarshaler for sum. Needed mostly to avoid MetricValueType.Unmarshal inheritance. +func (d *sum) Unmarshal(parser *confmap.Conf) error { + if !parser.IsSet("aggregation_temporality") { + return errors.New("missing required field: `aggregation_temporality`") + } + if err := d.MetricValueType.Unmarshal(parser); err != nil { + return err + } + return parser.Unmarshal(d, confmap.WithIgnoreUnused()) +} + // TODO: Currently, this func will not be called because of https://github.com/open-telemetry/opentelemetry-collector/issues/6671. Uncomment function and // add a test case to Test_loadMetadata for file no_monotonic.yaml once the issue is solved. //