From 98e1c7e35329b5440b6a780271db5c8c47a61361 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:10:19 -0700 Subject: [PATCH 1/3] [service] fix bug with inconsistent resources The tracer and logger provider were instantiating different resources objects that didn't have a `service.instance.id` attribute. This change fixes that by instantiating the SDK in the service and passing it to the factory via the telemetry.Settings struct. This also removes the duplicate code to instantiate the SDK multiple times, which will be useful when we move to instantiating MeterProvider via the config SDK. This bug is blocking the change to bump up the dependency on the config package. There are a few alternatives that were considered: 1. could set the resource's service.instance.id on the config object instead. this seemed messy as it would be editing the config struct and the instantiation of the SDK would remain duplicated. 2. update the factory to pass in a resource object option, i didn't want to update the NewFactory func. 3. update the CreateLogger, CreateTracerProvider to receive either a resource or sdk parameter, both of those seemed incorrect as well. Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- service/attributes.go | 27 ++++++++++++ service/{telemetry => }/attributes_test.go | 17 ++++---- service/service.go | 28 ++++++++++++ service/telemetry/attributes.go | 22 ---------- service/telemetry/factory.go | 7 +-- service/telemetry/logger.go | 31 ++------------ service/telemetry/logger_test.go | 2 +- service/telemetry/tracer.go | 50 +++------------------- service/telemetry/tracer_test.go | 3 +- 9 files changed, 78 insertions(+), 109 deletions(-) create mode 100644 service/attributes.go rename service/{telemetry => }/attributes_test.go (66%) delete mode 100644 service/telemetry/attributes.go diff --git a/service/attributes.go b/service/attributes.go new file mode 100644 index 00000000000..28adb3c1632 --- /dev/null +++ b/service/attributes.go @@ -0,0 +1,27 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package service // import "go.opentelemetry.io/collector/service" + +import ( + semconv "go.opentelemetry.io/otel/semconv/v1.26.0" + + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/service/telemetry" +) + +func attributes(buildInfo component.BuildInfo, cfg telemetry.Config) map[string]interface{} { + attrs := map[string]interface{}{ + string(semconv.ServiceNameKey): buildInfo.Command, + string(semconv.ServiceVersionKey): buildInfo.Version, + } + for k, v := range cfg.Resource { + if v != nil { + attrs[k] = *v + } else { + // the new value is nil, delete the existing key + delete(attrs, k) + } + } + return attrs +} diff --git a/service/telemetry/attributes_test.go b/service/attributes_test.go similarity index 66% rename from service/telemetry/attributes_test.go rename to service/attributes_test.go index c40a21f11f1..af70f61e9cc 100644 --- a/service/telemetry/attributes_test.go +++ b/service/attributes_test.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package telemetry // import "go.opentelemetry.io/collector/service/telemetry" +package service // import "go.opentelemetry.io/collector/service" import ( "testing" @@ -9,47 +9,48 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/service/telemetry" ) func TestAttributes(t *testing.T) { tests := []struct { name string - cfg Config + cfg telemetry.Config buildInfo component.BuildInfo wantAttributes map[string]interface{} }{ { name: "no build info and no resource config", - cfg: Config{}, + cfg: telemetry.Config{}, wantAttributes: map[string]interface{}{"service.name": "", "service.version": ""}, }, { name: "build info and no resource config", - cfg: Config{}, + cfg: telemetry.Config{}, buildInfo: component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"}, wantAttributes: map[string]interface{}{"service.name": "otelcoltest", "service.version": "0.0.0-test"}, }, { name: "no build info and resource config", - cfg: Config{Resource: map[string]*string{"service.name": ptr("resource.name"), "service.version": ptr("resource.version"), "test": ptr("test")}}, + cfg: telemetry.Config{Resource: map[string]*string{"service.name": newPtr("resource.name"), "service.version": newPtr("resource.version"), "test": newPtr("test")}}, wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test"}, }, { name: "build info and resource config", buildInfo: component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"}, - cfg: Config{Resource: map[string]*string{"service.name": ptr("resource.name"), "service.version": ptr("resource.version"), "test": ptr("test")}}, + cfg: telemetry.Config{Resource: map[string]*string{"service.name": newPtr("resource.name"), "service.version": newPtr("resource.version"), "test": newPtr("test")}}, wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test"}, }, { name: "deleting a nil value", buildInfo: component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"}, - cfg: Config{Resource: map[string]*string{"service.name": nil, "service.version": ptr("resource.version"), "test": ptr("test")}}, + cfg: telemetry.Config{Resource: map[string]*string{"service.name": nil, "service.version": newPtr("resource.version"), "test": newPtr("test")}}, wantAttributes: map[string]interface{}{"service.version": "resource.version", "test": "test"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - attrs := attributes(Settings{BuildInfo: tt.buildInfo}, tt.cfg) + attrs := attributes(tt.buildInfo, tt.cfg) require.Equal(t, tt.wantAttributes, attrs) }) } diff --git a/service/service.go b/service/service.go index bf7e5265d0a..3b530e28a7d 100644 --- a/service/service.go +++ b/service/service.go @@ -11,6 +11,7 @@ import ( "fmt" "runtime" + "go.opentelemetry.io/contrib/config" "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" @@ -28,6 +29,7 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/processor" "go.opentelemetry.io/collector/receiver" + semconv "go.opentelemetry.io/collector/semconv/v1.26.0" "go.opentelemetry.io/collector/service/extensions" "go.opentelemetry.io/collector/service/internal/builders" "go.opentelemetry.io/collector/service/internal/graph" @@ -118,10 +120,36 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { res := resource.New(set.BuildInfo, cfg.Telemetry.Resource) pcommonRes := pdataFromSdk(res) + sch := semconv.SchemaURL + cfgRes := config.Resource{ + SchemaUrl: &sch, + Attributes: attributes(set.BuildInfo, cfg.Telemetry), + } + + sdk, err := config.NewSDK( + config.WithContext(ctx), + config.WithOpenTelemetryConfiguration( + config.OpenTelemetryConfiguration{ + LoggerProvider: &config.LoggerProvider{ + Processors: cfg.Telemetry.Logs.Processors, + }, + TracerProvider: &config.TracerProvider{ + Processors: cfg.Telemetry.Traces.Processors, + }, + Resource: &cfgRes, + }, + ), + ) + + if err != nil { + return nil, fmt.Errorf("failed to create SDK: %w", err) + } + telFactory := telemetry.NewFactory() telset := telemetry.Settings{ BuildInfo: set.BuildInfo, ZapOptions: set.LoggingOptions, + SDK: &sdk, } logger, lp, err := telFactory.CreateLogger(ctx, telset, &cfg.Telemetry) diff --git a/service/telemetry/attributes.go b/service/telemetry/attributes.go deleted file mode 100644 index 60b23a34591..00000000000 --- a/service/telemetry/attributes.go +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package telemetry // import "go.opentelemetry.io/collector/service/telemetry" - -import semconv "go.opentelemetry.io/otel/semconv/v1.4.0" - -func attributes(set Settings, cfg Config) map[string]interface{} { - attrs := map[string]interface{}{ - string(semconv.ServiceNameKey): set.BuildInfo.Command, - string(semconv.ServiceVersionKey): set.BuildInfo.Version, - } - for k, v := range cfg.Resource { - if v != nil { - attrs[k] = *v - } else { - // the new value is nil, delete the existing key - delete(attrs, k) - } - } - return attrs -} diff --git a/service/telemetry/factory.go b/service/telemetry/factory.go index fde93bba2c2..5437c88bf9e 100644 --- a/service/telemetry/factory.go +++ b/service/telemetry/factory.go @@ -40,6 +40,7 @@ type Settings struct { BuildInfo component.BuildInfo AsyncErrorChannel chan error ZapOptions []zap.Option + SDK *config.SDK } // Factory is factory interface for telemetry. @@ -68,11 +69,11 @@ func NewFactory() Factory { return newFactory(createDefaultConfig, withLogger(func(ctx context.Context, set Settings, cfg component.Config) (*zap.Logger, log.LoggerProvider, error) { c := *cfg.(*Config) - return newLogger(ctx, set, c) + return newLogger(set, c) }), - withTracerProvider(func(ctx context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) { + withTracerProvider(func(_ context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) { c := *cfg.(*Config) - return newTracerProvider(ctx, set, c) + return newTracerProvider(set, c) }), withMeterProvider(func(_ context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) { c := *cfg.(*Config) diff --git a/service/telemetry/logger.go b/service/telemetry/logger.go index a57e3f9cd09..388d8833e8a 100644 --- a/service/telemetry/logger.go +++ b/service/telemetry/logger.go @@ -4,18 +4,14 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry" import ( - "context" - "go.opentelemetry.io/contrib/bridges/otelzap" - "go.opentelemetry.io/contrib/config" "go.opentelemetry.io/otel/log" - semconv "go.opentelemetry.io/otel/semconv/v1.26.0" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) // newLogger creates a Logger and a LoggerProvider from Config. -func newLogger(ctx context.Context, set Settings, cfg Config) (*zap.Logger, log.LoggerProvider, error) { +func newLogger(set Settings, cfg Config) (*zap.Logger, log.LoggerProvider, error) { // Copied from NewProductionConfig. zapCfg := &zap.Config{ Level: zap.NewAtomicLevelAt(cfg.Logs.Level), @@ -42,29 +38,8 @@ func newLogger(ctx context.Context, set Settings, cfg Config) (*zap.Logger, log. var lp log.LoggerProvider - if len(cfg.Logs.Processors) > 0 { - sch := semconv.SchemaURL - res := config.Resource{ - SchemaUrl: &sch, - Attributes: attributes(set, cfg), - } - sdk, err := config.NewSDK( - config.WithContext(ctx), - config.WithOpenTelemetryConfiguration( - config.OpenTelemetryConfiguration{ - LoggerProvider: &config.LoggerProvider{ - Processors: cfg.Logs.Processors, - }, - Resource: &res, - }, - ), - ) - - if err != nil { - return nil, nil, err - } - - lp = sdk.LoggerProvider() + if len(cfg.Logs.Processors) > 0 && set.SDK != nil { + lp = set.SDK.LoggerProvider() logger = logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core { return zapcore.NewTee( diff --git a/service/telemetry/logger_test.go b/service/telemetry/logger_test.go index de901d5142c..631ba5e9dfe 100644 --- a/service/telemetry/logger_test.go +++ b/service/telemetry/logger_test.go @@ -85,7 +85,7 @@ func TestNewLogger(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - l, lp, err := newLogger(context.Background(), Settings{}, tt.cfg) + l, lp, err := newLogger(Settings{}, tt.cfg) if tt.wantErr != nil { require.ErrorContains(t, err, tt.wantErr.Error()) require.Nil(t, tt.wantCoreType) diff --git a/service/telemetry/tracer.go b/service/telemetry/tracer.go index d7b9131d51b..9febba8e29f 100644 --- a/service/telemetry/tracer.go +++ b/service/telemetry/tracer.go @@ -7,11 +7,9 @@ import ( "context" "errors" - "go.opentelemetry.io/contrib/config" "go.opentelemetry.io/contrib/propagators/b3" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/propagation" - semconv "go.opentelemetry.io/otel/semconv/v1.26.0" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/embedded" "go.opentelemetry.io/otel/trace/noop" @@ -55,59 +53,21 @@ func (n *noopNoContextTracerProvider) Tracer(_ string, _ ...trace.TracerOption) } // newTracerProvider creates a new TracerProvider from Config. -func newTracerProvider(ctx context.Context, set Settings, cfg Config) (trace.TracerProvider, error) { +func newTracerProvider(set Settings, cfg Config) (trace.TracerProvider, error) { if noopTracerProvider.IsEnabled() || cfg.Traces.Level == configtelemetry.LevelNone { return &noopNoContextTracerProvider{}, nil } - sch := semconv.SchemaURL - res := config.Resource{ - SchemaUrl: &sch, - Attributes: attributes(set, cfg), - } - - sdk, err := config.NewSDK( - config.WithContext(ctx), - config.WithOpenTelemetryConfiguration( - config.OpenTelemetryConfiguration{ - Resource: &res, - TracerProvider: &config.TracerProvider{ - Processors: cfg.Traces.Processors, - // TODO: once https://github.com/open-telemetry/opentelemetry-configuration/issues/83 is resolved, - // configuration for sampler should be done here via something like the following: - // - // Sampler: &config.Sampler{ - // ParentBased: &config.SamplerParentBased{ - // LocalParentSampled: &config.Sampler{ - // AlwaysOn: config.SamplerAlwaysOn{}, - // }, - // LocalParentNotSampled: &config.Sampler{ - // RecordOnly: config.SamplerRecordOnly{}, - // }, - // RemoteParentSampled: &config.Sampler{ - // AlwaysOn: config.SamplerAlwaysOn{}, - // }, - // RemoteParentNotSampled: &config.Sampler{ - // RecordOnly: config.SamplerRecordOnly{}, - // }, - // }, - // }, - }, - }, - ), - ) - - if err != nil { - return nil, err - } - if tp, err := textMapPropagatorFromConfig(cfg.Traces.Propagators); err == nil { otel.SetTextMapPropagator(tp) } else { return nil, err } - return sdk.TracerProvider(), nil + if set.SDK != nil { + return set.SDK.TracerProvider(), nil + } + return nil, errors.New("no sdk set") } func textMapPropagatorFromConfig(props []string) (propagation.TextMapPropagator, error) { diff --git a/service/telemetry/tracer_test.go b/service/telemetry/tracer_test.go index 62b022af597..51350aea771 100644 --- a/service/telemetry/tracer_test.go +++ b/service/telemetry/tracer_test.go @@ -4,7 +4,6 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry" import ( - "context" "testing" "github.com/stretchr/testify/require" @@ -49,7 +48,7 @@ func TestNewTracerProvider(t *testing.T) { defer func() { require.NoError(t, featuregate.GlobalRegistry().Set(noopTracerProvider.ID(), previousValue)) }() - provider, err := newTracerProvider(context.TODO(), Settings{}, tt.cfg) + provider, err := newTracerProvider(Settings{}, tt.cfg) require.NoError(t, err) require.IsType(t, tt.wantTracerProvider, provider) }) From 49f44d218a2146b2464fecb64349670b29d879c2 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Wed, 30 Oct 2024 14:35:46 -0700 Subject: [PATCH 2/3] fix tests Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .../codeboten_fix-resource-descrepancy.yaml | 25 ++++++++++++ service/attributes.go | 12 +++--- service/attributes_test.go | 22 ++++++---- service/service.go | 2 +- service/service_test.go | 40 +++++++++++++++++++ service/telemetry/logger_test.go | 24 +++-------- service/telemetry/tracer_test.go | 7 +++- 7 files changed, 98 insertions(+), 34 deletions(-) create mode 100644 .chloggen/codeboten_fix-resource-descrepancy.yaml diff --git a/.chloggen/codeboten_fix-resource-descrepancy.yaml b/.chloggen/codeboten_fix-resource-descrepancy.yaml new file mode 100644 index 00000000000..815fbba9c4a --- /dev/null +++ b/.chloggen/codeboten_fix-resource-descrepancy.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: service + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: ensure traces and logs emitted by the otel go SDK use the same resource information + +# One or more tracking issues or pull requests related to the change +issues: [11578] + +# (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/service/attributes.go b/service/attributes.go index 28adb3c1632..056ea2a0453 100644 --- a/service/attributes.go +++ b/service/attributes.go @@ -4,17 +4,17 @@ package service // import "go.opentelemetry.io/collector/service" import ( - semconv "go.opentelemetry.io/otel/semconv/v1.26.0" + sdkresource "go.opentelemetry.io/otel/sdk/resource" - "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/service/telemetry" ) -func attributes(buildInfo component.BuildInfo, cfg telemetry.Config) map[string]interface{} { - attrs := map[string]interface{}{ - string(semconv.ServiceNameKey): buildInfo.Command, - string(semconv.ServiceVersionKey): buildInfo.Version, +func attributes(res *sdkresource.Resource, cfg telemetry.Config) map[string]interface{} { + attrs := map[string]interface{}{} + for _, r := range res.Attributes() { + attrs[string(r.Key)] = r.Value.AsString() } + for k, v := range cfg.Resource { if v != nil { attrs[k] = *v diff --git a/service/attributes_test.go b/service/attributes_test.go index af70f61e9cc..f876663e7ed 100644 --- a/service/attributes_test.go +++ b/service/attributes_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/service/internal/resource" "go.opentelemetry.io/collector/service/telemetry" ) @@ -22,36 +23,43 @@ func TestAttributes(t *testing.T) { { name: "no build info and no resource config", cfg: telemetry.Config{}, - wantAttributes: map[string]interface{}{"service.name": "", "service.version": ""}, + wantAttributes: map[string]interface{}{"service.name": "", "service.version": "", "service.instance.id": ""}, }, { name: "build info and no resource config", cfg: telemetry.Config{}, buildInfo: component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"}, - wantAttributes: map[string]interface{}{"service.name": "otelcoltest", "service.version": "0.0.0-test"}, + wantAttributes: map[string]interface{}{"service.name": "otelcoltest", "service.version": "0.0.0-test", "service.instance.id": ""}, }, { name: "no build info and resource config", cfg: telemetry.Config{Resource: map[string]*string{"service.name": newPtr("resource.name"), "service.version": newPtr("resource.version"), "test": newPtr("test")}}, - wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test"}, + wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test", "service.instance.id": ""}, }, { name: "build info and resource config", buildInfo: component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"}, cfg: telemetry.Config{Resource: map[string]*string{"service.name": newPtr("resource.name"), "service.version": newPtr("resource.version"), "test": newPtr("test")}}, - wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test"}, + wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test", "service.instance.id": ""}, }, { name: "deleting a nil value", buildInfo: component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"}, cfg: telemetry.Config{Resource: map[string]*string{"service.name": nil, "service.version": newPtr("resource.version"), "test": newPtr("test")}}, - wantAttributes: map[string]interface{}{"service.version": "resource.version", "test": "test"}, + wantAttributes: map[string]interface{}{"service.version": "resource.version", "test": "test", "service.instance.id": ""}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - attrs := attributes(tt.buildInfo, tt.cfg) - require.Equal(t, tt.wantAttributes, attrs) + attrs := attributes(resource.New(tt.buildInfo, tt.cfg.Resource), tt.cfg) + require.Len(t, attrs, len(tt.wantAttributes)) + for k, v := range tt.wantAttributes { + if k == "service.instance.id" { + require.NotNil(t, attrs[k]) + } else { + require.Equal(t, v, attrs[k]) + } + } }) } } diff --git a/service/service.go b/service/service.go index 3b530e28a7d..b5e9f8e2384 100644 --- a/service/service.go +++ b/service/service.go @@ -123,7 +123,7 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { sch := semconv.SchemaURL cfgRes := config.Resource{ SchemaUrl: &sch, - Attributes: attributes(set.BuildInfo, cfg.Telemetry), + Attributes: attributes(res, cfg.Telemetry), } sdk, err := config.NewSDK( diff --git a/service/service_test.go b/service/service_test.go index d1663a8212e..1b7b1eca704 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -469,6 +469,46 @@ func TestServiceFatalError(t *testing.T) { require.ErrorIs(t, err, assert.AnError) } +func TestServiceInvalidTelemetryConfiguration(t *testing.T) { + tests := []struct { + name string + wantErr error + cfg telemetry.Config + }{ + { + name: "log config with processors and invalid config", + cfg: telemetry.Config{ + Logs: telemetry.LogsConfig{ + Encoding: "console", + Processors: []config.LogRecordProcessor{ + { + Batch: &config.BatchLogRecordProcessor{ + Exporter: config.LogRecordExporter{ + OTLP: &config.OTLP{}, + }, + }, + }, + }, + }, + }, + wantErr: errors.New("unsupported protocol \"\""), + }, + } + for _, tt := range tests { + set := newNopSettings() + set.AsyncErrorChannel = make(chan error) + + cfg := newNopConfig() + cfg.Telemetry = tt.cfg + _, err := New(context.Background(), set, cfg) + if tt.wantErr != nil { + require.ErrorContains(t, err, tt.wantErr.Error()) + } else { + require.NoError(t, err) + } + } +} + func assertResourceLabels(t *testing.T, res pcommon.Resource, expectedLabels map[string]labelValue) { for key, labelValue := range expectedLabels { lookupKey, ok := prometheusToOtelConv[key] diff --git a/service/telemetry/logger_test.go b/service/telemetry/logger_test.go index 631ba5e9dfe..bc73b2de12b 100644 --- a/service/telemetry/logger_test.go +++ b/service/telemetry/logger_test.go @@ -41,24 +41,6 @@ func TestNewLogger(t *testing.T) { }, wantCoreType: "*zapcore.ioCore", }, - { - name: "log config with processors and invalid config", - cfg: Config{ - Logs: LogsConfig{ - Encoding: "console", - Processors: []config.LogRecordProcessor{ - { - Batch: &config.BatchLogRecordProcessor{ - Exporter: config.LogRecordExporter{ - OTLP: &config.OTLP{}, - }, - }, - }, - }, - }, - }, - wantErr: errors.New("unsupported protocol \"\""), - }, { name: "log config with processors", cfg: Config{ @@ -85,7 +67,11 @@ func TestNewLogger(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - l, lp, err := newLogger(Settings{}, tt.cfg) + sdk, _ := config.NewSDK(config.WithOpenTelemetryConfiguration(config.OpenTelemetryConfiguration{LoggerProvider: &config.LoggerProvider{ + Processors: tt.cfg.Logs.Processors, + }})) + + l, lp, err := newLogger(Settings{SDK: &sdk}, tt.cfg) if tt.wantErr != nil { require.ErrorContains(t, err, tt.wantErr.Error()) require.Nil(t, tt.wantCoreType) diff --git a/service/telemetry/tracer_test.go b/service/telemetry/tracer_test.go index 51350aea771..52610690c75 100644 --- a/service/telemetry/tracer_test.go +++ b/service/telemetry/tracer_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/contrib/config" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/collector/config/configtelemetry" @@ -48,7 +49,11 @@ func TestNewTracerProvider(t *testing.T) { defer func() { require.NoError(t, featuregate.GlobalRegistry().Set(noopTracerProvider.ID(), previousValue)) }() - provider, err := newTracerProvider(Settings{}, tt.cfg) + sdk, err := config.NewSDK(config.WithOpenTelemetryConfiguration(config.OpenTelemetryConfiguration{TracerProvider: &config.TracerProvider{ + Processors: tt.cfg.Traces.Processors, + }})) + require.NoError(t, err) + provider, err := newTracerProvider(Settings{SDK: &sdk}, tt.cfg) require.NoError(t, err) require.IsType(t, tt.wantTracerProvider, provider) }) From 7bd38416a786de3c49e539958111d44e712de739 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Wed, 30 Oct 2024 15:26:27 -0700 Subject: [PATCH 3/3] lint fixes Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- service/telemetry/factory.go | 2 +- service/telemetry/tracer_test.go | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/service/telemetry/factory.go b/service/telemetry/factory.go index 5437c88bf9e..6fbc155de79 100644 --- a/service/telemetry/factory.go +++ b/service/telemetry/factory.go @@ -67,7 +67,7 @@ type Factory interface { // NewFactory creates a new Factory. func NewFactory() Factory { return newFactory(createDefaultConfig, - withLogger(func(ctx context.Context, set Settings, cfg component.Config) (*zap.Logger, log.LoggerProvider, error) { + withLogger(func(_ context.Context, set Settings, cfg component.Config) (*zap.Logger, log.LoggerProvider, error) { c := *cfg.(*Config) return newLogger(set, c) }), diff --git a/service/telemetry/tracer_test.go b/service/telemetry/tracer_test.go index 52610690c75..1af064e6499 100644 --- a/service/telemetry/tracer_test.go +++ b/service/telemetry/tracer_test.go @@ -59,7 +59,3 @@ func TestNewTracerProvider(t *testing.T) { }) } } - -func ptr[T any](v T) *T { - return &v -}