Skip to content

Commit

Permalink
[config] Deprecate config.UnmarshalConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
atoulme committed Mar 13, 2024
1 parent ef9caab commit 311078f
Show file tree
Hide file tree
Showing 23 changed files with 168 additions and 128 deletions.
25 changes: 25 additions & 0 deletions .chloggen/embedded_struct_unmarshaler.yaml
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: component

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Deprecate `component.UnmarshalConfig`, use `conf.Unmarshal(&intoCfg)` instead.

# One or more tracking issues or pull requests related to the change
issues: [7102]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions cmd/mdatagen/loader.go
Expand Up @@ -124,11 +124,7 @@ func (m *metric) Unmarshal(parser *confmap.Conf) error {
if !parser.IsSet("enabled") {
return errors.New("missing required field: `enabled`")
}
err := parser.Unmarshal(m)
if err != nil {
return err
}
return nil
return parser.Unmarshal(m)
}
func (m metric) Data() MetricData {
if m.Sum != nil {
Expand Down
4 changes: 2 additions & 2 deletions cmd/mdatagen/loader_test.go
Expand Up @@ -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': 1 error(s) decoding:\n\n* error decoding 'value_type': 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': invalid value_type: \"unknown\"",
},
{
name: "testdata/no_aggregation.yaml",
Expand All @@ -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': 1 error(s) decoding:\n\n* error decoding 'aggregation_temporality': invalid aggregation: \"invalidaggregation\"",
wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[default.metric]': 1 error(s) decoding:\n\n* error decoding 'sum': invalid aggregation: \"invalidaggregation\"",
},
{
name: "testdata/invalid_type_attr.yaml",
Expand Down
52 changes: 15 additions & 37 deletions cmd/mdatagen/metricdata.go
Expand Up @@ -28,18 +28,20 @@ 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
Aggregation pmetric.AggregationTemporality `mapstructure:"aggregation_temporality"`
}

// UnmarshalText implements the encoding.TextUnmarshaler interface.
func (agg *AggregationTemporality) UnmarshalText(text []byte) error {
switch vtStr := string(text); vtStr {
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 {
case "cumulative":
agg.Aggregation = pmetric.AggregationTemporalityCumulative
case "delta":
agg.Aggregation = pmetric.AggregationTemporalityDelta
default:
return fmt.Errorf("invalid aggregation: %q", vtStr)
return fmt.Errorf("invalid aggregation: %q", vt)
}
return nil
}
Expand Down Expand Up @@ -73,36 +75,31 @@ 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
ValueType pmetric.NumberDataPointValueType `mapstructure:"value_type"`
}

func (mvt *MetricValueType) Unmarshal(parser *confmap.Conf) error {
if !parser.IsSet("value_type") {
return errors.New("missing required field: `value_type`")
}
return nil
}

// UnmarshalText implements the encoding.TextUnmarshaler interface.
func (mvt *MetricValueType) UnmarshalText(text []byte) error {
switch vtStr := string(text); vtStr {
switch vt := parser.Get("value_type"); vt {
case "int":
mvt.ValueType = pmetric.NumberDataPointValueTypeInt
case "double":
mvt.ValueType = pmetric.NumberDataPointValueTypeDouble
default:
return fmt.Errorf("invalid value_type: %q", vtStr)
return fmt.Errorf("invalid value_type: %q", vt)
}
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"
Expand All @@ -116,18 +113,10 @@ func (mvt MetricValueType) BasicType() string {
}

type gauge struct {
MetricValueType `mapstructure:"value_type"`
MetricValueType `mapstructure:",squash"`
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"
}
Expand All @@ -141,23 +130,12 @@ func (d gauge) HasAggregated() bool {
}

type sum struct {
AggregationTemporality `mapstructure:"aggregation_temporality"`
AggregationTemporality `mapstructure:",squash"`
Mono `mapstructure:",squash"`
MetricValueType `mapstructure:"value_type"`
MetricValueType `mapstructure:",squash"`
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.
//
Expand Down
10 changes: 5 additions & 5 deletions cmd/mdatagen/templates/component_test.go.tmpl
Expand Up @@ -88,7 +88,7 @@ func TestComponentLifecycle(t *testing.T) {
cfg := factory.CreateDefaultConfig()
sub, err := cm.Sub("tests::config")
require.NoError(t, err)
require.NoError(t, component.UnmarshalConfig(sub, cfg))
require.NoError(t, sub.Unmarshal(&cfg))

for _, test := range tests {
{{- if not .Tests.SkipShutdown }}
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestComponentLifecycle(t *testing.T) {
cfg := factory.CreateDefaultConfig()
sub, err := cm.Sub("tests::config")
require.NoError(t, err)
require.NoError(t, component.UnmarshalConfig(sub, cfg))
require.NoError(t, sub.Unmarshal(&cfg))

for _, test := range tests {
{{- if not .Tests.SkipShutdown }}
Expand Down Expand Up @@ -268,7 +268,7 @@ func TestComponentLifecycle(t *testing.T) {
cfg := factory.CreateDefaultConfig()
sub, err := cm.Sub("tests::config")
require.NoError(t, err)
require.NoError(t, component.UnmarshalConfig(sub, cfg))
require.NoError(t, sub.Unmarshal(&cfg))

for _, test := range tests {
{{- if not .Tests.SkipShutdown }}
Expand Down Expand Up @@ -307,7 +307,7 @@ func TestComponentLifecycle(t *testing.T) {
cfg := factory.CreateDefaultConfig()
sub, err := cm.Sub("tests::config")
require.NoError(t, err)
require.NoError(t, component.UnmarshalConfig(sub, cfg))
require.NoError(t, sub.Unmarshal(&cfg))

{{- if not .Tests.SkipShutdown }}
t.Run("shutdown", func(t *testing.T) {
Expand Down Expand Up @@ -421,7 +421,7 @@ func TestComponentLifecycle(t *testing.T) {
cfg := factory.CreateDefaultConfig()
sub, err := cm.Sub("tests::config")
require.NoError(t, err)
require.NoError(t, component.UnmarshalConfig(sub, cfg))
require.NoError(t, sub.Unmarshal(&cfg))

for _, test := range tests {
{{- if not .Tests.SkipShutdown }}
Expand Down
7 changes: 1 addition & 6 deletions component/config.go
Expand Up @@ -27,13 +27,8 @@ type Config any
var configValidatorType = reflect.TypeOf((*ConfigValidator)(nil)).Elem()

// UnmarshalConfig helper function to UnmarshalConfig a Config.
// It checks if the config implements confmap.Unmarshaler and uses that if available,
// otherwise uses Map.UnmarshalExact, erroring if a field is nonexistent.
// Deprecated: Use conf.Unmarshal(&intoCfg)
func UnmarshalConfig(conf *confmap.Conf, intoCfg Config) error {
if cu, ok := intoCfg.(confmap.Unmarshaler); ok {
return cu.Unmarshal(conf)
}

return conf.Unmarshal(intoCfg)
}

Expand Down
27 changes: 0 additions & 27 deletions component/config_test.go
Expand Up @@ -11,8 +11,6 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/confmap"
)

var _ fmt.Stringer = Type{}
Expand Down Expand Up @@ -419,28 +417,3 @@ func TestNewType(t *testing.T) {
})
}
}

type configWithEmbeddedStruct struct {
String string `mapstructure:"string"`
Num int `mapstructure:"num"`
embeddedUnmarshallingConfig
}

type embeddedUnmarshallingConfig struct {
}

func (euc *embeddedUnmarshallingConfig) Unmarshal(_ *confmap.Conf) error {
return nil // do nothing.
}
func TestStructWithEmbeddedUnmarshaling(t *testing.T) {
t.Skip("Skipping, to be fixed with https://github.com/open-telemetry/opentelemetry-collector/issues/7102")
cfgMap := confmap.NewFromStringMap(map[string]any{
"string": "foo",
"num": 123,
})
tc := &configWithEmbeddedStruct{}
err := UnmarshalConfig(cfgMap, tc)
require.NoError(t, err)
assert.Equal(t, "foo", tc.String)
assert.Equal(t, 123, tc.Num)
}

0 comments on commit 311078f

Please sign in to comment.