Apply @dmitryax feedback. Keep Sampling config in general logger

Author: antonjim-te

.chloggen/SampledLoggerTelemetry.yaml

@@ -15,7 +15,8 @@ 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 by `LogsSamplingConfig` (`service.telemetry.logs.sampling`).
+subtext: The sampled logger is built from the logger. It is built using default config (sample 100 logs in a second after 10 initial logs)
+
 
 # Optional: The change log or logs in which this entry should be included.
 # e.g. '[user]' or '[user, api]'

service/telemetry/config.go

@@ -5,7 +5,6 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry"
 
 import (
 	"fmt"
-	"time"
 
 	"go.uber.org/zap/zapcore"
 
@@ -55,11 +54,7 @@ type LogsConfig struct {
 	// (default = false)
 	DisableStacktrace bool `mapstructure:"disable_stacktrace"`
 
-	// Sampling sets a sampling policy for the more efficient sampled logger.
-	// Default:
-	//     initial: 1
-	//     thereafter: 100
-	//     tick: 10s
+	// Sampling sets a sampling policy. A nil SamplingConfig disables sampling.
 	Sampling *LogsSamplingConfig `mapstructure:"sampling"`
 
 	// OutputPaths is a list of URLs or file paths to write logging output to.
@@ -92,13 +87,12 @@ type LogsConfig struct {
 	InitialFields map[string]any `mapstructure:"initial_fields"`
 }
 
-// LogsSamplingConfig sets a sampling strategy for the more efficient sampled logger. Sampling caps the
+// LogsSamplingConfig sets a sampling strategy for the logger. Sampling caps the
 // 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"`
-	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

@@ -21,7 +21,6 @@ type Telemetry struct {
 
 	createSampledLogger sync.Once
 	sampledLogger       *zap.Logger
-	samplingCfg         *LogsSamplingConfig
 }
 
 func (t *Telemetry) TracerProvider() trace.TracerProvider {
@@ -35,7 +34,7 @@ func (t *Telemetry) Logger() *zap.Logger {
 func (t *Telemetry) SampledLogger() func() *zap.Logger {
 	return func() *zap.Logger {
 		t.createSampledLogger.Do(func() {
-			t.sampledLogger = newSampledLogger(t.samplingCfg, t.logger)
+			t.sampledLogger = newSampledLogger(t.logger)
 		})
 		return t.sampledLogger
 	}
@@ -67,7 +66,6 @@ func New(_ context.Context, set Settings, cfg Config) (*Telemetry, error) {
 	return &Telemetry{
 		logger:         logger,
 		tracerProvider: tp,
-		samplingCfg:    cfg.Logs.Sampling,
 	}, nil
 }
 
@@ -76,6 +74,7 @@ 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,
@@ -98,29 +97,26 @@ func newLogger(cfg LogsConfig, options []zap.Option) (*zap.Logger, error) {
 	return logger, nil
 }
 
-func newSampledLogger(cfg *LogsSamplingConfig, logger *zap.Logger) *zap.Logger {
-	if cfg == nil {
-		cfg = newDefaultLogsSamplingConfig()
+func toSamplingConfig(sc *LogsSamplingConfig) *zap.SamplingConfig {
+	if sc == nil {
+		return nil
 	}
+	return &zap.SamplingConfig{
+		Initial:    sc.Initial,
+		Thereafter: sc.Thereafter,
+	}
+}
 
-	// Create a logger that samples all messages to "initial" per "tick" initially,
-	// and cfg.Initial/cfg.Thereafter of messages after that.
+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.
 	opts := zap.WrapCore(func(core zapcore.Core) zapcore.Core {
 		return zapcore.NewSamplerWithOptions(
 			core,
-			cfg.Tick,
-			cfg.Initial,
-			cfg.Thereafter,
+			1*time.Second,
+			10,
+			100,
 		)
 	})
 	return logger.WithOptions(opts)
 }
-
-// newDefaultLogsSamplingConfig returns a default LogsSamplingConfig.
-func newDefaultLogsSamplingConfig() *LogsSamplingConfig {
-	return &LogsSamplingConfig{
-		Initial:    1,
-		Thereafter: 100,
-		Tick:       10 * time.Second,
-	}
-}

service/telemetry/telemetry_test.go

@@ -6,7 +6,6 @@ package telemetry
 import (
 	"context"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"go.uber.org/zap"
@@ -83,14 +82,13 @@ func TestSampledLoggerCreateFirstTime(t *testing.T) {
 			},
 		},
 		{
-			name: "Custom sampling",
+			name: "Already using sampling",
 			cfg: &Config{
 				Logs: LogsConfig{
 					Level:    zapcore.DebugLevel,
 					Encoding: "console",
 					Sampling: &LogsSamplingConfig{
 						Initial:    50,
-						Tick:       2 * time.Second,
 						Thereafter: 40,
 					},
 				},