tflog+tfsdklog: Added WithAdditionalLocationOffset function (#36)

Reference: #29

Allows implementations to fix the location offset of caller information when using helper functions. While typically expected for subsystem loggers, SDKs could also offer the configuration of the root provider logger as well.

Since each `hclog.Logger` is mainly immutable once its created (except for level and creating named sub-loggers), the `hclog.LoggerOptions` are saved to the context as well, allowing `NewSubsystem` to appropriately have all the root logger options available to create new sub-loggers. Maybe this can be changed upstream in the future, although that would technically be a breaking change since the interface would need to be modified.

This changeset also includes setting the name of the test root SDK/provider loggers, so that subsystem naming (and therefore the unit testing) matches real world usage.

.changelog/36.txt

@@ -0,0 +1,7 @@
+```release-note:enhancement
+tflog: Added `WithAdditionalLocationOffset` function, which allows implementations to adjust the location offset when using helper functions
+```
+
+```release-note:enhancement
+tfsdklog: Added `WithAdditionalLocationOffset` function, which allows implementations to adjust the location offset when using helper functions
+```

internal/hclogutils/logger_options.go

@@ -0,0 +1,29 @@
+package hclogutils
+
+import (
+	"github.com/hashicorp/go-hclog"
+)
+
+// LoggerOptionsCopy will safely copy LoggerOptions. Manually implemented
+// to save importing a dependency such as github.com/mitchellh/copystructure.
+func LoggerOptionsCopy(src *hclog.LoggerOptions) *hclog.LoggerOptions {
+	if src == nil {
+		return nil
+	}
+
+	return &hclog.LoggerOptions{
+		AdditionalLocationOffset: src.AdditionalLocationOffset,
+		Color:                    src.Color,
+		DisableTime:              src.DisableTime,
+		Exclude:                  src.Exclude,
+		IncludeLocation:          src.IncludeLocation,
+		IndependentLevels:        src.IndependentLevels,
+		JSONFormat:               src.JSONFormat,
+		Level:                    src.Level,
+		Mutex:                    src.Mutex,
+		Name:                     src.Name,
+		Output:                   src.Output,
+		TimeFormat:               src.TimeFormat,
+		TimeFn:                   src.TimeFn,
+	}
+}

internal/loggertest/provider.go

@@ -4,19 +4,27 @@ import (
 	"context"
 	"io"
 
-	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/terraform-plugin-log/internal/logging"
+	"github.com/hashicorp/terraform-plugin-log/tfsdklog"
 )
 
 func ProviderRoot(ctx context.Context, output io.Writer) context.Context {
-	loggerOptions := &hclog.LoggerOptions{
-		DisableTime: true,
-		JSONFormat:  true,
-		Level:       hclog.Trace,
-		Output:      output,
-	}
-
-	ctx = logging.SetProviderRootLogger(ctx, hclog.New(loggerOptions))
+	return tfsdklog.NewRootProviderLogger(
+		ctx,
+		logging.WithoutLocation(),
+		logging.WithoutTimestamp(),
+		logging.WithOutput(output),
+	)
+}
 
-	return ctx
+// ProviderRootWithLocation is for testing code that affects go-hclog's caller
+// information (location offset). Most testing code should avoid this, since
+// correctly checking differences including the location is extra effort
+// with little benefit.
+func ProviderRootWithLocation(ctx context.Context, output io.Writer) context.Context {
+	return tfsdklog.NewRootProviderLogger(
+		ctx,
+		logging.WithoutTimestamp(),
+		logging.WithOutput(output),
+	)
 }

internal/loggertest/sdk.go

@@ -4,19 +4,27 @@ import (
 	"context"
 	"io"
 
-	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/terraform-plugin-log/internal/logging"
+	"github.com/hashicorp/terraform-plugin-log/tfsdklog"
 )
 
 func SDKRoot(ctx context.Context, output io.Writer) context.Context {
-	loggerOptions := &hclog.LoggerOptions{
-		DisableTime: true,
-		JSONFormat:  true,
-		Level:       hclog.Trace,
-		Output:      output,
-	}
-
-	ctx = logging.SetSDKRootLogger(ctx, hclog.New(loggerOptions))
+	return tfsdklog.NewRootSDKLogger(
+		ctx,
+		logging.WithoutLocation(),
+		logging.WithoutTimestamp(),
+		logging.WithOutput(output),
+	)
+}
 
-	return ctx
+// SDKRootWithLocation is for testing code that affects go-hclog's caller
+// information (location offset). Most testing code should avoid this, since
+// correctly checking differences including the location is extra effort
+// with little benefit.
+func SDKRootWithLocation(ctx context.Context, output io.Writer) context.Context {
+	return tfsdklog.NewRootSDKLogger(
+		ctx,
+		logging.WithoutTimestamp(),
+		logging.WithOutput(output),
+	)
 }

internal/logging/log.go

@@ -5,6 +5,14 @@ import (
 	"os"
 )
 
+const (
+	// Default provider root logger name.
+	DefaultProviderRootLoggerName string = "provider"
+
+	// Default SDK root logger name.
+	DefaultSDKRootLoggerName string = "sdk"
+)
+
 // loggerKey defines context keys for locating loggers in context.Context
 // it's a private type to make sure no other packages can override the key
 type loggerKey string
@@ -14,10 +22,22 @@ const (
 	// logger for writing logs from within provider code.
 	ProviderRootLoggerKey loggerKey = "provider"
 
+	// ProviderRootLoggerOptionsKey is the loggerKey that will hold the root
+	// logger options when the root provider logger is created. This is to
+	// assist creating subsystem loggers, as most options cannot be fetched and
+	// a logger does not provide set methods for these options.
+	ProviderRootLoggerOptionsKey loggerKey = "provider-options"
+
 	// SDKRootLoggerKey is the loggerKey that will hold the root logger for
 	// writing logs from with SDKs.
 	SDKRootLoggerKey loggerKey = "sdk"
 
+	// SDKRootLoggerOptionsKey is the loggerKey that will hold the root
+	// logger options when the SDK provider logger is created. This is to
+	// assist creating subsystem loggers, as most options cannot be fetched and
+	// a logger does not provide set methods for these options.
+	SDKRootLoggerOptionsKey loggerKey = "sdk-options"
+
 	// SinkKey is the loggerKey that will hold the logging sink used for
 	// test frameworks.
 	SinkKey loggerKey = ""

internal/logging/options.go

@@ -22,6 +22,11 @@ type LoggerOpts struct {
 	// of the logging statement or not.
 	IncludeLocation bool
 
+	// AdditionalLocationOffset is the number of additional stack levels to
+	// skip when finding the file and line information for the log line.
+	// Defaults to 1 to account for the tflog and tfsdklog logging functions.
+	AdditionalLocationOffset int
+
 	// Output dictates where logs are written to. Output should only ever
 	// be set by tflog or tfsdklog, never by SDK authors or provider
 	// developers. Where logs get written to is complex and delicate and
@@ -37,21 +42,34 @@ type LoggerOpts struct {
 }
 
 // ApplyLoggerOpts generates a LoggerOpts out of a list of Option
-// implementations. By default, IncludeLocation is true, IncludeTime is true,
-// and Output is os.Stderr.
+// implementations. By default, AdditionalLocationOffset is 1, IncludeLocation
+// is true, IncludeTime is true, and Output is os.Stderr.
 func ApplyLoggerOpts(opts ...Option) LoggerOpts {
 	// set some defaults
 	l := LoggerOpts{
-		IncludeLocation: true,
-		IncludeTime:     true,
-		Output:          os.Stderr,
+		AdditionalLocationOffset: 1,
+		IncludeLocation:          true,
+		IncludeTime:              true,
+		Output:                   os.Stderr,
 	}
 	for _, opt := range opts {
 		l = opt(l)
 	}
 	return l
 }
 
+// WithAdditionalLocationOffset sets the WithAdditionalLocationOffset
+// configuration option, allowing implementations to fix location information
+// when implementing helper functions. The default offset of 1 is automatically
+// added to the provided value to account for the tflog and tfsdk logging
+// functions.
+func WithAdditionalLocationOffset(additionalLocationOffset int) Option {
+	return func(l LoggerOpts) LoggerOpts {
+		l.AdditionalLocationOffset = additionalLocationOffset + 1
+		return l
+	}
+}
+
 // WithOutput sets the Output configuration option, controlling where logs get
 // written to. This is mostly used for testing (to write to os.Stdout, so the
 // test framework can compare it against the example output) and as a helper
@@ -63,6 +81,16 @@ func WithOutput(output io.Writer) Option {
 	}
 }
 
+// WithoutLocation disables the location included with logging statements. It
+// should only ever be used to make log output deterministic when testing
+// terraform-plugin-log.
+func WithoutLocation() Option {
+	return func(l LoggerOpts) LoggerOpts {
+		l.IncludeLocation = false
+		return l
+	}
+}
+
 // WithoutTimestamp disables the timestamp included with logging statements. It
 // should only ever be used to make log output deterministic when testing
 // terraform-plugin-log.

internal/logging/provider.go

@@ -16,12 +16,41 @@ func GetProviderRootLogger(ctx context.Context) hclog.Logger {
 	return logger.(hclog.Logger)
 }
 
+// GetProviderRootLoggerOptions returns the root logger options used for
+// creating the root provider logger. If the root logger has not been created
+// or the options are not present, it will return nil.
+func GetProviderRootLoggerOptions(ctx context.Context) *hclog.LoggerOptions {
+	if GetProviderRootLogger(ctx) == nil {
+		return nil
+	}
+
+	loggerOptionsRaw := ctx.Value(ProviderRootLoggerOptionsKey)
+
+	if loggerOptionsRaw == nil {
+		return nil
+	}
+
+	loggerOptions, ok := loggerOptionsRaw.(*hclog.LoggerOptions)
+
+	if !ok {
+		return nil
+	}
+
+	return loggerOptions
+}
+
 // SetProviderRootLogger sets `logger` as the root logger used for writing
 // logs from a provider.
 func SetProviderRootLogger(ctx context.Context, logger hclog.Logger) context.Context {
 	return context.WithValue(ctx, ProviderRootLoggerKey, logger)
 }
 
+// SetProviderRootLoggerOptions sets `loggerOptions` as the root logger options
+// used for creating the provider root logger.
+func SetProviderRootLoggerOptions(ctx context.Context, loggerOptions *hclog.LoggerOptions) context.Context {
+	return context.WithValue(ctx, ProviderRootLoggerOptionsKey, loggerOptions)
+}
+
 // NewProviderSubsystemLoggerWarning is the text included in log output when a
 // subsystem is auto-generated by terraform-plugin-log because it was used
 // before the provider instantiated it.

internal/logging/sdk.go

@@ -16,12 +16,41 @@ func GetSDKRootLogger(ctx context.Context) hclog.Logger {
 	return logger.(hclog.Logger)
 }
 
+// GetSDKRootLoggerOptions returns the root logger options used for
+// creating the root SDK logger. If the root logger has not been created or
+// the options are not present, it will return nil.
+func GetSDKRootLoggerOptions(ctx context.Context) *hclog.LoggerOptions {
+	if GetSDKRootLogger(ctx) == nil {
+		return nil
+	}
+
+	loggerOptionsRaw := ctx.Value(SDKRootLoggerOptionsKey)
+
+	if loggerOptionsRaw == nil {
+		return nil
+	}
+
+	loggerOptions, ok := loggerOptionsRaw.(*hclog.LoggerOptions)
+
+	if !ok {
+		return nil
+	}
+
+	return loggerOptions
+}
+
 // SetSDKRootLogger sets `logger` as the root logger used for writing logs from
 // an SDK.
 func SetSDKRootLogger(ctx context.Context, logger hclog.Logger) context.Context {
 	return context.WithValue(ctx, SDKRootLoggerKey, logger)
 }
 
+// SetSDKRootLoggerOptions sets `loggerOptions` as the root logger options
+// used for creating the SDK root logger.
+func SetSDKRootLoggerOptions(ctx context.Context, loggerOptions *hclog.LoggerOptions) context.Context {
+	return context.WithValue(ctx, SDKRootLoggerOptionsKey, loggerOptions)
+}
+
 // NewSDKSubsystemLoggerWarning is the text included in log output when a
 // subsystem is auto-generated by terraform-plugin-log because it was used
 // before the SDK instantiated it.

tflog/options.go

@@ -12,6 +12,14 @@ import (
 // to NewSubsystem prior to calling it.
 type Options []logging.Option
 
+// WithAdditionalLocationOffset returns an option that allowing implementations
+// to fix location information when implementing helper functions. The default
+// offset of 1 is automatically added to the provided value to account for the
+// tflog logging functions.
+func WithAdditionalLocationOffset(additionalLocationOffset int) logging.Option {
+	return logging.WithAdditionalLocationOffset(additionalLocationOffset)
+}
+
 // WithLevelFromEnv returns an option that will set the level of the logger
 // based on the string in an environment variable. The environment variable
 // checked will be `name` and `subsystems`, joined by _ and in all caps.