Update default sampling logger configuration in the telemetry configu…

…ration

.chloggen/SampledLoggerTelemetry.yaml

@@ -7,15 +7,15 @@ change_type: enhancement
 component: service/telemetry exporter/exporterhelper
 
 # A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
-note: "New sampled logger in the telemetry configuration used to avoid flooding the logs with messages that are repeated frequently."
+note: "Update default sampling logger configuration in the telemetry configuration."
 
 # One or more tracking issues or pull requests related to the change
 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 built from the logger. It is configured to sample 100 logs in a second after 10 initial logs.
+subtext: The sampled configuration can be disabled easily by setting the 'enabled' field to 'false'
 
 
 # Optional: The change log or logs in which this entry should be included.

component/componenttest/nop_telemetry.go

@@ -17,7 +17,6 @@ import (
 func NewNopTelemetrySettings() component.TelemetrySettings {
 	return component.TelemetrySettings{
 		Logger:         zap.NewNop(),
-		SampledLogger:  zap.NewNop,
 		TracerProvider: trace.NewNoopTracerProvider(),
 		MeterProvider:  noop.NewMeterProvider(),
 		MetricsLevel:   configtelemetry.LevelNone,

component/telemetry.go

@@ -17,11 +17,6 @@ type TelemetrySettings struct {
 	// component to be used later as well.
 	Logger *zap.Logger
 
-	// 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.
-	// 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
 

exporter/exporterhelper/common.go

@@ -7,8 +7,6 @@ import (
 	"context"
 	"time"
 
-	"go.uber.org/zap"
-
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/consumer"
 	"go.opentelemetry.io/collector/exporter"
@@ -114,7 +112,7 @@ func WithTimeout(timeoutSettings TimeoutSettings) Option {
 // The default RetrySettings is to disable retries.
 func WithRetry(retrySettings RetrySettings) Option {
 	return func(o *baseExporter) {
-		o.retrySender = newRetrySender(o.set.ID, retrySettings, o.sampledLogger, o.onTemporaryFailure)
+		o.retrySender = newRetrySender(o.set.ID, retrySettings, o.set.Logger, o.onTemporaryFailure)
 	}
 }
 
@@ -134,7 +132,7 @@ func WithQueue(config QueueSettings) Option {
 				queue = internal.NewPersistentQueue(config.QueueSize, config.NumConsumers, *config.StorageID, o.marshaler, o.unmarshaler)
 			}
 		}
-		qs := newQueueSender(o.set.ID, o.signal, queue, o.sampledLogger)
+		qs := newQueueSender(o.set.ID, o.signal, queue, o.set.Logger)
 		o.queueSender = qs
 		o.setOnTemporaryFailure(qs.onTemporaryFailure)
 	}
@@ -159,9 +157,8 @@ type baseExporter struct {
 	unmarshaler     internal.RequestUnmarshaler
 	signal          component.DataType
 
-	set           exporter.CreateSettings
-	obsrep        *obsExporter
-	sampledLogger *zap.Logger
+	set    exporter.CreateSettings
+	obsrep *obsExporter
 
 	// Chain of senders that the exporter helper applies before passing the data to the actual exporter.
 	// The data is handled by each sender in the respective order starting from the queueSender.
@@ -197,9 +194,8 @@ func newBaseExporter(set exporter.CreateSettings, signal component.DataType, req
 		retrySender:   &baseRequestSender{},
 		timeoutSender: &timeoutSender{cfg: NewDefaultTimeoutSettings()},
 
-		set:           set,
-		obsrep:        obsrep,
-		sampledLogger: set.SampledLogger(),
+		set:    set,
+		obsrep: obsrep,
 	}
 
 	for _, op := range options {

otelcol/unmarshaler.go

@@ -4,6 +4,8 @@
 package otelcol // import "go.opentelemetry.io/collector/otelcol"
 
 import (
+	"time"
+
 	"go.uber.org/zap/zapcore"
 
 	"go.opentelemetry.io/collector/config/configtelemetry"
@@ -45,7 +47,9 @@ func unmarshal(v *confmap.Conf, factories Factories) (*configSettings, error) {
 					Development: false,
 					Encoding:    "console",
 					Sampling: &telemetry.LogsSamplingConfig{
-						Initial:    100,
+						Enabled:    true,
+						Tick:       1 * time.Second,
+						Initial:    10,
 						Thereafter: 100,
 					},
 					OutputPaths:       []string{"stderr"},

otelcol/unmarshaler_test.go

@@ -5,6 +5,7 @@ package otelcol
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -45,7 +46,9 @@ func TestUnmarshalEmptyAllSections(t *testing.T) {
 		Development: zapProdCfg.Development,
 		Encoding:    "console",
 		Sampling: &telemetry.LogsSamplingConfig{
-			Initial:    100,
+			Enabled:    true,
+			Tick:       1 * time.Second,
+			Initial:    10,
 			Thereafter: 100,
 		},
 		DisableCaller:     zapProdCfg.DisableCaller,

service/service.go

@@ -106,7 +106,6 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) {
 
 	srv.telemetrySettings = component.TelemetrySettings{
 		Logger:         srv.telemetry.Logger(),
-		SampledLogger:  srv.telemetry.SampledLogger(),
 		TracerProvider: srv.telemetry.TracerProvider(),
 		MeterProvider:  noop.NewMeterProvider(),
 		MetricsLevel:   cfg.Telemetry.Metrics.Level,

service/service_test.go

@@ -403,7 +403,7 @@ func TestNilCollectorEffectiveConfig(t *testing.T) {
 	require.NoError(t, srv.Shutdown(context.Background()))
 }
 
-func TestServiceTelemetryLoggers(t *testing.T) {
+func TestServiceTelemetryLogger(t *testing.T) {
 	srv, err := New(context.Background(), newNopSettings(), newNopConfig())
 	require.NoError(t, err)
 
@@ -412,8 +412,6 @@ 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())
 }
 
 func assertResourceLabels(t *testing.T, res pcommon.Resource, expectedLabels map[string]labelValue) {

service/telemetry/config.go

@@ -5,6 +5,7 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry"
 
 import (
 	"fmt"
+	"time"
 
 	"go.uber.org/zap/zapcore"
 
@@ -54,7 +55,14 @@ type LogsConfig struct {
 	// (default = false)
 	DisableStacktrace bool `mapstructure:"disable_stacktrace"`
 
-	// Sampling sets a sampling policy. A nil SamplingConfig disables sampling.
+	// Sampling sets a sampling policy.
+	// Default:
+	// 		sampling:
+	//	   		enabled: true
+	//	   		tick: 1s
+	//	   		initial: 10
+	//	   		thereafter: 100
+	// Sampling can be disabled by setting 'enabled' to false
 	Sampling *LogsSamplingConfig `mapstructure:"sampling"`
 
 	// OutputPaths is a list of URLs or file paths to write logging output to.
@@ -91,8 +99,10 @@ type LogsConfig struct {
 // global CPU and I/O load that logging puts on your process while attempting
 // to preserve a representative subset of your logs.
 type LogsSamplingConfig struct {
-	Initial    int `mapstructure:"initial"`
-	Thereafter int `mapstructure:"thereafter"`
+	Enabled    bool          `mapstructure:"enabled"`
+	Tick       time.Duration `mapstructure:"tick"`
+	Initial    int           `mapstructure:"initial"`
+	Thereafter int           `mapstructure:"thereafter"`
 }
 
 // MetricsConfig exposes the common Telemetry configuration for one component.

service/telemetry/telemetry.go

@@ -5,8 +5,6 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry"
 
 import (
 	"context"
-	"sync"
-	"time"
 
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 	"go.opentelemetry.io/otel/trace"
@@ -18,9 +16,6 @@ import (
 type Telemetry struct {
 	logger         *zap.Logger
 	tracerProvider *sdktrace.TracerProvider
-
-	createSampledLogger sync.Once
-	sampledLogger       *zap.Logger
 }
 
 func (t *Telemetry) TracerProvider() trace.TracerProvider {
@@ -31,15 +26,6 @@ func (t *Telemetry) Logger() *zap.Logger {
 	return t.logger
 }
 
-func (t *Telemetry) SampledLogger() func() *zap.Logger {
-	return func() *zap.Logger {
-		t.createSampledLogger.Do(func() {
-			t.sampledLogger = newSampledLogger(t.logger)
-		})
-		return t.sampledLogger
-	}
-}
-
 func (t *Telemetry) Shutdown(ctx context.Context) error {
 	// TODO: Sync logger.
 	return multierr.Combine(
@@ -73,7 +59,6 @@ func newLogger(cfg LogsConfig, options []zap.Option) (*zap.Logger, error) {
 	zapCfg := &zap.Config{
 		Level:             zap.NewAtomicLevelAt(cfg.Level),
 		Development:       cfg.Development,
-		Sampling:          toSamplingConfig(cfg.Sampling),
 		Encoding:          cfg.Encoding,
 		EncoderConfig:     zap.NewProductionEncoderConfig(),
 		OutputPaths:       cfg.OutputPaths,
@@ -92,29 +77,22 @@ func newLogger(cfg LogsConfig, options []zap.Option) (*zap.Logger, error) {
 	if err != nil {
 		return nil, err
 	}
+	if cfg.Sampling != nil && cfg.Sampling.Enabled {
+		logger = newSampledLogger(logger, cfg.Sampling)
+	}
 
 	return logger, nil
 }
 
-func toSamplingConfig(sc *LogsSamplingConfig) *zap.SamplingConfig {
-	if sc == nil {
-		return nil
-	}
-	return &zap.SamplingConfig{
-		Initial:    sc.Initial,
-		Thereafter: sc.Thereafter,
-	}
-}
-
-func newSampledLogger(logger *zap.Logger) *zap.Logger {
-	// Create a logger that samples all messages to 10 per second initially,
-	// and 10/100 of messages after that.
+func newSampledLogger(logger *zap.Logger, sc *LogsSamplingConfig) *zap.Logger {
+	// Create a logger that samples all messages to sc.Tick per second initially,
+	// and sc.Initial/sc.Thereafter of messages after that.
 	opts := zap.WrapCore(func(core zapcore.Core) zapcore.Core {
 		return zapcore.NewSamplerWithOptions(
 			core,
-			1*time.Second,
-			10,
-			100,
+			sc.Tick,
+			sc.Initial,
+			sc.Thereafter,
 		)
 	})
 	return logger.WithOptions(opts)

service/telemetry/telemetry_test.go

@@ -6,6 +6,7 @@ package telemetry
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"go.uber.org/zap"
@@ -63,7 +64,7 @@ func TestTelemetryConfiguration(t *testing.T) {
 	}
 }
 
-func TestSampledLoggerCreateFirstTime(t *testing.T) {
+func TestSampledLogger(t *testing.T) {
 	tests := []struct {
 		name string
 		cfg  *Config
@@ -72,29 +73,34 @@ func TestSampledLoggerCreateFirstTime(t *testing.T) {
 			name: "Default sampling",
 			cfg: &Config{
 				Logs: LogsConfig{
-					Level:    zapcore.DebugLevel,
 					Encoding: "console",
 				},
-				Metrics: MetricsConfig{
-					Level:   configtelemetry.LevelBasic,
-					Address: "127.0.0.1:3333",
-				},
 			},
 		},
 		{
-			name: "Already using sampling",
+			name: "Custom sampling",
 			cfg: &Config{
 				Logs: LogsConfig{
 					Level:    zapcore.DebugLevel,
 					Encoding: "console",
 					Sampling: &LogsSamplingConfig{
-						Initial:    50,
-						Thereafter: 40,
+						Enabled:    true,
+						Tick:       1 * time.Second,
+						Initial:    100,
+						Thereafter: 100,
 					},
 				},
-				Metrics: MetricsConfig{
-					Level:   configtelemetry.LevelBasic,
-					Address: "127.0.0.1:3333",
+			},
+		},
+		{
+			name: "Disable sampling",
+			cfg: &Config{
+				Logs: LogsConfig{
+					Level:    zapcore.DebugLevel,
+					Encoding: "console",
+					Sampling: &LogsSamplingConfig{
+						Enabled: false,
+					},
 				},
 			},
 		},
@@ -105,10 +111,7 @@ func TestSampledLoggerCreateFirstTime(t *testing.T) {
 			telemetry, err := New(context.Background(), Settings{ZapOptions: []zap.Option{}}, *tt.cfg)
 			assert.NoError(t, err)
 			assert.NotNil(t, telemetry)
-			assert.Nil(t, telemetry.sampledLogger)
-			getSampledLogger := telemetry.SampledLogger()
-			assert.NotNil(t, getSampledLogger())
-			assert.Equal(t, getSampledLogger(), telemetry.sampledLogger)
+			assert.NotNil(t, telemetry.Logger())
 		})
 	}
 }