Skip to content

Commit e8ca607

Browse files
mx-psimx-psi
authored
[receiver/otlpreceiver] Use configoptional type (#13119)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description <!-- Issue number if applicable --> Uses `configoptional.Optional` for fields in `protocols` section. Removes `Unmarshal` method since it is no longer needed. These are both breaking changes, I think it would be a bit difficult to do this in two steps, I am happy to work on fixing contrib after this is merged. #### Link to tracking issue Fixes #12980
1 parent 543c65f commit e8ca607

File tree

14 files changed

+163
-121
lines changed

14 files changed

+163
-121
lines changed

‎.chloggen/mx-psi_otlpreciever-configoptional.yaml‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: breaking
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: otlpreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Use `configoptional.Optional` to define optional configuration sections in the OTLP receiver. Remove `Unmarshal` method.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13119]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [api]

‎cmd/builder/internal/builder/main_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ var replaceModules = []string{
5050
"/config/configmiddleware",
5151
"/config/confignet",
5252
"/config/configopaque",
53+
"/config/configoptional",
5354
"/config/configretry",
5455
"/config/configtelemetry",
5556
"/config/configtls",

‎cmd/otelcorecol/builder-config.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ replaces:
4949
- go.opentelemetry.io/collector/config/configmiddleware => ../../config/configmiddleware
5050
- go.opentelemetry.io/collector/config/confignet => ../../config/confignet
5151
- go.opentelemetry.io/collector/config/configopaque => ../../config/configopaque
52+
- go.opentelemetry.io/collector/config/configoptional => ../../config/configoptional
5253
- go.opentelemetry.io/collector/config/configretry => ../../config/configretry
5354
- go.opentelemetry.io/collector/config/configtelemetry => ../../config/configtelemetry
5455
- go.opentelemetry.io/collector/config/configtls => ../../config/configtls

‎cmd/otelcorecol/go.mod‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ require (
9595
go.opentelemetry.io/collector/config/configmiddleware v0.127.0 // indirect
9696
go.opentelemetry.io/collector/config/confignet v1.33.0 // indirect
9797
go.opentelemetry.io/collector/config/configopaque v1.33.0 // indirect
98+
go.opentelemetry.io/collector/config/configoptional v0.0.0-00010101000000-000000000000 // indirect
9899
go.opentelemetry.io/collector/config/configretry v1.33.0 // indirect
99100
go.opentelemetry.io/collector/config/configtelemetry v0.127.0 // indirect
100101
go.opentelemetry.io/collector/config/configtls v1.33.0 // indirect
@@ -197,6 +198,8 @@ replace go.opentelemetry.io/collector/config/confignet => ../../config/confignet
197198

198199
replace go.opentelemetry.io/collector/config/configopaque => ../../config/configopaque
199200

201+
replace go.opentelemetry.io/collector/config/configoptional => ../../config/configoptional
202+
200203
replace go.opentelemetry.io/collector/config/configretry => ../../config/configretry
201204

202205
replace go.opentelemetry.io/collector/config/configtelemetry => ../../config/configtelemetry

‎internal/e2e/consume_contract_test.go‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@ import (
77
"testing"
88
"time"
99

10+
"github.com/stretchr/testify/require"
11+
1012
"go.opentelemetry.io/collector/component"
1113
"go.opentelemetry.io/collector/config/configgrpc"
14+
"go.opentelemetry.io/collector/config/configoptional"
1215
"go.opentelemetry.io/collector/config/configretry"
1316
"go.opentelemetry.io/collector/config/configtls"
17+
"go.opentelemetry.io/collector/confmap"
1418
"go.opentelemetry.io/collector/exporter/exporterhelper"
1519
"go.opentelemetry.io/collector/exporter/exportertest"
1620
"go.opentelemetry.io/collector/exporter/otlpexporter"
@@ -19,6 +23,19 @@ import (
1923
"go.opentelemetry.io/collector/receiver/otlpreceiver"
2024
)
2125

26+
// GetOrInsertDefault is a helper function to get or insert a default value for a configoptional.Optional type.
27+
func GetOrInsertDefault[T any](t *testing.T, opt *configoptional.Optional[T]) *T {
28+
if opt.HasValue() {
29+
return opt.Get()
30+
}
31+
32+
empty := confmap.NewFromStringMap(map[string]any{})
33+
require.NoError(t, empty.Unmarshal(opt))
34+
val := opt.Get()
35+
require.NotNil(t, "Expected a default value to be set for %T", val)
36+
return val
37+
}
38+
2239
func testExporterConfig(endpoint string) component.Config {
2340
retryConfig := configretry.NewDefaultBackOffConfig()
2441
retryConfig.InitialInterval = time.Millisecond // interval is short for the test purposes
@@ -34,10 +51,9 @@ func testExporterConfig(endpoint string) component.Config {
3451
}
3552
}
3653

37-
func testReceiverConfig(endpoint string) component.Config {
54+
func testReceiverConfig(t *testing.T, endpoint string) component.Config {
3855
cfg := otlpreceiver.NewFactory().CreateDefaultConfig()
39-
cfg.(*otlpreceiver.Config).HTTP = nil
40-
cfg.(*otlpreceiver.Config).GRPC.NetAddr.Endpoint = endpoint
56+
GetOrInsertDefault(t, &cfg.(*otlpreceiver.Config).GRPC).NetAddr.Endpoint = endpoint
4157
return cfg
4258
}
4359

@@ -52,7 +68,7 @@ func TestConsumeContractOtlpLogs(t *testing.T) {
5268
Signal: pipeline.SignalLogs,
5369
ExporterConfig: testExporterConfig(addr),
5470
ReceiverFactory: otlpreceiver.NewFactory(),
55-
ReceiverConfig: testReceiverConfig(addr),
71+
ReceiverConfig: testReceiverConfig(t, addr),
5672
})
5773
}
5874

@@ -65,7 +81,7 @@ func TestConsumeContractOtlpTraces(t *testing.T) {
6581
ExporterFactory: otlpexporter.NewFactory(),
6682
ExporterConfig: testExporterConfig(addr),
6783
ReceiverFactory: otlpreceiver.NewFactory(),
68-
ReceiverConfig: testReceiverConfig(addr),
84+
ReceiverConfig: testReceiverConfig(t, addr),
6985
})
7086
}
7187

@@ -78,6 +94,6 @@ func TestConsumeContractOtlpMetrics(t *testing.T) {
7894
Signal: pipeline.SignalMetrics,
7995
ExporterConfig: testExporterConfig(addr),
8096
ReceiverFactory: otlpreceiver.NewFactory(),
81-
ReceiverConfig: testReceiverConfig(addr),
97+
ReceiverConfig: testReceiverConfig(t, addr),
8298
})
8399
}

‎internal/e2e/go.mod‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ require (
1313
go.opentelemetry.io/collector/config/confighttp v0.127.0
1414
go.opentelemetry.io/collector/config/confignet v1.33.0
1515
go.opentelemetry.io/collector/config/configopaque v1.33.0
16+
go.opentelemetry.io/collector/config/configoptional v0.0.0-00010101000000-000000000000
1617
go.opentelemetry.io/collector/config/configretry v1.33.0
1718
go.opentelemetry.io/collector/config/configtelemetry v0.127.0
1819
go.opentelemetry.io/collector/config/configtls v1.33.0
@@ -151,6 +152,8 @@ replace go.opentelemetry.io/collector => ../..
151152

152153
replace go.opentelemetry.io/collector/config/configopaque => ../../config/configopaque
153154

155+
replace go.opentelemetry.io/collector/config/configoptional => ../../config/configoptional
156+
154157
replace go.opentelemetry.io/collector/config/configgrpc => ../../config/configgrpc
155158

156159
replace go.opentelemetry.io/collector/config/confignet => ../../config/confignet

‎internal/e2e/otlphttp_test.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,32 +327,31 @@ func createConfig(baseURL string, defaultCfg component.Config) *otlphttpexporter
327327

328328
func startTracesReceiver(t *testing.T, addr string, next consumer.Traces) {
329329
factory := otlpreceiver.NewFactory()
330-
cfg := createReceiverConfig(addr, factory.CreateDefaultConfig())
330+
cfg := createReceiverConfig(t, addr, factory.CreateDefaultConfig())
331331
recv, err := factory.CreateTraces(context.Background(), receivertest.NewNopSettings(factory.Type()), cfg, next)
332332
require.NoError(t, err)
333333
startAndCleanup(t, recv)
334334
}
335335

336336
func startMetricsReceiver(t *testing.T, addr string, next consumer.Metrics) {
337337
factory := otlpreceiver.NewFactory()
338-
cfg := createReceiverConfig(addr, factory.CreateDefaultConfig())
338+
cfg := createReceiverConfig(t, addr, factory.CreateDefaultConfig())
339339
recv, err := factory.CreateMetrics(context.Background(), receivertest.NewNopSettings(factory.Type()), cfg, next)
340340
require.NoError(t, err)
341341
startAndCleanup(t, recv)
342342
}
343343

344344
func startLogsReceiver(t *testing.T, addr string, next consumer.Logs) {
345345
factory := otlpreceiver.NewFactory()
346-
cfg := createReceiverConfig(addr, factory.CreateDefaultConfig())
346+
cfg := createReceiverConfig(t, addr, factory.CreateDefaultConfig())
347347
recv, err := factory.CreateLogs(context.Background(), receivertest.NewNopSettings(factory.Type()), cfg, next)
348348
require.NoError(t, err)
349349
startAndCleanup(t, recv)
350350
}
351351

352-
func createReceiverConfig(addr string, defaultCfg component.Config) *otlpreceiver.Config {
352+
func createReceiverConfig(t *testing.T, addr string, defaultCfg component.Config) *otlpreceiver.Config {
353353
cfg := defaultCfg.(*otlpreceiver.Config)
354-
cfg.HTTP.ServerConfig.Endpoint = addr
355-
cfg.GRPC = nil
354+
GetOrInsertDefault(t, &cfg.HTTP).ServerConfig.Endpoint = addr
356355
return cfg
357356
}
358357

‎receiver/otlpreceiver/config.go‎

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,7 @@ import (
1313
"go.opentelemetry.io/collector/component"
1414
"go.opentelemetry.io/collector/config/configgrpc"
1515
"go.opentelemetry.io/collector/config/confighttp"
16-
"go.opentelemetry.io/collector/confmap"
17-
)
18-
19-
const (
20-
// Protocol values.
21-
protoGRPC = "protocols::grpc"
22-
protoHTTP = "protocols::http"
16+
"go.opentelemetry.io/collector/config/configoptional"
2317
)
2418

2519
type SanitizedURLPath string
@@ -58,8 +52,8 @@ type HTTPConfig struct {
5852

5953
// Protocols is the configuration for the supported protocols.
6054
type Protocols struct {
61-
GRPC *configgrpc.ServerConfig `mapstructure:"grpc"`
62-
HTTP *HTTPConfig `mapstructure:"http"`
55+
GRPC configoptional.Optional[configgrpc.ServerConfig] `mapstructure:"grpc"`
56+
HTTP configoptional.Optional[HTTPConfig] `mapstructure:"http"`
6357
// prevent unkeyed literal initialization
6458
_ struct{}
6559
}
@@ -70,34 +64,12 @@ type Config struct {
7064
Protocols `mapstructure:"protocols"`
7165
}
7266

73-
var (
74-
_ component.Config = (*Config)(nil)
75-
_ confmap.Unmarshaler = (*Config)(nil)
76-
)
67+
var _ component.Config = (*Config)(nil)
7768

7869
// Validate checks the receiver configuration is valid
7970
func (cfg *Config) Validate() error {
80-
if cfg.GRPC == nil && cfg.HTTP == nil {
71+
if !cfg.GRPC.HasValue() && !cfg.HTTP.HasValue() {
8172
return errors.New("must specify at least one protocol when using the OTLP receiver")
8273
}
8374
return nil
8475
}
85-
86-
// Unmarshal a confmap.Conf into the config struct.
87-
func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
88-
// first load the config normally
89-
err := conf.Unmarshal(cfg)
90-
if err != nil {
91-
return err
92-
}
93-
94-
if !conf.IsSet(protoGRPC) {
95-
cfg.GRPC = nil
96-
}
97-
98-
if !conf.IsSet(protoHTTP) {
99-
cfg.HTTP = nil
100-
}
101-
102-
return nil
103-
}

‎receiver/otlpreceiver/config_test.go‎

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,36 @@ import (
1717
"go.opentelemetry.io/collector/config/confighttp"
1818
"go.opentelemetry.io/collector/config/confignet"
1919
"go.opentelemetry.io/collector/config/configopaque"
20+
"go.opentelemetry.io/collector/config/configoptional"
2021
"go.opentelemetry.io/collector/config/configtls"
2122
"go.opentelemetry.io/collector/confmap"
2223
"go.opentelemetry.io/collector/confmap/confmaptest"
2324
"go.opentelemetry.io/collector/confmap/xconfmap"
2425
)
2526

27+
// GetOrInsertDefault is a helper function to get or insert a default value for a configoptional.Optional type.
28+
func GetOrInsertDefault[T any](t *testing.T, opt *configoptional.Optional[T]) *T {
29+
if opt.HasValue() {
30+
return opt.Get()
31+
}
32+
33+
empty := confmap.NewFromStringMap(map[string]any{})
34+
require.NoError(t, empty.Unmarshal(opt))
35+
val := opt.Get()
36+
require.NotNil(t, "Expected a default value to be set for %T", val)
37+
return val
38+
}
39+
2640
func TestUnmarshalDefaultConfig(t *testing.T) {
2741
cm, err := confmaptest.LoadConf(filepath.Join("testdata", "default.yaml"))
2842
require.NoError(t, err)
2943
factory := NewFactory()
3044
cfg := factory.CreateDefaultConfig()
3145
require.NoError(t, cm.Unmarshal(&cfg))
32-
assert.Equal(t, factory.CreateDefaultConfig(), cfg)
46+
expectedCfg := factory.CreateDefaultConfig().(*Config)
47+
GetOrInsertDefault(t, &expectedCfg.GRPC)
48+
GetOrInsertDefault(t, &expectedCfg.HTTP)
49+
assert.Equal(t, expectedCfg, cfg)
3350
}
3451

3552
func TestUnmarshalConfigOnlyGRPC(t *testing.T) {
@@ -40,7 +57,7 @@ func TestUnmarshalConfigOnlyGRPC(t *testing.T) {
4057
require.NoError(t, cm.Unmarshal(&cfg))
4158

4259
defaultOnlyGRPC := factory.CreateDefaultConfig().(*Config)
43-
defaultOnlyGRPC.HTTP = nil
60+
GetOrInsertDefault(t, &defaultOnlyGRPC.GRPC)
4461
assert.Equal(t, defaultOnlyGRPC, cfg)
4562
}
4663

@@ -52,7 +69,7 @@ func TestUnmarshalConfigOnlyHTTP(t *testing.T) {
5269
require.NoError(t, cm.Unmarshal(&cfg))
5370

5471
defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
55-
defaultOnlyHTTP.GRPC = nil
72+
GetOrInsertDefault(t, &defaultOnlyHTTP.HTTP)
5673
assert.Equal(t, defaultOnlyHTTP, cfg)
5774
}
5875

@@ -64,7 +81,7 @@ func TestUnmarshalConfigOnlyHTTPNull(t *testing.T) {
6481
require.NoError(t, cm.Unmarshal(&cfg))
6582

6683
defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
67-
defaultOnlyHTTP.GRPC = nil
84+
GetOrInsertDefault(t, &defaultOnlyHTTP.HTTP)
6885
assert.Equal(t, defaultOnlyHTTP, cfg)
6986
}
7087

@@ -76,7 +93,7 @@ func TestUnmarshalConfigOnlyHTTPEmptyMap(t *testing.T) {
7693
require.NoError(t, cm.Unmarshal(&cfg))
7794

7895
defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
79-
defaultOnlyHTTP.GRPC = nil
96+
GetOrInsertDefault(t, &defaultOnlyHTTP.HTTP)
8097
assert.Equal(t, defaultOnlyHTTP, cfg)
8198
}
8299

@@ -89,7 +106,7 @@ func TestUnmarshalConfig(t *testing.T) {
89106
assert.Equal(t,
90107
&Config{
91108
Protocols: Protocols{
92-
GRPC: &configgrpc.ServerConfig{
109+
GRPC: configoptional.Some(configgrpc.ServerConfig{
93110
NetAddr: confignet.AddrConfig{
94111
Endpoint: "localhost:4317",
95112
Transport: confignet.TransportTypeTCP,
@@ -117,8 +134,8 @@ func TestUnmarshalConfig(t *testing.T) {
117134
PermitWithoutStream: true,
118135
},
119136
},
120-
},
121-
HTTP: &HTTPConfig{
137+
}),
138+
HTTP: configoptional.Some(HTTPConfig{
122139
ServerConfig: confighttp.ServerConfig{
123140
Auth: &confighttp.AuthConfig{
124141
Config: configauth.Config{
@@ -141,7 +158,7 @@ func TestUnmarshalConfig(t *testing.T) {
141158
TracesURLPath: "/traces",
142159
MetricsURLPath: "/v2/metrics",
143160
LogsURLPath: "/log/ingest",
144-
},
161+
}),
145162
},
146163
}, cfg)
147164
}
@@ -155,15 +172,15 @@ func TestUnmarshalConfigUnix(t *testing.T) {
155172
assert.Equal(t,
156173
&Config{
157174
Protocols: Protocols{
158-
GRPC: &configgrpc.ServerConfig{
175+
GRPC: configoptional.Some(configgrpc.ServerConfig{
159176
NetAddr: confignet.AddrConfig{
160177
Endpoint: "/tmp/grpc_otlp.sock",
161178
Transport: confignet.TransportTypeUnix,
162179
},
163180
ReadBufferSize: 512 * 1024,
164181
Keepalive: configgrpc.NewDefaultKeepaliveServerConfig(),
165-
},
166-
HTTP: &HTTPConfig{
182+
}),
183+
HTTP: configoptional.Some(HTTPConfig{
167184
ServerConfig: confighttp.ServerConfig{
168185
Endpoint: "/tmp/http_otlp.sock",
169186
CORS: confighttp.NewDefaultCORSConfig(),
@@ -172,7 +189,7 @@ func TestUnmarshalConfigUnix(t *testing.T) {
172189
TracesURLPath: defaultTracesURLPath,
173190
MetricsURLPath: defaultMetricsURLPath,
174191
LogsURLPath: defaultLogsURLPath,
175-
},
192+
}),
176193
},
177194
}, cfg)
178195
}

0 commit comments

Comments
 (0)