Refactoring

 - Better name for some functions/variables
 - Allowing the typedLogger to have any output, its the caller's
   responsibility to restric output usage
 - Moved `typedLogger` to its own file
 - Updated tests accoringly

logp/configure/logging.go

@@ -81,8 +81,7 @@ func LoggingWithOutputs(beatName string, cfg, typedCfg *config.C, outputs ...zap
 
 // LoggingWithTypedOutputs applies some defaults then calls ConfigureWithTypedOutputs
 //
-// At the moment only the sensitive output is supported. It will log any entry that sets
-// `log.type: sensitive` to a different file.
+// TODO (Tiago): Do we even need this?
 func LoggingWithTypedOutputs(beatName string, cfg, typedCfg *config.C, logKey, kind string, outputs ...zapcore.Core) error {
 	config := logp.DefaultConfig(environment)
 	config.Beat = beatName
@@ -101,7 +100,7 @@ func LoggingWithTypedOutputs(beatName string, cfg, typedCfg *config.C, logKey, k
 		return fmt.Errorf("cannot unpack sensitiveCfg: %w", err)
 	}
 
-	return logp.ConfigureWithTypedOutputs(config, typedLogpConfig, "log.type", "sensitive", outputs...)
+	return logp.ConfigureWithTypedOutput(config, typedLogpConfig, logKey, kind, outputs...)
 }
 
 func applyFlags(cfg *logp.Config) {

logp/core.go

@@ -139,7 +139,7 @@ func ConfigureWithOutputs(defaultLoggerCfg Config, outputs ...zapcore.Core) erro
 	return nil
 }
 
-// ConfigureWithTypedOutputs configures the global logger to use typed outputs.
+// ConfigureWithTypedOutput configures the global logger to use typed outputs.
 //
 // If a log entry matches the defined key/value, this entry is logged using the
 // core generated from `typedLoggerCfg`, otherwise it will be logged by all
@@ -153,15 +153,15 @@ func ConfigureWithOutputs(defaultLoggerCfg Config, outputs ...zapcore.Core) erro
 //   - `value` is the value compared against the `logKey` entry
 //   - `outputs` is a list of cores that will be added together with the core
 //     generated by `defaultLoggerCfg` as the default output for the loggger.
-func ConfigureWithTypedOutputs(defaultLoggerCfg, typedLoggerCfg Config, key, value string, outputs ...zapcore.Core) error {
+func ConfigureWithTypedOutput(defaultLoggerCfg, typedLoggerCfg Config, key, value string, outputs ...zapcore.Core) error {
 	sink, level, observedLogs, selectors, err := createSink(defaultLoggerCfg, outputs...)
 	if err != nil {
 		return err
 	}
 
-	typedCore, err := makeFileOutput(typedLoggerCfg, level)
+	typedCore, err := createLogOutput(typedLoggerCfg, level)
 	if err != nil {
-		return fmt.Errorf("could not create sensitive file output: %w", err)
+		return fmt.Errorf("could not create typed logger output: %w", err)
 	}
 
 	sink = &typedLoggerCore{
@@ -289,34 +289,6 @@ func makeEventLogOutput(cfg Config, enab zapcore.LevelEnabler) (zapcore.Core, er
 	return wrappedCore(core), nil
 }
 
-// WithFileOrStderrOutput configures the logger with an output based on cfg.
-// If neither `cfg.ToFiles` nor `cfg.ToSdterr` are true, then a noop output
-// is created. If both are true, the file output has preference.
-func WithFileOrStderrOutput(cfg Config) func(zapcore.Core) zapcore.Core {
-	var out zapcore.Core
-	var err error
-
-	switch {
-	case cfg.ToFiles:
-		out, err = makeFileOutput(cfg, cfg.Level.ZapLevel())
-	case cfg.ToStderr:
-		out, err = makeStderrOutput(cfg, cfg.Level.ZapLevel())
-	default:
-		out = zapcore.NewNopCore()
-		err = errors.New("unexpected output type, only allowed outputs are 'file' and 'stderr'")
-	}
-
-	if err != nil {
-		L().Errorf("could not create log output: %s", err)
-	}
-
-	f := func(zapcore.Core) zapcore.Core {
-		return out
-	}
-
-	return f
-}
-
 func makeFileOutput(cfg Config, enab zapcore.LevelEnabler) (zapcore.Core, error) {
 	filename := paths.Resolve(paths.Logs, filepath.Join(cfg.Files.Path, cfg.LogFilename()))
 
@@ -424,59 +396,3 @@ func (m multiCore) Sync() error {
 	}
 	return errors.Join(errs...)
 }
-
-// typedLoggerCore takes two cores and directs logs entries to one of them
-// with the value of the field defined by the pair `key` and `value`
-//
-// If `entry[key] == value` the typedCore is used, otherwise the
-// defaultCore  is used.
-// WARNING: The level of both cores must always be the same!
-// typedLoggerCore will only use the defaultCore level to decide
-// whether to log an entry or not
-type typedLoggerCore struct {
-	typedCore   zapcore.Core
-	defaultCore zapcore.Core
-	value       string
-	key         string
-}
-
-func (t *typedLoggerCore) Enabled(l zapcore.Level) bool {
-	return t.defaultCore.Enabled(l)
-}
-
-func (t *typedLoggerCore) With(fields []zapcore.Field) zapcore.Core {
-	t.defaultCore = t.defaultCore.With(fields)
-	t.typedCore = t.typedCore.With(fields)
-	return t
-}
-
-func (t *typedLoggerCore) Check(e zapcore.Entry, ce *zapcore.CheckedEntry) *zapcore.CheckedEntry {
-	if t.defaultCore.Enabled(e.Level) {
-		return ce.AddCore(e, t)
-	}
-
-	return ce
-}
-
-func (t *typedLoggerCore) Sync() error {
-	defaultErr := t.defaultCore.Sync()
-	typedErr := t.typedCore.Sync()
-
-	if defaultErr != nil || typedErr != nil {
-		return fmt.Errorf("error syncing logger. DefaultCore: '%w', typedCore: '%w'", defaultErr, typedErr)
-	}
-
-	return nil
-}
-
-func (t *typedLoggerCore) Write(e zapcore.Entry, fields []zapcore.Field) error {
-	for _, f := range fields {
-		if f.Key == t.key {
-			if f.String == t.value {
-				return t.typedCore.Write(e, fields)
-			}
-		}
-	}
-
-	return t.defaultCore.Write(e, fields)
-}

logp/core_test.go

@@ -250,7 +250,7 @@ func TestCreatingNewLoggerWithDifferentOutput(t *testing.T) {
 	}
 
 	expectedLogMessage := "this is a log message"
-	expectedLogLogger := t.Name() + "-second"
+	expectedLogLoggerName := t.Name() + "-second"
 
 	// We follow the same approach as on a Beat, first the logger
 	// (always global) is configured and used, then we instantiate
@@ -283,16 +283,26 @@ func TestCreatingNewLoggerWithDifferentOutput(t *testing.T) {
 	secondCfg.Files.Name = "test-log-file"
 	secondCfg.Files.Path = tempDir2
 
+	// Create a new output for the second logger using the same level
+	// as the global logger
+	out, err := createLogOutput(secondCfg, loggerCfg.Level.ZapLevel())
+	if err != nil {
+		t.Fatalf("could not create output for second config")
+	}
+	outCore := func(zapcore.Core) zapcore.Core { return out }
+
 	// We do not call Configure here as we do not want to affect
 	// the global logger configuration
 	secondLogger := NewLogger(t.Name() + "-second")
-	secondLogger = secondLogger.WithOptions(zap.WrapCore(WithFileOrStderrOutput(secondCfg)))
+	secondLogger = secondLogger.WithOptions(zap.WrapCore(outCore))
 	secondLogger.Info(expectedLogMessage)
 	if err := secondLogger.Sync(); err != nil {
 		t.Fatalf("could not sync log file from second logger: %s", err)
 	}
 
-	// Writes again with the first logger to ensure it has not been affected
+	// TODO: Refactor it into a function and use to assert both loggers worked as expected
+
+	// Write again with the first logger to ensure it has not been affected
 	// by the new configuration on the second logger.
 	logger.Info("not the message we want")
 	if err := logger.Sync(); err != nil {
@@ -328,60 +338,14 @@ func TestCreatingNewLoggerWithDifferentOutput(t *testing.T) {
 	}
 
 	// Ensure a couple of fields exist
-	if logEntry["log.logger"] != expectedLogLogger {
-		t.Fatalf("expecting 'log.logger' to be '%s', got '%s' instead", expectedLogLogger, logEntry["log.logger"])
+	if logEntry["log.logger"] != expectedLogLoggerName {
+		t.Fatalf("expecting 'log.logger' to be '%s', got '%s' instead", expectedLogLoggerName, logEntry["log.logger"])
 	}
 	if logEntry["message"] != expectedLogMessage {
 		t.Fatalf("expecting 'message' to be '%s, got '%s' instead", expectedLogMessage, logEntry["message"])
 	}
 }
 
-func TestWithFileOrStderrOutput(t *testing.T) {
-	testCases := []struct {
-		name     string
-		toStderr bool
-		toFile   bool
-	}{
-		{
-			name:     "stderr output",
-			toStderr: true,
-			toFile:   false,
-		},
-		{
-			name:     "file output",
-			toStderr: false,
-			toFile:   true,
-		},
-	}
-
-	notEnabledLevels := []zapcore.Level{zapcore.InfoLevel, zapcore.DebugLevel}
-	enabledLevels := []zapcore.Level{zapcore.ErrorLevel, zapcore.PanicLevel}
-
-	for _, tc := range testCases {
-		cfg := DefaultConfig(DefaultEnvironment)
-		cfg.ToStderr = tc.toStderr
-		cfg.ToFiles = tc.toFile
-		cfg.Level = ErrorLevel
-
-		f := WithFileOrStderrOutput(cfg)
-		core := f(zapcore.NewNopCore())
-
-		t.Run(tc.name, func(t *testing.T) {
-			for _, l := range notEnabledLevels {
-				if core.Enabled(l) {
-					t.Errorf("level %s must not be enabled", l.String())
-				}
-			}
-
-			for _, l := range enabledLevels {
-				if !core.Enabled(l) {
-					t.Errorf("level %s must Vbe enabled", l.String())
-				}
-			}
-		})
-	}
-}
-
 type writeSyncer struct {
 	strings.Builder
 }

logp/typedloggercore.go

@@ -0,0 +1,63 @@
+package logp
+
+import (
+	"fmt"
+
+	"go.uber.org/zap/zapcore"
+)
+
+// typedLoggerCore takes two cores and directs logs entries to one of them
+// with the value of the field defined by the pair `key` and `value`
+//
+// If `entry[key] == value` the typedCore is used, otherwise the
+// defaultCore  is used.
+// WARNING: The level of both cores must always be the same!
+// typedLoggerCore will only use the defaultCore level to decide
+// whether to log an entry or not
+type typedLoggerCore struct {
+	typedCore   zapcore.Core
+	defaultCore zapcore.Core
+	value       string
+	key         string
+}
+
+func (t *typedLoggerCore) Enabled(l zapcore.Level) bool {
+	return t.defaultCore.Enabled(l)
+}
+
+func (t *typedLoggerCore) With(fields []zapcore.Field) zapcore.Core {
+	t.defaultCore = t.defaultCore.With(fields)
+	t.typedCore = t.typedCore.With(fields)
+	return t
+}
+
+func (t *typedLoggerCore) Check(e zapcore.Entry, ce *zapcore.CheckedEntry) *zapcore.CheckedEntry {
+	if t.defaultCore.Enabled(e.Level) {
+		return ce.AddCore(e, t)
+	}
+
+	return ce
+}
+
+func (t *typedLoggerCore) Sync() error {
+	defaultErr := t.defaultCore.Sync()
+	typedErr := t.typedCore.Sync()
+
+	if defaultErr != nil || typedErr != nil {
+		return fmt.Errorf("error syncing logger. DefaultCore: '%w', typedCore: '%w'", defaultErr, typedErr)
+	}
+
+	return nil
+}
+
+func (t *typedLoggerCore) Write(e zapcore.Entry, fields []zapcore.Field) error {
+	for _, f := range fields {
+		if f.Key == t.key {
+			if f.String == t.value {
+				return t.typedCore.Write(e, fields)
+			}
+		}
+	}
+
+	return t.defaultCore.Write(e, fields)
+}