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.

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

[otelcol] Preserve internal representation for components' configurations #10897

Merged
merged 6 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions .chloggen/mx-psi_preserve-internal-repr.yaml
Original file line number Diff line number Diff line change
@@ -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: bug_fix

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Preserve internal representation when unmarshaling component configs

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

# (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: []
10 changes: 8 additions & 2 deletions otelcol/internal/configunmarshaler/configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,25 @@
// Prepare resulting map.
c.cfgs = make(map[component.ID]component.Config)
// Iterate over raw configs and create a config for each.
for id, value := range rawCfgs {
for id := range rawCfgs {
// Find factory based on component kind and type that we read from config source.
factory, ok := c.factories[id.Type()]
if !ok {
return errorUnknownType(id, maps.Keys(c.factories))
}

// Get the configuration from the confmap.Conf to preserve internal representation.
sub, err := conf.Sub(id.String())
if err != nil {
return errorUnmarshalError(id, err)

Check warning on line 44 in otelcol/internal/configunmarshaler/configs.go

View check run for this annotation

Codecov / codecov/patch

otelcol/internal/configunmarshaler/configs.go#L44

Added line #L44 was not covered by tests
}

// Create the default config for this component.
cfg := factory.CreateDefaultConfig()

// Now that the default config struct is created we can Unmarshal into it,
// and it will apply user-defined config on top of the default.
if err := confmap.NewFromStringMap(value).Unmarshal(&cfg); err != nil {
if err := sub.Unmarshal(&cfg); err != nil {
return errorUnmarshalError(id, err)
}

Expand Down
115 changes: 115 additions & 0 deletions otelcol/unmarshal_dry_run_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package otelcol

import (
"context"
"testing"

"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/component"
codeboten marked this conversation as resolved.
Show resolved Hide resolved
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/receiver"
)

var _ component.Config = (*Config)(nil)

type ValidateTestConfig struct {
Number int `mapstructure:"number"`
String string `mapstructure:"string"`
}

var genericType component.Type = component.MustNewType("generic")

func NewFactories(_ *testing.T) func() (Factories, error) {
return func() (Factories, error) {
factories, err := nopFactories()
if err != nil {
return Factories{}, err
}
factories.Receivers[genericType] = receiver.NewFactory(
genericType,
func() component.Config {
return &ValidateTestConfig{
Number: 1,
String: "default",
}
})

return factories, nil
}
}

var sampleYAMLConfig = `
receivers:
generic:
number: ${mock:number}
string: ${mock:number}

exporters:
nop:

service:
pipelines:
traces:
receivers: [generic]
exporters: [nop]
`

func TestDryRunWithExpandedValues(t *testing.T) {
tests := []struct {
name string
yamlConfig string
mockMap map[string]string
expectErr bool
}{
{
name: "string that looks like an integer",
yamlConfig: sampleYAMLConfig,
mockMap: map[string]string{
"number": "123",
},
},
{
name: "string that looks like a bool",
yamlConfig: sampleYAMLConfig,
mockMap: map[string]string{
"number": "true",
},
expectErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
collector, err := NewCollector(CollectorSettings{
Factories: NewFactories(t),
ConfigProviderSettings: ConfigProviderSettings{
ResolverSettings: confmap.ResolverSettings{
URIs: []string{"file:file"},
DefaultScheme: "mock",
ProviderFactories: []confmap.ProviderFactory{
newFakeProvider("mock", func(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
return confmap.NewRetrievedFromYAML([]byte(tt.mockMap[uri[len("mock:"):]]))
}),
newFakeProvider("file", func(_ context.Context, _ string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
return confmap.NewRetrievedFromYAML([]byte(tt.yamlConfig))
}),
},
},
},
SkipSettingGRPCLogger: true,
})
require.NoError(t, err)

err = collector.DryRun(context.Background())
if tt.expectErr {
require.Error(t, err)
return
}
require.NoError(t, err)
})
}
}
Loading