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 new file mode 100644 index 00000000000..056ea2a0453 --- /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 ( + sdkresource "go.opentelemetry.io/otel/sdk/resource" + + "go.opentelemetry.io/collector/service/telemetry" +) + +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 + } 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 51% rename from service/telemetry/attributes_test.go rename to service/attributes_test.go index c40a21f11f1..f876663e7ed 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,48 +9,57 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/service/internal/resource" + "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{}, - wantAttributes: map[string]interface{}{"service.name": "", "service.version": ""}, + cfg: telemetry.Config{}, + wantAttributes: map[string]interface{}{"service.name": "", "service.version": "", "service.instance.id": ""}, }, { 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"}, + wantAttributes: map[string]interface{}{"service.name": "otelcoltest", "service.version": "0.0.0-test", "service.instance.id": ""}, }, { 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")}}, - wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "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", "service.instance.id": ""}, }, { 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")}}, - wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "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", "service.instance.id": ""}, }, { 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")}}, - wantAttributes: map[string]interface{}{"service.version": "resource.version", "test": "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", "service.instance.id": ""}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - attrs := attributes(Settings{BuildInfo: 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 bf7e5265d0a..b5e9f8e2384 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(res, 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/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/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..6fbc155de79 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. @@ -66,13 +67,13 @@ 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(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..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(context.Background(), 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.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..1af064e6499 100644 --- a/service/telemetry/tracer_test.go +++ b/service/telemetry/tracer_test.go @@ -4,10 +4,10 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry" import ( - "context" "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/contrib/config" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/collector/config/configtelemetry" @@ -49,13 +49,13 @@ func TestNewTracerProvider(t *testing.T) { defer func() { require.NoError(t, featuregate.GlobalRegistry().Set(noopTracerProvider.ID(), previousValue)) }() - provider, err := newTracerProvider(context.TODO(), 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) }) } } - -func ptr[T any](v T) *T { - return &v -}