Skip to content

Commit

Permalink
Apply feedback from @mx-psi . Create the sampledLogger the first time…
Browse files Browse the repository at this point in the history
… using it
  • Loading branch information
antonjim-te committed Sep 7, 2023
1 parent 20536a1 commit 0b60ef8
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .chloggen/SampledLoggerTelemetry.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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]'
Expand Down
2 changes: 1 addition & 1 deletion component/componenttest/nop_telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions component/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion exporter/exporterhelper/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
18 changes: 13 additions & 5 deletions service/telemetry/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry"

import (
"context"
"sync"
"time"

sdktrace "go.opentelemetry.io/otel/sdk/trace"
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -50,16 +59,15 @@ 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
sdktrace.WithSampler(alwaysRecord()),
)
return &Telemetry{
logger: logger,
sampledLogger: sampledLogger,
tracerProvider: tp,
samplingCfg: cfg.Logs.Sampling,
}, nil
}

Expand Down

0 comments on commit 0b60ef8

Please sign in to comment.