From 36813f5b4a3b2590b6c37c2e898b278aab2b7720 Mon Sep 17 00:00:00 2001 From: Andrzej Stencel Date: Wed, 25 Sep 2024 18:35:40 +0200 Subject: [PATCH 1/7] [chore] docs(builder): remove outdated note on builder version reporting (#11261) This has been fixed since v0.91.0: https://github.com/open-telemetry/opentelemetry-collector/pull/8994/. --- cmd/builder/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/builder/README.md b/cmd/builder/README.md index e1852948253..159ac167ae7 100644 --- a/cmd/builder/README.md +++ b/cmd/builder/README.md @@ -67,11 +67,11 @@ This is the recommended installation method. Download the binary for your respec You need to have a `go` compiler in your PATH. Run the following command to install the latest version: -``` +```console go install go.opentelemetry.io/collector/cmd/builder@latest ``` -If installing through this method the binary will be called `builder`. Binaries installed through this method [will incorrectly show `dev` as their version](https://github.com/open-telemetry/opentelemetry-collector/issues/8691). +If installing through this method the binary will be called `builder`. ## Running From 368cdd0425ae5098f3c69de8cf67b8fe91b4c130 Mon Sep 17 00:00:00 2001 From: Jade Guiton Date: Wed, 25 Sep 2024 18:44:44 +0200 Subject: [PATCH 2/7] [exporterhelper] Remove deprecated aliases for Queue/TimeoutSettings (#11264) #### Description Follow-up to #11132 now that 0.110 has been released. #### Link to tracking issue Should be the final step for #6767. --- .../exporterhelper-remove-deprecated.yaml | 25 +++++++++++++++++++ exporter/exporterhelper/common.go | 4 +-- .../exporterhelper/internal/queue_sender.go | 8 ------ exporter/exporterhelper/queue_sender.go | 8 ------ exporter/exporterhelper/timeout_sender.go | 8 ------ 5 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 .chloggen/exporterhelper-remove-deprecated.yaml diff --git a/.chloggen/exporterhelper-remove-deprecated.yaml b/.chloggen/exporterhelper-remove-deprecated.yaml new file mode 100644 index 00000000000..a025ed3fab3 --- /dev/null +++ b/.chloggen/exporterhelper-remove-deprecated.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: exporterhelper + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Removed deprecated `QueueTimeout`/`TimeoutSettings` aliases in favor of `QueueConfig`/`TimeoutConfig`. + +# One or more tracking issues or pull requests related to the change +issues: [11264, 6767] + +# (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: "`NewDefaultQueueSettings` and `NewDefaultTimeoutSettings` have been similarly renamed." + +# 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] diff --git a/exporter/exporterhelper/common.go b/exporter/exporterhelper/common.go index f3e9f06b2ec..de9822ee6aa 100644 --- a/exporter/exporterhelper/common.go +++ b/exporter/exporterhelper/common.go @@ -27,8 +27,8 @@ func WithShutdown(shutdown component.ShutdownFunc) Option { return internal.WithShutdown(shutdown) } -// WithTimeout overrides the default TimeoutSettings for an exporter. -// The default TimeoutSettings is 5 seconds. +// WithTimeout overrides the default TimeoutConfig for an exporter. +// The default TimeoutConfig is 5 seconds. func WithTimeout(timeoutConfig TimeoutConfig) Option { return internal.WithTimeout(timeoutConfig) } diff --git a/exporter/exporterhelper/internal/queue_sender.go b/exporter/exporterhelper/internal/queue_sender.go index 99f63c022d5..4e78b670b55 100644 --- a/exporter/exporterhelper/internal/queue_sender.go +++ b/exporter/exporterhelper/internal/queue_sender.go @@ -22,9 +22,6 @@ import ( const defaultQueueSize = 1000 -// Deprecated: [v0.110.0] Use QueueConfig instead. -type QueueSettings = QueueConfig - // QueueConfig defines configuration for queueing batches before sending to the consumerSender. type QueueConfig struct { // Enabled indicates whether to not enqueue batches before sending to the consumerSender. @@ -40,11 +37,6 @@ type QueueConfig struct { StorageID *component.ID `mapstructure:"storage"` } -// Deprecated: [v0.110.0] Use NewDefaultQueueConfig instead. -func NewDefaultQueueSettings() QueueSettings { - return NewDefaultQueueConfig() -} - // NewDefaultQueueConfig returns the default config for QueueConfig. func NewDefaultQueueConfig() QueueConfig { return QueueConfig{ diff --git a/exporter/exporterhelper/queue_sender.go b/exporter/exporterhelper/queue_sender.go index b81e2036fab..f09932c5b86 100644 --- a/exporter/exporterhelper/queue_sender.go +++ b/exporter/exporterhelper/queue_sender.go @@ -5,17 +5,9 @@ package exporterhelper // import "go.opentelemetry.io/collector/exporter/exporte import "go.opentelemetry.io/collector/exporter/exporterhelper/internal" -// Deprecated: [v0.110.0] Use QueueConfig instead. -type QueueSettings = internal.QueueConfig - // QueueConfig defines configuration for queueing batches before sending to the consumerSender. type QueueConfig = internal.QueueConfig -// Deprecated: [v0.110.0] Use NewDefaultQueueConfig instead. -func NewDefaultQueueSettings() QueueSettings { - return internal.NewDefaultQueueConfig() -} - // NewDefaultQueueConfig returns the default config for QueueConfig. func NewDefaultQueueConfig() QueueConfig { return internal.NewDefaultQueueConfig() diff --git a/exporter/exporterhelper/timeout_sender.go b/exporter/exporterhelper/timeout_sender.go index 9788397b7d2..090caf2d7d9 100644 --- a/exporter/exporterhelper/timeout_sender.go +++ b/exporter/exporterhelper/timeout_sender.go @@ -7,16 +7,8 @@ import ( "go.opentelemetry.io/collector/exporter/exporterhelper/internal" ) -// Deprecated: [v0.110.0] Use TimeoutConfig instead. -type TimeoutSettings = TimeoutConfig - type TimeoutConfig = internal.TimeoutConfig -// Deprecated: [v0.110.0] Use NewDefaultTimeoutConfig instead. -func NewDefaultTimeoutSettings() TimeoutSettings { - return internal.NewDefaultTimeoutConfig() -} - // NewDefaultTimeoutConfig returns the default config for TimeoutConfig. func NewDefaultTimeoutConfig() TimeoutConfig { return internal.NewDefaultTimeoutConfig() From 7589901a4363cad6ead661d5b26932cd53a67ea9 Mon Sep 17 00:00:00 2001 From: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:16:06 -0400 Subject: [PATCH 3/7] Add mdatagen replace option to check-contrib target (#11223) #### Description This PR addresses the issue of contrib tests not testing the updated mdatagen version. It modifies the `check-contrib` and `restore-contrib` targets in the Makefile to ensure that the contrib tests use the local version of the `mdatagen` package. #### Link to tracking issue Fixes #11167 #### Testing All existing tests pass with `make test` Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com> --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 58b0335e895..404c97632a9 100644 --- a/Makefile +++ b/Makefile @@ -262,6 +262,7 @@ check-contrib: @$(MAKE) -C $(CONTRIB_PATH) for-all CMD="$(GOCMD) mod edit \ -replace go.opentelemetry.io/collector=$(CURDIR) \ -replace go.opentelemetry.io/collector/client=$(CURDIR)/client \ + -replace go.opentelemetry.io/collector/cmd/mdatagen=$(CURDIR)/cmd/mdatagen \ -replace go.opentelemetry.io/collector/component=$(CURDIR)/component \ -replace go.opentelemetry.io/collector/component/componentprofiles=$(CURDIR)/component/componentprofiles \ -replace go.opentelemetry.io/collector/component/componentstatus=$(CURDIR)/component/componentstatus \ @@ -333,6 +334,7 @@ restore-contrib: @$(MAKE) -C $(CONTRIB_PATH) for-all CMD="$(GOCMD) mod edit \ -dropreplace go.opentelemetry.io/collector \ -dropreplace go.opentelemetry.io/collector/client \ + -dropreplace go.opentelemetry.io/collector/cmd/mdatagen \ -dropreplace go.opentelemetry.io/collector/component \ -dropreplace go.opentelemetry.io/collector/component/componentprofiles \ -dropreplace go.opentelemetry.io/collector/component/componentstatus \ From 0ed97aa2a06629c5fa01f2a51bdeeb75bca03a40 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Wed, 25 Sep 2024 12:51:44 -0700 Subject: [PATCH 4/7] [chore] Move grpc ClientConfig validation to Validate (#11233) Signed-off-by: Bogdan Drutu --- config/configgrpc/configgrpc.go | 18 +++--- config/configgrpc/configgrpc_test.go | 84 +++++++++++----------------- 2 files changed, 43 insertions(+), 59 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 2bf15063c57..e774bb690f7 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -202,6 +202,16 @@ func NewDefaultServerConfig() *ServerConfig { } } +func (gcs *ClientConfig) Validate() error { + if gcs.BalancerName != "" { + if balancer.Get(gcs.BalancerName) == nil { + return fmt.Errorf("invalid balancer_name: %s", gcs.BalancerName) + } + } + + return nil +} + // sanitizedEndpoint strips the prefix of either http:// or https:// from configgrpc.ClientConfig.Endpoint. func (gcs *ClientConfig) sanitizedEndpoint() string { switch { @@ -334,10 +344,6 @@ func (gcs *ClientConfig) getGrpcDialOptions( } if gcs.BalancerName != "" { - valid := validateBalancerName(gcs.BalancerName) - if !valid { - return nil, fmt.Errorf("invalid balancer_name: %s", gcs.BalancerName) - } opts = append(opts, grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingPolicy":"%s"}`, gcs.BalancerName))) } @@ -363,10 +369,6 @@ func (gcs *ClientConfig) getGrpcDialOptions( return opts, nil } -func validateBalancerName(balancerName string) bool { - return balancer.Get(balancerName) != nil -} - func (gss *ServerConfig) Validate() error { if gss.MaxRecvMsgSizeMiB*1024*1024 < 0 { return fmt.Errorf("invalid max_recv_msg_size_mib value, must be between 1 and %d: %d", math.MaxInt/1024/1024, gss.MaxRecvMsgSizeMiB) diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index e46c8a546d3..6f7398ceac6 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -18,7 +18,6 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" "google.golang.org/grpc" - "google.golang.org/grpc/balancer" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" "google.golang.org/grpc/peer" @@ -92,21 +91,6 @@ func TestNewDefaultServerConfig(t *testing.T) { assert.Equal(t, expected, result) } -// testBalancerBuilder facilitates testing validateBalancerName(). -type testBalancerBuilder struct{} - -func (testBalancerBuilder) Build(balancer.ClientConn, balancer.BuildOptions) balancer.Balancer { - return nil -} - -func (testBalancerBuilder) Name() string { - return "configgrpc_balancer_test" -} - -func init() { - balancer.Register(testBalancerBuilder{}) -} - var ( componentID = component.MustNewID("component") testAuthID = component.MustNewID("testauth") @@ -333,7 +317,7 @@ func TestGrpcServerValidate(t *testing.T) { t.Run(tt.err, func(t *testing.T) { err := tt.gss.Validate() require.Error(t, err) - assert.Regexp(t, tt.err, err) + assert.ErrorContains(t, err, tt.err) }) } } @@ -390,18 +374,37 @@ func TestGrpcServerAuthSettings(t *testing.T) { assert.NotNil(t, srv) } -func TestGRPCClientSettingsError(t *testing.T) { - tt, err := componenttest.SetupTelemetry(componentID) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) +func TestGrpcClientConfigInvalidBalancer(t *testing.T) { + settings := ClientConfig{ + Headers: map[string]configopaque.String{ + "test": "test", + }, + Endpoint: "localhost:1234", + Compression: "gzip", + TLSSetting: configtls.ClientConfig{ + Insecure: false, + }, + Keepalive: &KeepaliveClientConfig{ + Time: time.Second, + Timeout: time.Second, + PermitWithoutStream: true, + }, + ReadBufferSize: 1024, + WriteBufferSize: 1024, + WaitForReady: true, + BalancerName: "test", + } + assert.ErrorContains(t, settings.Validate(), "invalid balancer_name: test") +} +func TestGRPCClientSettingsError(t *testing.T) { tests := []struct { settings ClientConfig err string host component.Host }{ { - err: "^failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:", + err: "failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:", settings: ClientConfig{ Headers: nil, Endpoint: "", @@ -417,7 +420,7 @@ func TestGRPCClientSettingsError(t *testing.T) { }, }, { - err: "^failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither", + err: "failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither", settings: ClientConfig{ Headers: nil, Endpoint: "", @@ -432,28 +435,6 @@ func TestGRPCClientSettingsError(t *testing.T) { Keepalive: nil, }, }, - { - err: "invalid balancer_name: test", - settings: ClientConfig{ - Headers: map[string]configopaque.String{ - "test": "test", - }, - Endpoint: "localhost:1234", - Compression: "gzip", - TLSSetting: configtls.ClientConfig{ - Insecure: false, - }, - Keepalive: &KeepaliveClientConfig{ - Time: time.Second, - Timeout: time.Second, - PermitWithoutStream: true, - }, - ReadBufferSize: 1024, - WriteBufferSize: 1024, - WaitForReady: true, - BalancerName: "test", - }, - }, { err: "failed to resolve authenticator \"doesntexist\": authenticator not found", settings: ClientConfig{ @@ -506,9 +487,10 @@ func TestGRPCClientSettingsError(t *testing.T) { } for _, test := range tests { t.Run(test.err, func(t *testing.T) { - _, err := test.settings.ToClientConn(context.Background(), test.host, tt.TelemetrySettings()) + require.NoError(t, test.settings.Validate()) + _, err := test.settings.ToClientConn(context.Background(), test.host, componenttest.NewNopTelemetrySettings()) require.Error(t, err) - assert.Regexp(t, test.err, err) + assert.ErrorContains(t, err, test.err) }) } } @@ -587,7 +569,7 @@ func TestGRPCServerSettingsError(t *testing.T) { err string }{ { - err: "^failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:", + err: "failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:", settings: ServerConfig{ NetAddr: confignet.AddrConfig{ Endpoint: "127.0.0.1:1234", @@ -601,7 +583,7 @@ func TestGRPCServerSettingsError(t *testing.T) { }, }, { - err: "^failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither", + err: "failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither", settings: ServerConfig{ NetAddr: confignet.AddrConfig{ Endpoint: "127.0.0.1:1234", @@ -615,7 +597,7 @@ func TestGRPCServerSettingsError(t *testing.T) { }, }, { - err: "^failed to load client CA CertPool: failed to load CA /doesnt/exist:", + err: "failed to load client CA CertPool: failed to load CA /doesnt/exist:", settings: ServerConfig{ NetAddr: confignet.AddrConfig{ Endpoint: "127.0.0.1:1234", @@ -630,7 +612,7 @@ func TestGRPCServerSettingsError(t *testing.T) { for _, test := range tests { t.Run(test.err, func(t *testing.T) { _, err := test.settings.ToServer(context.Background(), componenttest.NewNopHost(), componenttest.NewNopTelemetrySettings()) - assert.Regexp(t, test.err, err) + assert.ErrorContains(t, err, test.err) }) } } From 3eec76c4a37d8715f1844f19d8219fbb8ab56cc5 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Wed, 25 Sep 2024 13:34:53 -0700 Subject: [PATCH 5/7] Deprecate service::telemetry::metrics::address (#11205) Existing configs like: ```yaml service: telemetry: metrics: level: "basic" address: "localhost:8888" ``` Should change to: ```yaml service: telemetry: metrics: level: "basic" readers: - pull: exporter: prometheus: host: "localhost" port: 8888 ``` --------- Signed-off-by: Bogdan Drutu Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .chloggen/deprecate-address.yaml | 20 +++ otelcol/collector_test.go | 4 +- otelcol/config_test.go | 18 ++- otelcol/go.mod | 2 +- ...address.yaml => otelcol-emptyreaders.yaml} | 2 +- .../otelcol-invalid-receiver-type.yaml | 7 +- otelcol/testdata/otelcol-invalid.yaml | 7 +- otelcol/testdata/otelcol-invalidprop.yaml | 7 +- otelcol/testdata/otelcol-nop.yaml | 7 +- ...-nometrics.yaml => otelcol-noreaders.yaml} | 0 otelcol/testdata/otelcol-statuswatcher.yaml | 7 +- otelcol/testdata/otelcol-validprop.yaml | 7 +- otelcol/unmarshaler_test.go | 7 +- service/config_test.go | 12 +- service/service.go | 1 + service/service_test.go | 86 ++---------- service/telemetry/config.go | 62 ++++++++- service/telemetry/config_test.go | 123 +++++++++++++++--- service/telemetry/factory.go | 14 +- service/telemetry/factory_test.go | 19 ++- service/telemetry/metrics.go | 29 +---- service/telemetry/metrics_test.go | 8 +- .../testdata/config_deprecated_address.yaml | 5 + ...config_deprecated_address_and_readers.yaml | 11 ++ .../testdata/config_empty_readers.yaml | 5 + .../config_invalid_deprecated_address.yaml | 5 + 26 files changed, 321 insertions(+), 154 deletions(-) create mode 100644 .chloggen/deprecate-address.yaml rename otelcol/testdata/{otelcol-noaddress.yaml => otelcol-emptyreaders.yaml} (88%) rename otelcol/testdata/{otelcol-nometrics.yaml => otelcol-noreaders.yaml} (100%) create mode 100644 service/telemetry/testdata/config_deprecated_address.yaml create mode 100644 service/telemetry/testdata/config_deprecated_address_and_readers.yaml create mode 100644 service/telemetry/testdata/config_empty_readers.yaml create mode 100644 service/telemetry/testdata/config_invalid_deprecated_address.yaml diff --git a/.chloggen/deprecate-address.yaml b/.chloggen/deprecate-address.yaml new file mode 100644 index 00000000000..2ebaab41b54 --- /dev/null +++ b/.chloggen/deprecate-address.yaml @@ -0,0 +1,20 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'deprecation' + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: service/telemetry + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Deprecate service::telemetry::metrics::address in favor of service::telemetry::metrics::readers. + +# One or more tracking issues or pull requests related to the change +issues: [11205] + +# 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: [user] diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index 7db8215eb39..2e84e32b876 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -389,8 +389,8 @@ func TestCollectorRun(t *testing.T) { tests := []struct { file string }{ - {file: "otelcol-nometrics.yaml"}, - {file: "otelcol-noaddress.yaml"}, + {file: "otelcol-noreaders.yaml"}, + {file: "otelcol-emptyreaders.yaml"}, } for _, tt := range tests { diff --git a/otelcol/config_test.go b/otelcol/config_test.go index 25b1417cd2c..c1e43afba69 100644 --- a/otelcol/config_test.go +++ b/otelcol/config_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/contrib/config" "go.uber.org/zap/zapcore" "go.opentelemetry.io/collector/component" @@ -275,8 +276,17 @@ func generateConfig() *Config { InitialFields: map[string]any{"fieldKey": "filed-value"}, }, Metrics: telemetry.MetricsConfig{ - Level: configtelemetry.LevelNormal, - Address: ":8080", + Level: configtelemetry.LevelNormal, + Readers: []config.MetricReader{ + { + Pull: &config.PullMetricReader{Exporter: config.MetricExporter{ + Prometheus: &config.Prometheus{ + Host: newPtr("localhost"), + Port: newPtr(8080), + }, + }}, + }, + }, }, }, Extensions: []component.ID{component.MustNewID("nop")}, @@ -290,3 +300,7 @@ func generateConfig() *Config { }, } } + +func newPtr[T int | string](str T) *T { + return &str +} diff --git a/otelcol/go.mod b/otelcol/go.mod index 34c33f0dc94..685d785e5ed 100644 --- a/otelcol/go.mod +++ b/otelcol/go.mod @@ -17,6 +17,7 @@ require ( go.opentelemetry.io/collector/processor v0.110.0 go.opentelemetry.io/collector/receiver v0.110.0 go.opentelemetry.io/collector/service v0.110.0 + go.opentelemetry.io/contrib/config v0.10.0 go.uber.org/goleak v1.3.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 @@ -79,7 +80,6 @@ require ( go.opentelemetry.io/collector/processor/processorprofiles v0.110.0 // indirect go.opentelemetry.io/collector/receiver/receiverprofiles v0.110.0 // indirect go.opentelemetry.io/collector/semconv v0.110.0 // indirect - go.opentelemetry.io/contrib/config v0.10.0 // indirect go.opentelemetry.io/contrib/propagators/b3 v1.30.0 // indirect go.opentelemetry.io/otel v1.30.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.6.0 // indirect diff --git a/otelcol/testdata/otelcol-noaddress.yaml b/otelcol/testdata/otelcol-emptyreaders.yaml similarity index 88% rename from otelcol/testdata/otelcol-noaddress.yaml rename to otelcol/testdata/otelcol-emptyreaders.yaml index c363f848e53..3a71b541f1a 100644 --- a/otelcol/testdata/otelcol-noaddress.yaml +++ b/otelcol/testdata/otelcol-emptyreaders.yaml @@ -7,7 +7,7 @@ exporters: service: telemetry: metrics: - address: "" + readers: [] pipelines: metrics: receivers: [nop] diff --git a/otelcol/testdata/otelcol-invalid-receiver-type.yaml b/otelcol/testdata/otelcol-invalid-receiver-type.yaml index 5837810fcca..7bcad410bb9 100644 --- a/otelcol/testdata/otelcol-invalid-receiver-type.yaml +++ b/otelcol/testdata/otelcol-invalid-receiver-type.yaml @@ -10,7 +10,12 @@ exporters: service: telemetry: metrics: - address: localhost:8888 + readers: + - pull: + exporter: + prometheus: + host: "localhost" + port: 8888 pipelines: traces: receivers: [nop_logs] diff --git a/otelcol/testdata/otelcol-invalid.yaml b/otelcol/testdata/otelcol-invalid.yaml index 133ce74d85b..dc48aee782f 100644 --- a/otelcol/testdata/otelcol-invalid.yaml +++ b/otelcol/testdata/otelcol-invalid.yaml @@ -10,7 +10,12 @@ exporters: service: telemetry: metrics: - address: localhost:8888 + readers: + - pull: + exporter: + prometheus: + host: "localhost" + port: 8888 pipelines: traces: receivers: [nop] diff --git a/otelcol/testdata/otelcol-invalidprop.yaml b/otelcol/testdata/otelcol-invalidprop.yaml index c84fbdbd017..4022c90cbea 100644 --- a/otelcol/testdata/otelcol-invalidprop.yaml +++ b/otelcol/testdata/otelcol-invalidprop.yaml @@ -14,7 +14,12 @@ service: - "unknown" - "tracecontext" metrics: - address: localhost:8888 + readers: + - pull: + exporter: + prometheus: + host: "localhost" + port: 8888 pipelines: traces: receivers: [nop] diff --git a/otelcol/testdata/otelcol-nop.yaml b/otelcol/testdata/otelcol-nop.yaml index c83a07c536c..3d361b0afdd 100644 --- a/otelcol/testdata/otelcol-nop.yaml +++ b/otelcol/testdata/otelcol-nop.yaml @@ -16,7 +16,12 @@ connectors: service: telemetry: metrics: - address: localhost:8888 + readers: + - pull: + exporter: + prometheus: + host: "localhost" + port: 8888 extensions: [nop] pipelines: traces: diff --git a/otelcol/testdata/otelcol-nometrics.yaml b/otelcol/testdata/otelcol-noreaders.yaml similarity index 100% rename from otelcol/testdata/otelcol-nometrics.yaml rename to otelcol/testdata/otelcol-noreaders.yaml diff --git a/otelcol/testdata/otelcol-statuswatcher.yaml b/otelcol/testdata/otelcol-statuswatcher.yaml index 2dcc322d341..90970a234d7 100644 --- a/otelcol/testdata/otelcol-statuswatcher.yaml +++ b/otelcol/testdata/otelcol-statuswatcher.yaml @@ -14,7 +14,12 @@ extensions: service: telemetry: metrics: - address: localhost:8888 + readers: + - pull: + exporter: + prometheus: + host: "localhost" + port: 8888 extensions: [statuswatcher] pipelines: traces: diff --git a/otelcol/testdata/otelcol-validprop.yaml b/otelcol/testdata/otelcol-validprop.yaml index 5c611333c17..1d478cabae9 100644 --- a/otelcol/testdata/otelcol-validprop.yaml +++ b/otelcol/testdata/otelcol-validprop.yaml @@ -14,7 +14,12 @@ service: - "b3" - "tracecontext" metrics: - address: localhost:8888 + readers: + - pull: + exporter: + prometheus: + host: "localhost" + port: 8888 pipelines: traces: receivers: [nop] diff --git a/otelcol/unmarshaler_test.go b/otelcol/unmarshaler_test.go index 436d88c0c0f..9fdb0832491 100644 --- a/otelcol/unmarshaler_test.go +++ b/otelcol/unmarshaler_test.go @@ -159,7 +159,7 @@ func TestServiceUnmarshalError(t *testing.T) { }, }, }), - expectError: "error decoding 'telemetry.logs.level': unrecognized level: \"UNKNOWN\"", + expectError: "error decoding 'telemetry': decoding failed due to the following error(s):\n\nerror decoding 'logs.level': unrecognized level: \"UNKNOWN\"", }, { name: "invalid-metrics-level", @@ -170,7 +170,7 @@ func TestServiceUnmarshalError(t *testing.T) { }, }, }), - expectError: "error decoding 'telemetry.metrics.level': unknown metrics level \"unknown\"", + expectError: "error decoding 'telemetry': decoding failed due to the following error(s):\n\nerror decoding 'metrics.level': unknown metrics level \"unknown\"", }, { name: "invalid-service-extensions-section", @@ -204,8 +204,7 @@ func TestServiceUnmarshalError(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { err := tt.conf.Unmarshal(&service.Config{}) - require.Error(t, err) - assert.Contains(t, err.Error(), tt.expectError) + require.ErrorContains(t, err, tt.expectError) }) } } diff --git a/service/config_test.go b/service/config_test.go index 2dd0246e734..413887de731 100644 --- a/service/config_test.go +++ b/service/config_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/contrib/config" "go.uber.org/zap/zapcore" "go.opentelemetry.io/collector/component" @@ -67,7 +68,7 @@ func TestConfigValidate(t *testing.T) { cfgFn: func() *Config { cfg := generateConfig() cfg.Telemetry.Metrics.Level = configtelemetry.LevelBasic - cfg.Telemetry.Metrics.Address = "" + cfg.Telemetry.Metrics.Readers = nil return cfg }, expected: nil, @@ -96,8 +97,13 @@ func generateConfig() *Config { InitialFields: map[string]any{"fieldKey": "filed-value"}, }, Metrics: telemetry.MetricsConfig{ - Level: configtelemetry.LevelNormal, - Address: ":8080", + Level: configtelemetry.LevelNormal, + Readers: []config.MetricReader{{ + Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{ + Host: newPtr("localhost"), + Port: newPtr(8080), + }}}}, + }, }, }, Extensions: extensions.Config{component.MustNewID("nop")}, diff --git a/service/service.go b/service/service.go index 83a8a12c848..ada723b7c90 100644 --- a/service/service.go +++ b/service/service.go @@ -187,6 +187,7 @@ func logsAboutMeterProvider(logger *zap.Logger, cfg telemetry.MetricsConfig, mp return } + //nolint if len(cfg.Address) != 0 { logger.Warn("service::telemetry::metrics::address is being deprecated in favor of service::telemetry::metrics::readers") } diff --git a/service/service_test.go b/service/service_test.go index 8420e52a62c..23cbaa5509f 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -289,78 +289,6 @@ func TestServiceTelemetryCleanupOnError(t *testing.T) { } func TestServiceTelemetry(t *testing.T) { - for _, tc := range ownMetricsTestCases() { - t.Run(fmt.Sprintf("ipv4_%s", tc.name), func(t *testing.T) { - testCollectorStartHelper(t, tc, "tcp4") - }) - t.Run(fmt.Sprintf("ipv6_%s", tc.name), func(t *testing.T) { - testCollectorStartHelper(t, tc, "tcp6") - }) - } -} - -func testCollectorStartHelper(t *testing.T, tc ownMetricsTestCase, network string) { - var once sync.Once - loggingHookCalled := false - hook := func(zapcore.Entry) error { - once.Do(func() { - loggingHookCalled = true - }) - return nil - } - - var ( - metricsAddr string - zpagesAddr string - ) - switch network { - case "tcp", "tcp4": - metricsAddr = testutil.GetAvailableLocalAddress(t) - zpagesAddr = testutil.GetAvailableLocalAddress(t) - case "tcp6": - metricsAddr = testutil.GetAvailableLocalIPv6Address(t) - zpagesAddr = testutil.GetAvailableLocalIPv6Address(t) - } - require.NotZero(t, metricsAddr, "network must be either of tcp, tcp4 or tcp6") - require.NotZero(t, zpagesAddr, "network must be either of tcp, tcp4 or tcp6") - - set := newNopSettings() - set.BuildInfo = component.BuildInfo{Version: "test version", Command: otelCommand} - set.ExtensionsConfigs = map[component.ID]component.Config{ - component.MustNewID("zpages"): &zpagesextension.Config{ - ServerConfig: confighttp.ServerConfig{Endpoint: zpagesAddr}, - }, - } - set.ExtensionsFactories = map[component.Type]extension.Factory{component.MustNewType("zpages"): zpagesextension.NewFactory()} - set.LoggingOptions = []zap.Option{zap.Hooks(hook)} - - cfg := newNopConfig() - cfg.Extensions = []component.ID{component.MustNewID("zpages")} - cfg.Telemetry.Metrics.Address = metricsAddr - cfg.Telemetry.Resource = make(map[string]*string) - // Include resource attributes under the service::telemetry::resource key. - for k, v := range tc.userDefinedResource { - cfg.Telemetry.Resource[k] = v - } - - // Create a service, check for metrics, shutdown and repeat to ensure that telemetry can be started/shutdown and started again. - for i := 0; i < 2; i++ { - srv, err := New(context.Background(), set, cfg) - require.NoError(t, err) - - require.NoError(t, srv.Start(context.Background())) - // Sleep for 1 second to ensure the http server is started. - time.Sleep(1 * time.Second) - assert.True(t, loggingHookCalled) - - assertResourceLabels(t, srv.telemetrySettings.Resource, tc.expectedLabels) - assertMetrics(t, metricsAddr, tc.expectedLabels) - assertZPages(t, zpagesAddr) - require.NoError(t, srv.Shutdown(context.Background())) - } -} - -func TestServiceTelemetryWithReaders(t *testing.T) { for _, tc := range ownMetricsTestCases() { t.Run(fmt.Sprintf("ipv4_%s", tc.name), func(t *testing.T) { testCollectorStartHelperWithReaders(t, tc, "tcp4") @@ -408,7 +336,6 @@ func testCollectorStartHelperWithReaders(t *testing.T, tc ownMetricsTestCase, ne cfg := newNopConfig() cfg.Extensions = []component.ID{component.MustNewID("zpages")} - cfg.Telemetry.Metrics.Address = "" cfg.Telemetry.Metrics.Readers = []config.MetricReader{ { Pull: &config.PullMetricReader{ @@ -742,8 +669,13 @@ func newNopConfigPipelineConfigs(pipelineCfgs pipelines.ConfigWithPipelineID) Co InitialFields: map[string]any(nil), }, Metrics: telemetry.MetricsConfig{ - Level: configtelemetry.LevelBasic, - Address: "localhost:8888", + Level: configtelemetry.LevelBasic, + Readers: []config.MetricReader{{ + Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{ + Host: newPtr("localhost"), + Port: newPtr(8888), + }}}}, + }, }, }, } @@ -775,3 +707,7 @@ func newConfigWatcherExtensionFactory(name component.Type) extension.Factory { component.StabilityLevelDevelopment, ) } + +func newPtr[T int | string](str T) *T { + return &str +} diff --git a/service/telemetry/config.go b/service/telemetry/config.go index 197adf491aa..29cfb7afb1a 100644 --- a/service/telemetry/config.go +++ b/service/telemetry/config.go @@ -5,14 +5,27 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry" import ( "fmt" + "net" + "strconv" "time" "go.opentelemetry.io/contrib/config" "go.uber.org/zap/zapcore" "go.opentelemetry.io/collector/config/configtelemetry" + "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/featuregate" ) +var _ confmap.Unmarshaler = (*Config)(nil) + +var disableAddressFieldForInternalTelemetryFeatureGate = featuregate.GlobalRegistry().MustRegister( + "telemetry.disableAddressFieldForInternalTelemetry", + featuregate.StageAlpha, + featuregate.WithRegisterFromVersion("v0.111.0"), + featuregate.WithRegisterToVersion("v0.114.0"), + featuregate.WithRegisterDescription("controls whether the deprecated address field for internal telemetry is still supported")) + // Config defines the configurable settings for service telemetry. type Config struct { Logs LogsConfig `mapstructure:"logs"` @@ -119,7 +132,7 @@ type MetricsConfig struct { // - "detailed" adds dimensions and views to the previous levels. Level configtelemetry.Level `mapstructure:"level"` - // Address is the [address]:port that metrics exposition should be bound to. + // Deprecated: [v0.111.0] use readers configuration. Address string `mapstructure:"address"` // Readers allow configuration of metric readers to emit metrics to @@ -143,11 +156,52 @@ type TracesConfig struct { Processors []config.SpanProcessor `mapstructure:"processors"` } +func (c *Config) Unmarshal(conf *confmap.Conf) error { + if err := conf.Unmarshal(c); err != nil { + return err + } + + // If the support for "metrics::address" is disabled, nothing to do. + // TODO: when this gate is marked stable remove the whole Unmarshal definition. + if disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled() { + return nil + } + + if len(c.Metrics.Address) != 0 { + host, port, err := net.SplitHostPort(c.Metrics.Address) + if err != nil { + return fmt.Errorf("failing to parse metrics address %q: %w", c.Metrics.Address, err) + } + portInt, err := strconv.Atoi(port) + if err != nil { + return fmt.Errorf("failing to extract the port from the metrics address %q: %w", c.Metrics.Address, err) + } + + // User did not overwrite readers, so we will remove the default configured reader. + if !conf.IsSet("metrics::readers") { + c.Metrics.Readers = nil + } + + c.Metrics.Readers = append(c.Metrics.Readers, config.MetricReader{ + Pull: &config.PullMetricReader{ + Exporter: config.MetricExporter{ + Prometheus: &config.Prometheus{ + Host: &host, + Port: &portInt, + }, + }, + }, + }) + } + + return nil +} + // Validate checks whether the current configuration is valid func (c *Config) Validate() error { - // Check when service telemetry metric level is not none, the metrics address should not be empty - if c.Metrics.Level != configtelemetry.LevelNone && c.Metrics.Address == "" && len(c.Metrics.Readers) == 0 { - return fmt.Errorf("collector telemetry metric address or reader should exist when metric level is not none") + // Check when service telemetry metric level is not none, the metrics readers should not be empty + if c.Metrics.Level != configtelemetry.LevelNone && len(c.Metrics.Readers) == 0 { + return fmt.Errorf("collector telemetry metrics reader should exist when metric level is not none") } return nil diff --git a/service/telemetry/config_test.go b/service/telemetry/config_test.go index 5417347c772..4bd7c81bcaa 100644 --- a/service/telemetry/config_test.go +++ b/service/telemetry/config_test.go @@ -4,15 +4,109 @@ package telemetry import ( + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/contrib/config" + "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config/configtelemetry" + "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/confmap/confmaptest" + "go.opentelemetry.io/collector/featuregate" ) -func TestLoadConfig(t *testing.T) { +func TestComponentConfigStruct(t *testing.T) { + require.NoError(t, componenttest.CheckConfigStruct(NewFactory().CreateDefaultConfig())) +} + +func TestUnmarshalDefaultConfig(t *testing.T) { + factory := NewFactory() + cfg := factory.CreateDefaultConfig() + require.NoError(t, confmap.New().Unmarshal(&cfg)) + assert.Equal(t, factory.CreateDefaultConfig(), cfg) +} + +func TestUnmarshalEmptyMetricReaders(t *testing.T) { + prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled() + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), false)) + defer func() { + // Restore previous value. + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev)) + }() + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_empty_readers.yaml")) + require.NoError(t, err) + cfg := NewFactory().CreateDefaultConfig() + require.NoError(t, cm.Unmarshal(&cfg)) + require.Empty(t, cfg.(*Config).Metrics.Readers) +} + +func TestUnmarshalConfigDeprecatedAddress(t *testing.T) { + prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled() + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), false)) + defer func() { + // Restore previous value. + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev)) + }() + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_deprecated_address.yaml")) + require.NoError(t, err) + cfg := NewFactory().CreateDefaultConfig() + require.NoError(t, cm.Unmarshal(&cfg)) + require.Len(t, cfg.(*Config).Metrics.Readers, 1) + assert.Equal(t, "localhost", *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Host) + assert.Equal(t, 6666, *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Port) +} + +func TestUnmarshalConfigDeprecatedAddressGateEnabled(t *testing.T) { + prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled() + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), true)) + defer func() { + // Restore previous value. + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev)) + }() + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_deprecated_address.yaml")) + require.NoError(t, err) + cfg := NewFactory().CreateDefaultConfig() + require.NoError(t, cm.Unmarshal(&cfg)) + require.Len(t, cfg.(*Config).Metrics.Readers, 1) + assert.Equal(t, "", *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Host) + assert.Equal(t, 8888, *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Port) +} + +func TestUnmarshalConfigInvalidDeprecatedAddress(t *testing.T) { + prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled() + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), false)) + defer func() { + // Restore previous value. + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev)) + }() + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_invalid_deprecated_address.yaml")) + require.NoError(t, err) + cfg := NewFactory().CreateDefaultConfig() + require.Error(t, cm.Unmarshal(&cfg)) +} + +func TestUnmarshalConfigDeprecatedAddressAndReaders(t *testing.T) { + prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled() + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), false)) + defer func() { + // Restore previous value. + require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev)) + }() + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_deprecated_address_and_readers.yaml")) + require.NoError(t, err) + cfg := NewFactory().CreateDefaultConfig() + require.NoError(t, cm.Unmarshal(&cfg)) + require.Len(t, cfg.(*Config).Metrics.Readers, 2) + assert.Equal(t, "localhost", *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Host) + assert.Equal(t, 9999, *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Port) + assert.Equal(t, "192.168.0.1", *cfg.(*Config).Metrics.Readers[1].Pull.Exporter.Prometheus.Host) + assert.Equal(t, 6666, *cfg.(*Config).Metrics.Readers[1].Pull.Exporter.Prometheus.Port) +} + +func TestConfigValidate(t *testing.T) { tests := []struct { name string cfg *Config @@ -22,8 +116,13 @@ func TestLoadConfig(t *testing.T) { name: "basic metric telemetry", cfg: &Config{ Metrics: MetricsConfig{ - Level: configtelemetry.LevelBasic, - Address: "127.0.0.1:3333", + Level: configtelemetry.LevelBasic, + Readers: []config.MetricReader{{ + Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{ + Host: newPtr("127.0.0.1"), + Port: newPtr(3333), + }}}}, + }, }, }, success: true, @@ -33,32 +132,20 @@ func TestLoadConfig(t *testing.T) { cfg: &Config{ Metrics: MetricsConfig{ Level: configtelemetry.LevelBasic, - Address: "", + Readers: nil, }, }, success: false, }, - { - name: "valid metric telemetry with metric readers", - cfg: &Config{ - Metrics: MetricsConfig{ - Level: configtelemetry.LevelBasic, - Readers: []config.MetricReader{ - {Pull: &config.PullMetricReader{}}, - }, - }, - }, - success: true, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := tt.cfg.Validate() if tt.success { - assert.NoError(t, err) + require.NoError(t, err) } else { - assert.Error(t, err) + require.Error(t, err) } }) } diff --git a/service/telemetry/factory.go b/service/telemetry/factory.go index 474c245c22d..24802271e31 100644 --- a/service/telemetry/factory.go +++ b/service/telemetry/factory.go @@ -7,6 +7,7 @@ import ( "context" "time" + "go.opentelemetry.io/contrib/config" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -99,8 +100,17 @@ func createDefaultConfig() component.Config { InitialFields: map[string]any(nil), }, Metrics: MetricsConfig{ - Level: configtelemetry.LevelNormal, - Address: ":8888", + Level: configtelemetry.LevelNormal, + Readers: []config.MetricReader{{ + Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{ + Host: newPtr(""), + Port: newPtr(8888), + }}}}, + }, }, } } + +func newPtr[T int | string](str T) *T { + return &str +} diff --git a/service/telemetry/factory_test.go b/service/telemetry/factory_test.go index f777f8dbbdc..75f4196c6ce 100644 --- a/service/telemetry/factory_test.go +++ b/service/telemetry/factory_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/contrib/config" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -30,8 +31,13 @@ func TestTelemetryConfiguration(t *testing.T) { Encoding: "console", }, Metrics: MetricsConfig{ - Level: configtelemetry.LevelBasic, - Address: "127.0.0.1:3333", + Level: configtelemetry.LevelBasic, + Readers: []config.MetricReader{{ + Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{ + Host: newPtr("127.0.0.1"), + Port: newPtr(3333), + }}}}, + }, }, }, success: true, @@ -43,8 +49,13 @@ func TestTelemetryConfiguration(t *testing.T) { Level: zapcore.DebugLevel, }, Metrics: MetricsConfig{ - Level: configtelemetry.LevelBasic, - Address: "127.0.0.1:3333", + Level: configtelemetry.LevelBasic, + Readers: []config.MetricReader{{ + Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{ + Host: newPtr("127.0.0.1"), + Port: newPtr(3333), + }}}}, + }, }, }, success: false, diff --git a/service/telemetry/metrics.go b/service/telemetry/metrics.go index 76b0a758b5a..a3be9169c7c 100644 --- a/service/telemetry/metrics.go +++ b/service/telemetry/metrics.go @@ -5,12 +5,9 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry" import ( "context" - "net" "net/http" - "strconv" "sync" - "go.opentelemetry.io/contrib/config" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" sdkmetric "go.opentelemetry.io/otel/sdk/metric" @@ -40,34 +37,10 @@ type meterProviderSettings struct { } func newMeterProvider(set meterProviderSettings, disableHighCardinality bool) (metric.MeterProvider, error) { - if set.cfg.Level == configtelemetry.LevelNone || (set.cfg.Address == "" && len(set.cfg.Readers) == 0) { + if set.cfg.Level == configtelemetry.LevelNone || len(set.cfg.Readers) == 0 { return noop.NewMeterProvider(), nil } - if len(set.cfg.Address) != 0 { - host, port, err := net.SplitHostPort(set.cfg.Address) - if err != nil { - return nil, err - } - portInt, err := strconv.Atoi(port) - if err != nil { - return nil, err - } - if set.cfg.Readers == nil { - set.cfg.Readers = []config.MetricReader{} - } - set.cfg.Readers = append(set.cfg.Readers, config.MetricReader{ - Pull: &config.PullMetricReader{ - Exporter: config.MetricExporter{ - Prometheus: &config.Prometheus{ - Host: &host, - Port: &portInt, - }, - }, - }, - }) - } - mp := &meterProvider{} var opts []sdkmetric.Option for _, reader := range set.cfg.Readers { diff --git a/service/telemetry/metrics_test.go b/service/telemetry/metrics_test.go index f3bb140b1f3..3446ad219d6 100644 --- a/service/telemetry/metrics_test.go +++ b/service/telemetry/metrics_test.go @@ -17,7 +17,6 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configtelemetry" - "go.opentelemetry.io/collector/internal/testutil" semconv "go.opentelemetry.io/collector/semconv/v1.18.0" "go.opentelemetry.io/collector/service/internal/promtest" "go.opentelemetry.io/collector/service/internal/resource" @@ -208,8 +207,10 @@ func TestTelemetryInit(t *testing.T) { semconv.AttributeServiceInstanceID: &testInstanceID, }, Metrics: MetricsConfig{ - Level: configtelemetry.LevelDetailed, - Address: testutil.GetAvailableLocalAddress(t), + Level: configtelemetry.LevelDetailed, + Readers: []config.MetricReader{{ + Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: promtest.GetAvailableLocalAddressPrometheus(t)}}, + }}, }, } } @@ -276,5 +277,4 @@ func getMetricsFromPrometheus(t *testing.T, handler http.Handler) map[string]*io require.NoError(t, err) return parsed - } diff --git a/service/telemetry/testdata/config_deprecated_address.yaml b/service/telemetry/testdata/config_deprecated_address.yaml new file mode 100644 index 00000000000..abb68a70ad2 --- /dev/null +++ b/service/telemetry/testdata/config_deprecated_address.yaml @@ -0,0 +1,5 @@ +logs: + level: "info" +metrics: + level: "basic" + address: "localhost:6666" diff --git a/service/telemetry/testdata/config_deprecated_address_and_readers.yaml b/service/telemetry/testdata/config_deprecated_address_and_readers.yaml new file mode 100644 index 00000000000..987b9f5a7d6 --- /dev/null +++ b/service/telemetry/testdata/config_deprecated_address_and_readers.yaml @@ -0,0 +1,11 @@ +logs: + level: "info" +metrics: + level: "basic" + address: "192.168.0.1:6666" + readers: + - pull: + exporter: + prometheus: + host: "localhost" + port: 9999 diff --git a/service/telemetry/testdata/config_empty_readers.yaml b/service/telemetry/testdata/config_empty_readers.yaml new file mode 100644 index 00000000000..9c2fb6bd7b9 --- /dev/null +++ b/service/telemetry/testdata/config_empty_readers.yaml @@ -0,0 +1,5 @@ +logs: + level: "info" +metrics: + level: "basic" + readers: [] diff --git a/service/telemetry/testdata/config_invalid_deprecated_address.yaml b/service/telemetry/testdata/config_invalid_deprecated_address.yaml new file mode 100644 index 00000000000..fd81b91ad19 --- /dev/null +++ b/service/telemetry/testdata/config_invalid_deprecated_address.yaml @@ -0,0 +1,5 @@ +logs: + level: "info" +metrics: + level: "basic" + address: "1212:212121:2121" From 5aff73a243b9e8c8ac3543ba830551a8fa29c112 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Wed, 25 Sep 2024 13:35:04 -0700 Subject: [PATCH 6/7] [chore] Fix internal variable name (#11278) Signed-off-by: Bogdan Drutu --- service/telemetry/factory.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/service/telemetry/factory.go b/service/telemetry/factory.go index 24802271e31..630fe49f4e4 100644 --- a/service/telemetry/factory.go +++ b/service/telemetry/factory.go @@ -19,9 +19,9 @@ import ( "go.opentelemetry.io/collector/service/internal/resource" ) -// disableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable +// disableHighCardinalityMetricsFeatureGate is the feature gate that controls whether the collector should enable // potentially high cardinality metrics. The gate will be removed when the collector allows for view configuration. -var disableHighCardinalityMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister( +var disableHighCardinalityMetricsFeatureGate = featuregate.GlobalRegistry().MustRegister( "telemetry.disableHighCardinalityMetrics", featuregate.StageAlpha, featuregate.WithRegisterDescription("controls whether the collector should enable potentially high"+ @@ -68,7 +68,7 @@ func NewFactory() Factory { }), withMeterProvider(func(_ context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) { c := *cfg.(*Config) - disableHighCard := disableHighCardinalityMetricsfeatureGate.IsEnabled() + disableHighCard := disableHighCardinalityMetricsFeatureGate.IsEnabled() return newMeterProvider( meterProviderSettings{ res: resource.New(set.BuildInfo, c.Resource), From 04379ebb6597bd27fe896f38fdd2b40128079b8a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 25 Sep 2024 13:48:55 -0700 Subject: [PATCH 7/7] Unmarshal span and link flags from JSON (#11275) For both the `Span` and `SpanLink`, the current unmarshalJsoniter methods do not unmarshal the `flags` field. This updates the functionality to correctly decode these values into the destination. Fix #11267 Signed-off-by: Tyler Yahn --------- Signed-off-by: Tyler Yahn Co-authored-by: Bogdan Drutu Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .chloggen/json-decoded-flag.yaml | 25 +++++++++++++++++++++++++ pdata/ptrace/json.go | 4 ++++ pdata/ptrace/json_test.go | 4 +++- 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 .chloggen/json-decoded-flag.yaml diff --git a/.chloggen/json-decoded-flag.yaml b/.chloggen/json-decoded-flag.yaml new file mode 100644 index 00000000000..05734f7ee31 --- /dev/null +++ b/.chloggen/json-decoded-flag.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: 'bug_fix' + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: 'pdata' + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Unmarshal Span and SpanLink flags from JSON" + +# One or more tracking issues or pull requests related to the change +issues: [11267] + +# (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: [] diff --git a/pdata/ptrace/json.go b/pdata/ptrace/json.go index 64bd273ba96..b0926613e5c 100644 --- a/pdata/ptrace/json.go +++ b/pdata/ptrace/json.go @@ -112,6 +112,8 @@ func (dest Span) unmarshalJsoniter(iter *jsoniter.Iterator) { if err := dest.orig.ParentSpanId.UnmarshalJSON([]byte(iter.ReadString())); err != nil { iter.ReportError("readSpan.parentSpanId", fmt.Sprintf("parse parent_span_id:%v", err)) } + case "flags": + dest.orig.Flags = iter.ReadUint32() case "name": dest.orig.Name = iter.ReadString() case "kind": @@ -184,6 +186,8 @@ func (dest SpanLink) unmarshalJsoniter(iter *jsoniter.Iterator) { }) case "droppedAttributesCount", "dropped_attributes_count": dest.orig.DroppedAttributesCount = json.ReadUint32(iter) + case "flags": + dest.orig.Flags = json.ReadUint32(iter) default: iter.Skip() } diff --git a/pdata/ptrace/json_test.go b/pdata/ptrace/json_test.go index e0441760800..1be7d0061c0 100644 --- a/pdata/ptrace/json_test.go +++ b/pdata/ptrace/json_test.go @@ -37,6 +37,7 @@ var tracesOTLP = func() Traces { il.SetSchemaUrl("schemaURL") // Add spans. sp := il.Spans().AppendEmpty() + sp.SetFlags(1) sp.SetName("testSpan") sp.SetKind(SpanKindClient) sp.SetDroppedAttributesCount(1) @@ -83,13 +84,14 @@ var tracesOTLP = func() Traces { link.Attributes().PutInt("int", 1) link.Attributes().PutDouble("double", 1.1) link.Attributes().PutEmptyBytes("bytes").FromRaw([]byte("foo")) + link.SetFlags(1) // Add another span. sp2 := il.Spans().AppendEmpty() sp2.SetName("testSpan2") return td }() -var tracesJSON = `{"resourceSpans":[{"resource":{"attributes":[{"key":"host.name","value":{"stringValue":"testHost"}},{"key":"service.name","value":{"stringValue":"testService"}}],"droppedAttributesCount":1},"scopeSpans":[{"scope":{"name":"scope name","version":"scope version"},"spans":[{"traceId":"0102030405060708090a0b0c0d0e0f10","spanId":"1112131415161718","traceState":"state","parentSpanId":"1112131415161718","name":"testSpan","kind":3,"startTimeUnixNano":"1684617382541971000","endTimeUnixNano":"1684623646539558000","attributes":[{"key":"string","value":{"stringValue":"value"}},{"key":"bool","value":{"boolValue":true}},{"key":"int","value":{"intValue":"1"}},{"key":"double","value":{"doubleValue":1.1}},{"key":"bytes","value":{"bytesValue":"Zm9v"}},{"key":"array","value":{"arrayValue":{"values":[{"intValue":"1"},{"stringValue":"str"}]}}},{"key":"kvList","value":{"kvlistValue":{"values":[{"key":"int","value":{"intValue":"1"}},{"key":"string","value":{"stringValue":"string"}}]}}}],"droppedAttributesCount":1,"events":[{"timeUnixNano":"1684620382541971000","name":"eventName","attributes":[{"key":"string","value":{"stringValue":"value"}},{"key":"bool","value":{"boolValue":true}},{"key":"int","value":{"intValue":"1"}},{"key":"double","value":{"doubleValue":1.1}},{"key":"bytes","value":{"bytesValue":"Zm9v"}}],"droppedAttributesCount":1}],"droppedEventsCount":1,"links":[{"traceId":"0102030405060708090a0b0c0d0e0f10","spanId":"1112131415161718","traceState":"state","attributes":[{"key":"string","value":{"stringValue":"value"}},{"key":"bool","value":{"boolValue":true}},{"key":"int","value":{"intValue":"1"}},{"key":"double","value":{"doubleValue":1.1}},{"key":"bytes","value":{"bytesValue":"Zm9v"}}],"droppedAttributesCount":1}],"droppedLinksCount":1,"status":{"message":"message","code":1}},{"traceId":"","spanId":"","parentSpanId":"","name":"testSpan2","status":{}}],"schemaUrl":"schemaURL"}],"schemaUrl":"schemaURL"}]}` +var tracesJSON = `{"resourceSpans":[{"resource":{"attributes":[{"key":"host.name","value":{"stringValue":"testHost"}},{"key":"service.name","value":{"stringValue":"testService"}}],"droppedAttributesCount":1},"scopeSpans":[{"scope":{"name":"scope name","version":"scope version"},"spans":[{"traceId":"0102030405060708090a0b0c0d0e0f10","spanId":"1112131415161718","traceState":"state","parentSpanId":"1112131415161718","flags":1,"name":"testSpan","kind":3,"startTimeUnixNano":"1684617382541971000","endTimeUnixNano":"1684623646539558000","attributes":[{"key":"string","value":{"stringValue":"value"}},{"key":"bool","value":{"boolValue":true}},{"key":"int","value":{"intValue":"1"}},{"key":"double","value":{"doubleValue":1.1}},{"key":"bytes","value":{"bytesValue":"Zm9v"}},{"key":"array","value":{"arrayValue":{"values":[{"intValue":"1"},{"stringValue":"str"}]}}},{"key":"kvList","value":{"kvlistValue":{"values":[{"key":"int","value":{"intValue":"1"}},{"key":"string","value":{"stringValue":"string"}}]}}}],"droppedAttributesCount":1,"events":[{"timeUnixNano":"1684620382541971000","name":"eventName","attributes":[{"key":"string","value":{"stringValue":"value"}},{"key":"bool","value":{"boolValue":true}},{"key":"int","value":{"intValue":"1"}},{"key":"double","value":{"doubleValue":1.1}},{"key":"bytes","value":{"bytesValue":"Zm9v"}}],"droppedAttributesCount":1}],"droppedEventsCount":1,"links":[{"traceId":"0102030405060708090a0b0c0d0e0f10","spanId":"1112131415161718","traceState":"state","attributes":[{"key":"string","value":{"stringValue":"value"}},{"key":"bool","value":{"boolValue":true}},{"key":"int","value":{"intValue":"1"}},{"key":"double","value":{"doubleValue":1.1}},{"key":"bytes","value":{"bytesValue":"Zm9v"}}],"droppedAttributesCount":1,"flags":1}],"droppedLinksCount":1,"status":{"message":"message","code":1}},{"traceId":"","spanId":"","parentSpanId":"","name":"testSpan2","status":{}}],"schemaUrl":"schemaURL"}],"schemaUrl":"schemaURL"}]}` func TestJSONUnmarshal(t *testing.T) { decoder := &JSONUnmarshaler{}