Skip to content

Commit d537824

Browse files
authored
Merge branch 'main' into mx-psi/configoptional-for-exporterhelper-queuebatchconfig
2 parents 24bcec8 + fa20a0c commit d537824

File tree

13 files changed

+239
-43
lines changed

13 files changed

+239
-43
lines changed

.chloggen/14140.yaml

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

confmap/internal/decoder.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ func mapKeyStringToMapKeyTextUnmarshalerHookFunc() mapstructure.DecodeHookFuncTy
168168
}
169169

170170
// Create a map with key value of to's key to bool.
171-
fieldNameSet := reflect.MakeMap(reflect.MapOf(to.Key(), reflect.TypeOf(true)))
171+
fieldNameSet := reflect.MakeMap(reflect.MapOf(to.Key(), reflect.TypeFor[bool]()))
172172
for k := range data.(map[string]any) {
173173
// Create a new value of the to's key type.
174174
tKey := reflect.New(to.Key())

confmap/xconfmap/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515

1616
// As interface types are only used for static typing, a common idiom to find the reflection Type
1717
// for an interface type Foo is to use a *Foo value.
18-
var configValidatorType = reflect.TypeOf((*Validator)(nil)).Elem()
18+
var configValidatorType = reflect.TypeFor[Validator]()
1919

2020
// Validator defines an optional interface for configurations to implement to do validation.
2121
type Validator interface {

service/service.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -124,16 +124,23 @@ func New(ctx context.Context, set Settings, cfg Config) (_ *Service, resultErr e
124124
collectorConf: set.CollectorConf,
125125
}
126126

127-
// Create the logger & LoggerProvider first. These may be used
128-
// when creating the other telemetry providers.
127+
if set.TelemetryFactory == nil {
128+
return nil, errors.New("telemetry factory not provided")
129+
}
130+
131+
// Create the resource first. This ensures all telemetry providers
132+
// (logger, meter, tracer) use the same resource with a consistent service.instance.id.
129133
telemetrySettings := telemetry.Settings{BuildInfo: set.BuildInfo}
134+
resource, err := set.TelemetryFactory.CreateResource(ctx, telemetrySettings, cfg.Telemetry)
135+
if err != nil {
136+
return nil, fmt.Errorf("failed to create resource: %w", err)
137+
}
138+
telemetrySettings.Resource = &resource
139+
130140
loggerSettings := telemetry.LoggerSettings{
131141
Settings: telemetrySettings,
132142
ZapOptions: set.LoggingOptions,
133143
}
134-
if set.TelemetryFactory == nil {
135-
return nil, errors.New("telemetry factory not provided")
136-
}
137144
logger, loggerShutdownFunc, err := set.TelemetryFactory.CreateLogger(ctx, loggerSettings, cfg.Telemetry)
138145
if err != nil {
139146
return nil, fmt.Errorf("failed to create logger: %w", err)
@@ -177,11 +184,6 @@ func New(ctx context.Context, set Settings, cfg Config) (_ *Service, resultErr e
177184
}()
178185
srv.tracerProvider = tracerProvider
179186

180-
resource, err := set.TelemetryFactory.CreateResource(ctx, telemetrySettings, cfg.Telemetry)
181-
if err != nil {
182-
return nil, fmt.Errorf("failed to create resource: %w", err)
183-
}
184-
185187
srv.telemetrySettings = component.TelemetrySettings{
186188
Logger: logger,
187189
MeterProvider: meterProvider,

service/service_test.go

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -495,15 +495,15 @@ func TestServiceTelemetryCreateProvidersError(t *testing.T) {
495495
}
496496
for name, tc := range map[string]testcase{
497497
"CreateLogger": {
498-
opts: []telemetry.FactoryOption{loggerOpt, meterOpt, tracerOpt, resourceOpt},
498+
opts: []telemetry.FactoryOption{loggerOpt, meterOpt, tracerOpt},
499499
expectedErr: "failed to create logger: something went wrong",
500500
},
501501
"CreateMeterProvider": {
502-
opts: []telemetry.FactoryOption{meterOpt, tracerOpt, resourceOpt},
502+
opts: []telemetry.FactoryOption{meterOpt, tracerOpt},
503503
expectedErr: "failed to create meter provider: something went wrong",
504504
},
505505
"CreateTracerProvider": {
506-
opts: []telemetry.FactoryOption{tracerOpt, resourceOpt},
506+
opts: []telemetry.FactoryOption{tracerOpt},
507507
expectedErr: "failed to create tracer provider: something went wrong",
508508
},
509509
"CreateResource": {
@@ -527,6 +527,81 @@ func TestNew_NilTelemetryProvider(t *testing.T) {
527527
require.EqualError(t, err, "telemetry factory not provided")
528528
}
529529

530+
func TestServiceTelemetryConsistentInstanceID(t *testing.T) {
531+
var loggerResource, meterResource, tracerResource *pcommon.Resource
532+
533+
createLoggerCalled := false
534+
createMeterCalled := false
535+
createTracerCalled := false
536+
537+
baseFactory := otelconftelemetry.NewFactory()
538+
539+
set := newNopSettings()
540+
set.TelemetryFactory = telemetry.NewFactory(
541+
baseFactory.CreateDefaultConfig,
542+
telemetry.WithCreateResource(baseFactory.CreateResource),
543+
telemetry.WithCreateLogger(func(ctx context.Context, settings telemetry.LoggerSettings, cfg component.Config) (*zap.Logger, component.ShutdownFunc, error) {
544+
createLoggerCalled = true
545+
loggerResource = settings.Resource
546+
return baseFactory.CreateLogger(ctx, settings, cfg)
547+
}),
548+
telemetry.WithCreateMeterProvider(func(ctx context.Context, settings telemetry.MeterSettings, cfg component.Config) (telemetry.MeterProvider, error) {
549+
createMeterCalled = true
550+
meterResource = settings.Resource
551+
return baseFactory.CreateMeterProvider(ctx, settings, cfg)
552+
}),
553+
telemetry.WithCreateTracerProvider(func(ctx context.Context, settings telemetry.TracerSettings, cfg component.Config) (telemetry.TracerProvider, error) {
554+
createTracerCalled = true
555+
tracerResource = settings.Resource
556+
return baseFactory.CreateTracerProvider(ctx, settings, cfg)
557+
}),
558+
)
559+
560+
cfg := newNopConfig()
561+
srv, err := New(context.Background(), set, cfg)
562+
require.NoError(t, err)
563+
564+
require.True(t, createLoggerCalled, "logger should have been created")
565+
require.True(t, createMeterCalled, "meter provider should have been created")
566+
require.True(t, createTracerCalled, "tracer provider should have been created")
567+
568+
var serviceInstanceID string
569+
if sid, ok := srv.telemetrySettings.Resource.Attributes().Get("service.instance.id"); ok {
570+
serviceInstanceID = sid.AsString()
571+
}
572+
require.NotEmpty(t, serviceInstanceID, "service.instance.id not found in service resource")
573+
574+
require.NotNil(t, loggerResource, "logger should have received a resource")
575+
require.NotNil(t, meterResource, "meter provider should have received a resource")
576+
require.NotNil(t, tracerResource, "tracer provider should have received a resource")
577+
578+
var loggerInstanceID, meterInstanceID, tracerInstanceID string
579+
if sid, ok := loggerResource.Attributes().Get("service.instance.id"); ok {
580+
loggerInstanceID = sid.AsString()
581+
}
582+
if sid, ok := meterResource.Attributes().Get("service.instance.id"); ok {
583+
meterInstanceID = sid.AsString()
584+
}
585+
if sid, ok := tracerResource.Attributes().Get("service.instance.id"); ok {
586+
tracerInstanceID = sid.AsString()
587+
}
588+
589+
require.NotEmpty(t, loggerInstanceID, "logger resource should have service.instance.id")
590+
require.NotEmpty(t, meterInstanceID, "meter resource should have service.instance.id")
591+
require.NotEmpty(t, tracerInstanceID, "tracer resource should have service.instance.id")
592+
593+
assert.Equal(t, serviceInstanceID, loggerInstanceID,
594+
"logger should use the same service.instance.id as the service resource")
595+
assert.Equal(t, serviceInstanceID, meterInstanceID,
596+
"meter provider should use the same service.instance.id as the service resource")
597+
assert.Equal(t, serviceInstanceID, tracerInstanceID,
598+
"tracer provider should use the same service.instance.id as the service resource")
599+
600+
t.Logf("service.instance.id = %s (shared by logger, meter, and tracer)", serviceInstanceID)
601+
602+
require.NoError(t, srv.Shutdown(context.Background()))
603+
}
604+
530605
func newNopSettings() Settings {
531606
receiversConfigs, receiversFactories := builders.NewNopReceiverConfigsAndFactories()
532607
processorsConfigs, processorsFactories := builders.NewNopProcessorConfigsAndFactories()
@@ -582,7 +657,7 @@ func newNopConfigPipelineConfigs(pipelineCfgs pipelines.Config) Config {
582657
return Config{
583658
Extensions: extensions.Config{component.NewID(nopType)},
584659
Pipelines: pipelineCfgs,
585-
Telemetry: otelconftelemetry.Config{
660+
Telemetry: &otelconftelemetry.Config{
586661
Logs: otelconftelemetry.LogsConfig{
587662
Level: zapcore.InfoLevel,
588663
Development: false,

service/telemetry/otelconftelemetry/logger.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88

99
otelconf "go.opentelemetry.io/contrib/otelconf/v0.3.0"
10+
sdkresource "go.opentelemetry.io/otel/sdk/resource"
1011
"go.uber.org/zap"
1112
"go.uber.org/zap/zapcore"
1213

@@ -21,7 +22,8 @@ func createLogger(
2122
componentConfig component.Config,
2223
) (*zap.Logger, component.ShutdownFunc, error) {
2324
cfg := componentConfig.(*Config)
24-
res := newResource(set.Settings, cfg)
25+
attrs := pcommonAttrsToOTelAttrs(set.Resource)
26+
res := sdkresource.NewWithAttributes("", attrs...)
2527

2628
// Copied from NewProductionConfig.
2729
ec := zap.NewProductionEncoderConfig()

service/telemetry/otelconftelemetry/logger_test.go

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,12 @@ func TestCreateLogger(t *testing.T) {
133133
for _, tt := range tests {
134134
t.Run(tt.name, func(t *testing.T) {
135135
buildInfo := component.BuildInfo{}
136-
_, provider, err := NewFactory().CreateLogger(context.Background(), telemetry.LoggerSettings{
137-
Settings: telemetry.Settings{BuildInfo: buildInfo},
136+
factory := NewFactory()
137+
resource, err := factory.CreateResource(context.Background(), telemetry.Settings{BuildInfo: buildInfo}, &tt.cfg)
138+
require.NoError(t, err)
139+
140+
_, provider, err := factory.CreateLogger(context.Background(), telemetry.LoggerSettings{
141+
Settings: telemetry.Settings{BuildInfo: buildInfo, Resource: &resource},
138142
}, &tt.cfg)
139143
if tt.wantErr != nil {
140144
require.ErrorContains(t, err, tt.wantErr.Error())
@@ -226,13 +230,6 @@ func TestCreateLoggerWithResource(t *testing.T) {
226230
for _, tt := range tests {
227231
t.Run(tt.name, func(t *testing.T) {
228232
core, observedLogs := observer.New(zapcore.DebugLevel)
229-
set := telemetry.LoggerSettings{
230-
Settings: telemetry.Settings{BuildInfo: tt.buildInfo},
231-
ZapOptions: []zap.Option{
232-
// Redirect logs to the observer core
233-
zap.WrapCore(func(zapcore.Core) zapcore.Core { return core }),
234-
},
235-
}
236233
cfg := &Config{
237234
Logs: LogsConfig{
238235
Level: zapcore.InfoLevel,
@@ -241,6 +238,17 @@ func TestCreateLoggerWithResource(t *testing.T) {
241238
Resource: tt.resourceConfig,
242239
}
243240

241+
resource, err := createResource(t.Context(), telemetry.Settings{BuildInfo: tt.buildInfo}, cfg)
242+
require.NoError(t, err)
243+
244+
set := telemetry.LoggerSettings{
245+
Settings: telemetry.Settings{BuildInfo: tt.buildInfo, Resource: &resource},
246+
ZapOptions: []zap.Option{
247+
// Redirect logs to the observer core
248+
zap.WrapCore(func(zapcore.Core) zapcore.Core { return core }),
249+
},
250+
}
251+
244252
logger, loggerProvider, err := createLogger(t.Context(), set, cfg)
245253
require.NoError(t, err)
246254
defer func() {
@@ -337,7 +345,12 @@ func newOTLPLogger(t *testing.T, level zapcore.Level, handler func(plogotlp.Expo
337345
},
338346
}
339347

340-
logger, shutdown, err := createLogger(t.Context(), telemetry.LoggerSettings{}, cfg)
348+
resource, err := createResource(t.Context(), telemetry.Settings{}, cfg)
349+
require.NoError(t, err)
350+
351+
logger, shutdown, err := createLogger(t.Context(), telemetry.LoggerSettings{
352+
Settings: telemetry.Settings{Resource: &resource},
353+
}, cfg)
341354
require.NoError(t, err)
342355
t.Cleanup(func() {
343356
assert.NoError(t, shutdown.Shutdown(context.WithoutCancel(t.Context())))
@@ -371,12 +384,6 @@ func TestLogAttributeInjection(t *testing.T) {
371384
})
372385
t.Cleanup(srv.Close)
373386

374-
set := telemetry.LoggerSettings{
375-
ZapOptions: []zap.Option{
376-
// Redirect console logs to the observer core
377-
zap.WrapCore(func(zapcore.Core) zapcore.Core { return core }),
378-
},
379-
}
380387
cfg := &Config{
381388
Resource: map[string]*string{
382389
"service.instance.id": nil,
@@ -400,6 +407,17 @@ func TestLogAttributeInjection(t *testing.T) {
400407
},
401408
}
402409

410+
resource, err := createResource(t.Context(), telemetry.Settings{}, cfg)
411+
require.NoError(t, err)
412+
413+
set := telemetry.LoggerSettings{
414+
Settings: telemetry.Settings{Resource: &resource},
415+
ZapOptions: []zap.Option{
416+
// Redirect console logs to the observer core
417+
zap.WrapCore(func(zapcore.Core) zapcore.Core { return core }),
418+
},
419+
}
420+
403421
sourceLogger, loggerProvider, err := createLogger(t.Context(), set, cfg)
404422
require.NoError(t, err)
405423
defer func() {

service/telemetry/otelconftelemetry/metrics.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
config "go.opentelemetry.io/contrib/otelconf/v0.3.0"
1010
noopmetric "go.opentelemetry.io/otel/metric/noop"
11+
sdkresource "go.opentelemetry.io/otel/sdk/resource"
1112

1213
"go.opentelemetry.io/collector/component"
1314
"go.opentelemetry.io/collector/config/configtelemetry"
@@ -27,7 +28,8 @@ func createMeterProvider(
2728
cfg.Metrics.Views = set.DefaultViews(cfg.Metrics.Level)
2829
}
2930

30-
res := newResource(set.Settings, cfg)
31+
attrs := pcommonAttrsToOTelAttrs(set.Resource)
32+
res := sdkresource.NewWithAttributes("", attrs...)
3133
mpConfig := cfg.Metrics.MeterProvider
3234
sdk, err := newSDK(ctx, res, config.OpenTelemetryConfiguration{
3335
MeterProvider: &mpConfig,

service/telemetry/otelconftelemetry/metrics_test.go

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,12 @@ func TestCreateMeterProvider(t *testing.T) {
119119
}
120120

121121
t.Run(tt.name, func(t *testing.T) {
122-
mp, err := createMeterProvider(t.Context(), telemetry.MeterSettings{}, cfg)
122+
resource, err := createResource(t.Context(), telemetry.Settings{}, cfg)
123+
require.NoError(t, err)
124+
125+
mp, err := createMeterProvider(t.Context(), telemetry.MeterSettings{
126+
Settings: telemetry.Settings{Resource: &resource},
127+
}, cfg)
123128
require.NoError(t, err)
124129
defer func() {
125130
assert.NoError(t, mp.Shutdown(t.Context()))
@@ -209,7 +214,12 @@ func TestCreateMeterProvider_Invalid(t *testing.T) {
209214
// Invalid -- no OTLP protocol defined
210215
Periodic: &config.PeriodicMetricReader{Exporter: config.PushMetricExporter{OTLP: &config.OTLPMetric{}}},
211216
}}
212-
_, err := createMeterProvider(t.Context(), telemetry.MeterSettings{}, cfg)
217+
resource, err := createResource(t.Context(), telemetry.Settings{}, cfg)
218+
require.NoError(t, err)
219+
220+
_, err = createMeterProvider(t.Context(), telemetry.MeterSettings{
221+
Settings: telemetry.Settings{Resource: &resource},
222+
}, cfg)
213223
require.EqualError(t, err, "no valid metric exporter")
214224
}
215225

@@ -221,17 +231,27 @@ func TestCreateMeterProvider_Disabled(t *testing.T) {
221231
}}
222232

223233
core, observedLogs := observer.New(zapcore.DebugLevel)
224-
settings := telemetry.MeterSettings{}
225-
settings.Logger = zap.New(core)
226234

227235
factory := NewFactory()
228-
_, err := factory.CreateMeterProvider(context.Background(), settings, cfg)
236+
resource, err := factory.CreateResource(context.Background(), telemetry.Settings{}, cfg)
237+
require.NoError(t, err)
238+
239+
settings := telemetry.MeterSettings{
240+
Settings: telemetry.Settings{Resource: &resource},
241+
}
242+
settings.Logger = zap.New(core)
243+
244+
_, err = factory.CreateMeterProvider(context.Background(), settings, cfg)
229245
require.EqualError(t, err, "no valid metric exporter")
230246
assert.Zero(t, observedLogs.Len())
231247

232248
// Setting Metrics.Level to LevelNone disables metrics,
233249
// so the invalid configuration should not cause an error.
234250
cfg.Metrics.Level = configtelemetry.LevelNone
251+
resource2, err := createResource(t.Context(), telemetry.Settings{}, cfg)
252+
require.NoError(t, err)
253+
settings.Resource = &resource2
254+
235255
mp, err := createMeterProvider(t.Context(), settings, cfg)
236256
require.NoError(t, err)
237257
assert.NoError(t, mp.Shutdown(t.Context()))
@@ -249,7 +269,12 @@ func TestInstrumentEnabled(t *testing.T) {
249269
Pull: &config.PullMetricReader{Exporter: config.PullMetricExporter{Prometheus: prom}},
250270
}}
251271

252-
meterProvider, err := createMeterProvider(t.Context(), telemetry.MeterSettings{}, cfg)
272+
resource, err := createResource(t.Context(), telemetry.Settings{}, cfg)
273+
require.NoError(t, err)
274+
275+
meterProvider, err := createMeterProvider(t.Context(), telemetry.MeterSettings{
276+
Settings: telemetry.Settings{Resource: &resource},
277+
}, cfg)
253278
require.NoError(t, err)
254279
defer func() {
255280
assert.NoError(t, meterProvider.Shutdown(t.Context()))

0 commit comments

Comments
 (0)