From 0b60ef884dc1f6f2dac4d66e4d17e9831ccd6e7d Mon Sep 17 00:00:00 2001 From: Antonio Jimenez Date: Thu, 7 Sep 2023 16:40:53 +0200 Subject: [PATCH] Apply feedback from @mx-psi . Create the sampledLogger the first time using it --- .chloggen/SampledLoggerTelemetry.yaml | 2 +- component/componenttest/nop_telemetry.go | 2 +- component/telemetry.go | 5 +++-- exporter/exporterhelper/common.go | 2 +- service/service_test.go | 4 ++-- service/telemetry/telemetry.go | 18 +++++++++++++----- 6 files changed, 21 insertions(+), 12 deletions(-) diff --git a/.chloggen/SampledLoggerTelemetry.yaml b/.chloggen/SampledLoggerTelemetry.yaml index 8cb9be08455..c10dbe40aa7 100644 --- a/.chloggen/SampledLoggerTelemetry.yaml +++ b/.chloggen/SampledLoggerTelemetry.yaml @@ -15,7 +15,7 @@ issues: [8134] # (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: The sampled logger is configured by `LogsSamplingConfig` (`service.telemetry.logs.sampling`). +subtext: The sampled logger is built from the logger. It is configured by `LogsSamplingConfig` (`service.telemetry.logs.sampling`). # Optional: The change log or logs in which this entry should be included. # e.g. '[user]' or '[user, api]' diff --git a/component/componenttest/nop_telemetry.go b/component/componenttest/nop_telemetry.go index 6adc2efc7d2..8df6639f1d9 100644 --- a/component/componenttest/nop_telemetry.go +++ b/component/componenttest/nop_telemetry.go @@ -17,7 +17,7 @@ import ( func NewNopTelemetrySettings() component.TelemetrySettings { return component.TelemetrySettings{ Logger: zap.NewNop(), - SampledLogger: zap.NewNop(), + SampledLogger: zap.NewNop, TracerProvider: trace.NewNoopTracerProvider(), MeterProvider: noop.NewMeterProvider(), MetricsLevel: configtelemetry.LevelNone, diff --git a/component/telemetry.go b/component/telemetry.go index e76fb50919c..96c7e606a15 100644 --- a/component/telemetry.go +++ b/component/telemetry.go @@ -17,9 +17,10 @@ type TelemetrySettings struct { // component to be used later as well. Logger *zap.Logger - // SampledLogger passed to the created component. + // SampledLogger is built from the logger. It is passed to the created component. // It will be used to avoid flooding the logs with messages that are repeated frequently. - SampledLogger *zap.Logger + // It will be built the first time used. + SampledLogger func() *zap.Logger // TracerProvider that the factory can pass to other instrumented third-party libraries. TracerProvider trace.TracerProvider diff --git a/exporter/exporterhelper/common.go b/exporter/exporterhelper/common.go index 1d41b1f8ef0..ac3ac90684e 100644 --- a/exporter/exporterhelper/common.go +++ b/exporter/exporterhelper/common.go @@ -171,7 +171,7 @@ func newBaseExporter(set exporter.CreateSettings, bs *baseSettings, signal compo return nil, err } - be.qrSender = newQueuedRetrySender(set.ID, signal, bs.queue, bs.RetrySettings, &timeoutSender{cfg: bs.TimeoutSettings}, set.SampledLogger) + be.qrSender = newQueuedRetrySender(set.ID, signal, bs.queue, bs.RetrySettings, &timeoutSender{cfg: bs.TimeoutSettings}, set.SampledLogger()) be.sender = be.qrSender be.StartFunc = func(ctx context.Context, host component.Host) error { // First start the wrapped exporter. diff --git a/service/service_test.go b/service/service_test.go index d4b3ff5e608..bc023fa353b 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -412,8 +412,8 @@ func TestServiceTelemetryLoggers(t *testing.T) { assert.NoError(t, srv.Shutdown(context.Background())) }) assert.NotNil(t, srv.telemetrySettings.Logger) - assert.NotNil(t, srv.telemetrySettings.SampledLogger) - assert.NotEqual(t, srv.telemetrySettings.Logger, srv.telemetrySettings.SampledLogger) + assert.NotNil(t, srv.telemetrySettings.SampledLogger()) + assert.NotEqual(t, srv.telemetrySettings.Logger, srv.telemetrySettings.SampledLogger()) } func assertResourceLabels(t *testing.T, res pcommon.Resource, expectedLabels map[string]labelValue) { diff --git a/service/telemetry/telemetry.go b/service/telemetry/telemetry.go index 9bae0e518f1..3fbe1f72574 100644 --- a/service/telemetry/telemetry.go +++ b/service/telemetry/telemetry.go @@ -5,6 +5,7 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry" import ( "context" + "sync" "time" sdktrace "go.opentelemetry.io/otel/sdk/trace" @@ -16,8 +17,11 @@ import ( type Telemetry struct { logger *zap.Logger - sampledLogger *zap.Logger tracerProvider *sdktrace.TracerProvider + + createSampledLogger sync.Once + sampledLogger *zap.Logger + samplingCfg *LogsSamplingConfig } func (t *Telemetry) TracerProvider() trace.TracerProvider { @@ -28,8 +32,13 @@ func (t *Telemetry) Logger() *zap.Logger { return t.logger } -func (t *Telemetry) SampledLogger() *zap.Logger { - return t.sampledLogger +func (t *Telemetry) SampledLogger() func() *zap.Logger { + return func() *zap.Logger { + t.createSampledLogger.Do(func() { + t.sampledLogger = newSampledLogger(t.samplingCfg, t.logger) + }) + return t.sampledLogger + } } func (t *Telemetry) Shutdown(ctx context.Context) error { @@ -50,7 +59,6 @@ func New(_ context.Context, set Settings, cfg Config) (*Telemetry, error) { if err != nil { return nil, err } - sampledLogger := newSampledLogger(cfg.Logs.Sampling, logger) tp := sdktrace.NewTracerProvider( // needed for supporting the zpages extension @@ -58,8 +66,8 @@ func New(_ context.Context, set Settings, cfg Config) (*Telemetry, error) { ) return &Telemetry{ logger: logger, - sampledLogger: sampledLogger, tracerProvider: tp, + samplingCfg: cfg.Logs.Sampling, }, nil }