comments some more
This commit is contained in:
HiteshRepo
parent
3305f1ffd4
commit
8595ff1088
@ -3,12 +3,7 @@ package logger_test
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
|
||||||
"github.com/alcionai/clues"
|
|
||||||
|
|
||||||
"github.com/alcionai/corso/src/internal/m365/resource"
|
|
||||||
"github.com/alcionai/corso/src/pkg/logger"
|
|
||||||
"github.com/alcionai/corso/src/pkg/path"
|
"github.com/alcionai/corso/src/pkg/path"
|
||||||
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@ -33,166 +28,168 @@ func Example_seed() {
|
|||||||
// Corso's CLI layer seeds the logger in the cli initialization.
|
// Corso's CLI layer seeds the logger in the cli initialization.
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
ls := logger.Settings{
|
_ = ctx
|
||||||
File: logger.Stderr,
|
|
||||||
Level: logger.LLInfo,
|
|
||||||
PIIHandling: logger.PIIPlainText,
|
|
||||||
}
|
|
||||||
|
|
||||||
ctx, log := logger.Seed(ctx, ls)
|
// ls := logger.Settings{
|
||||||
|
// File: logger.Stderr,
|
||||||
|
// Level: logger.LLInfo,
|
||||||
|
// PIIHandling: logger.PIIPlainText,
|
||||||
|
// }
|
||||||
|
|
||||||
// SDK consumers who configure their own zap logger can Set their logger
|
// ctx, log := logger.Seed(ctx, ls)
|
||||||
// into the context directly, instead of Seeding a new one.
|
|
||||||
ctx = logger.Set(ctx, log)
|
|
||||||
|
|
||||||
// logs should always be flushed before exiting whichever func
|
// // SDK consumers who configure their own zap logger can Set their logger
|
||||||
// seeded the logger.
|
// // into the context directly, instead of Seeding a new one.
|
||||||
defer func() {
|
// ctx = logger.Set(ctx, log)
|
||||||
_ = log.Sync() // flush all logs in the buffer
|
|
||||||
}()
|
|
||||||
|
|
||||||
// downstream, the logger will retrieve its configuration from
|
// // logs should always be flushed before exiting whichever func
|
||||||
// the context.
|
// // seeded the logger.
|
||||||
func(ctx context.Context) {
|
// defer func() {
|
||||||
log := logger.Ctx(ctx)
|
// _ = log.Sync() // flush all logs in the buffer
|
||||||
log.Info("hello, world!")
|
// }()
|
||||||
}(ctx)
|
|
||||||
|
// // downstream, the logger will retrieve its configuration from
|
||||||
|
// // the context.
|
||||||
|
// func(ctx context.Context) {
|
||||||
|
// log := logger.Ctx(ctx)
|
||||||
|
// log.Info("hello, world!")
|
||||||
|
// }(ctx)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ExampleLoggerStandards reviews code standards around logging in Corso.
|
// ExampleLoggerStandards reviews code standards around logging in Corso.
|
||||||
func Example_logger_standards() {
|
func Example_logger_standards() {
|
||||||
log := logger.Ctx(context.Background())
|
// log := logger.Ctx(context.Background())
|
||||||
|
|
||||||
// 1. Keep messages short. When possible, messages should state the current action.
|
// // 1. Keep messages short. When possible, messages should state the current action.
|
||||||
// Lowercase text, no ending punctuation.
|
// // Lowercase text, no ending punctuation.
|
||||||
// This ensures logs are easy to scan, and simple to grok.
|
// // This ensures logs are easy to scan, and simple to grok.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
log.Info("getting item")
|
// log.Info("getting item")
|
||||||
// avoid
|
// // avoid
|
||||||
log.Info("Getting one item from the service so that we can send it through the item feed.")
|
// log.Info("Getting one item from the service so that we can send it through the item feed.")
|
||||||
|
|
||||||
// 2. Avoid statements like "unable to...", "failed to..", or "error when...".
|
// // 2. Avoid statements like "unable to...", "failed to..", or "error when...".
|
||||||
// Error level logs automatically imply a failure to do the action.
|
// // Error level logs automatically imply a failure to do the action.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
log.With("err", err).Error("connecting to repo")
|
// log.With("err", err).Error("connecting to repo")
|
||||||
// avoid
|
// // avoid
|
||||||
log.With("err", err).Error("unable to connect to repo")
|
// log.With("err", err).Error("unable to connect to repo")
|
||||||
|
|
||||||
// 3. Do not fmt values into the message. Use With() or -w() to add structured data.
|
// // 3. Do not fmt values into the message. Use With() or -w() to add structured data.
|
||||||
// By keeping dynamic data in a structured format, we maximize log readability,
|
// // By keeping dynamic data in a structured format, we maximize log readability,
|
||||||
// and make logs very easy to search or filter in bulk, and very easy to control pii.
|
// // and make logs very easy to search or filter in bulk, and very easy to control pii.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
log.With("err", err).Error("getting item")
|
// log.With("err", err).Error("getting item")
|
||||||
log.Errorw("getting item", "err", err)
|
// log.Errorw("getting item", "err", err)
|
||||||
// avoid
|
// // avoid
|
||||||
log.Errorf("getting item %s: %v", itemID, err)
|
// log.Errorf("getting item %s: %v", itemID, err)
|
||||||
|
|
||||||
// 4. Give data keys reasonable namespaces. Use snake_case.
|
// // 4. Give data keys reasonable namespaces. Use snake_case.
|
||||||
// Overly generic keys can collide unexpectedly.
|
// // Overly generic keys can collide unexpectedly.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
log.With("item_id", itemID).Info("getting item")
|
// log.With("item_id", itemID).Info("getting item")
|
||||||
// avoid
|
// // avoid
|
||||||
log.With("id", itemID).Error("getting item")
|
// log.With("id", itemID).Error("getting item")
|
||||||
|
|
||||||
// 4. Avoid Warn-level logging. Prefer Info or Error.
|
// // 4. Avoid Warn-level logging. Prefer Info or Error.
|
||||||
// Minimize confusion/contention about what level a log
|
// // Minimize confusion/contention about what level a log
|
||||||
// "should be". Error during a failure, Info (or Debug)
|
// // "should be". Error during a failure, Info (or Debug)
|
||||||
// otherwise.
|
// // otherwise.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
log.With("err", err).Error("getting item")
|
// log.With("err", err).Error("getting item")
|
||||||
// avoid
|
// // avoid
|
||||||
log.With("err", err).Warn("getting item")
|
// log.With("err", err).Warn("getting item")
|
||||||
|
|
||||||
// 5. Avoid Panic/Fatal-level logging. Prefer Error.
|
// // 5. Avoid Panic/Fatal-level logging. Prefer Error.
|
||||||
// Panic and Fatal logging can crash the application without
|
// // Panic and Fatal logging can crash the application without
|
||||||
// flushing buffered logs and finishing out other telemetry.
|
// // flushing buffered logs and finishing out other telemetry.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
log.With("err", err).Error("connecting to repo")
|
// log.With("err", err).Error("connecting to repo")
|
||||||
// avoid
|
// // avoid
|
||||||
log.With("err", err).Panic("connecting to repo")
|
// log.With("err", err).Panic("connecting to repo")
|
||||||
}
|
}
|
||||||
|
|
||||||
// ExampleLoggerCluesStandards reviews code standards around using the Clues package while logging.
|
// ExampleLoggerCluesStandards reviews code standards around using the Clues package while logging.
|
||||||
func Example_logger_clues_standards() {
|
func Example_logger_clues_standards() {
|
||||||
ctx := clues.Add(context.Background(), "foo", "bar")
|
// ctx := clues.Add(context.Background(), "foo", "bar")
|
||||||
log := logger.Ctx(ctx)
|
// log := logger.Ctx(ctx)
|
||||||
|
|
||||||
// 1. Clues Ctx values are always added in .Ctx(); you don't
|
// // 1. Clues Ctx values are always added in .Ctx(); you don't
|
||||||
// need to add them directly.
|
// // need to add them directly.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
ctx = clues.Add(ctx, "item_id", itemID)
|
// ctx = clues.Add(ctx, "item_id", itemID)
|
||||||
logger.Ctx(ctx).Info("getting item")
|
// logger.Ctx(ctx).Info("getting item")
|
||||||
//
|
// //
|
||||||
// avoid
|
// // avoid
|
||||||
ctx = clues.Add(ctx, "item_id", itemID)
|
// ctx = clues.Add(ctx, "item_id", itemID)
|
||||||
logger.Ctx(ctx).With(clues.In(ctx).Slice()...).Info("getting item")
|
// logger.Ctx(ctx).With(clues.In(ctx).Slice()...).Info("getting item")
|
||||||
|
|
||||||
// 2. The last func to handle a context must add the clues to the error.
|
// // 2. The last func to handle a context must add the clues to the error.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
err := clues.WrapWC(ctx, err, "reason")
|
// err := clues.WrapWC(ctx, err, "reason")
|
||||||
// this dereference added for linter happiness
|
// // this dereference added for linter happiness
|
||||||
_ = err
|
// _ = err
|
||||||
|
|
||||||
// 3. Always extract structured data from errors.
|
// // 3. Always extract structured data from errors.
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
log.With("error", err).Errorw("getting item", clues.InErr(err).Slice()...)
|
// log.With("error", err).Errorw("getting item", clues.InErr(err).Slice()...)
|
||||||
//
|
// //
|
||||||
// avoid
|
// // avoid
|
||||||
log.Errorw("getting item", "err", err)
|
// log.Errorw("getting item", "err", err)
|
||||||
//
|
// //
|
||||||
// you can use the logger helper CtxErr() for the same results.
|
// // you can use the logger helper CtxErr() for the same results.
|
||||||
// This helps to ensure all error values get packed into the logs
|
// // This helps to ensure all error values get packed into the logs
|
||||||
// in the expected format.
|
// // in the expected format.
|
||||||
logger.CtxErr(ctx, err).Error("getting item")
|
// logger.CtxErr(ctx, err).Error("getting item")
|
||||||
|
|
||||||
// 3. Protect pii in logs.
|
// // 3. Protect pii in logs.
|
||||||
// When it comes to protecting sensitive information, we only want
|
// // When it comes to protecting sensitive information, we only want
|
||||||
// to hand loggers (and, by extension, clues errors) using one of
|
// // to hand loggers (and, by extension, clues errors) using one of
|
||||||
// three approaches to securing values.
|
// // three approaches to securing values.
|
||||||
//
|
// //
|
||||||
// First: plain, unhidden data. This can only be logged if we are
|
// // First: plain, unhidden data. This can only be logged if we are
|
||||||
// absolutely assured that this data does not expose sensitive
|
// // absolutely assured that this data does not expose sensitive
|
||||||
// information for a user. Eg: internal ids and enums are fine to
|
// // information for a user. Eg: internal ids and enums are fine to
|
||||||
// log in plain text. Everything else must be considered wisely.
|
// // log in plain text. Everything else must be considered wisely.
|
||||||
//
|
// //
|
||||||
// Second: manually concealed values. Strings containing sensitive
|
// // Second: manually concealed values. Strings containing sensitive
|
||||||
// info, and structs from external pacakges containing sensitive info,
|
// // info, and structs from external pacakges containing sensitive info,
|
||||||
// can be logged by manually wrapping them with a clues.Hide() call.
|
// // can be logged by manually wrapping them with a clues.Hide() call.
|
||||||
// Ex: clues.Hide(userName). This will hash the value according to
|
// // Ex: clues.Hide(userName). This will hash the value according to
|
||||||
// the user's hash algorithm configuration.
|
// // the user's hash algorithm configuration.
|
||||||
//
|
// //
|
||||||
// Third: managed string concealers. Certain values have common
|
// // Third: managed string concealers. Certain values have common
|
||||||
// format and content, but appear commonly in the code as strings.
|
// // format and content, but appear commonly in the code as strings.
|
||||||
// Examples include URLs and kopia repository paths. These values
|
// // Examples include URLs and kopia repository paths. These values
|
||||||
// may have a concealer built specifically for them to maximize the
|
// // may have a concealer built specifically for them to maximize the
|
||||||
// data we can view when debugging, instead of hashing the complete
|
// // data we can view when debugging, instead of hashing the complete
|
||||||
// string. See graph/middleware.go LoggableURL{} and path/elements.go
|
// // string. See graph/middleware.go LoggableURL{} and path/elements.go
|
||||||
// LoggableDir{}.
|
// // LoggableDir{}.
|
||||||
//
|
// //
|
||||||
// Fourth: structs that comply with clues.Concealer. The Concealer
|
// // Fourth: structs that comply with clues.Concealer. The Concealer
|
||||||
// interface requires a struct to comply with Conceal() (for cases
|
// // interface requires a struct to comply with Conceal() (for cases
|
||||||
// where the struct is handed to a clues aggregator directly), and
|
// // where the struct is handed to a clues aggregator directly), and
|
||||||
// fmt's Format(state, verb), where the assumption is the standard
|
// // fmt's Format(state, verb), where the assumption is the standard
|
||||||
// format writer will be replaced with a Conceal() call (for cases
|
// // format writer will be replaced with a Conceal() call (for cases
|
||||||
// where the struct is handed to some non-compliant formatter/printer).
|
// // where the struct is handed to some non-compliant formatter/printer).
|
||||||
//
|
// //
|
||||||
// preferred
|
// // preferred
|
||||||
log.With(
|
// log.With(
|
||||||
// internal type, safe to log plainly
|
// // internal type, safe to log plainly
|
||||||
"resource_type", resource.Users,
|
// "resource_type", resource.Users,
|
||||||
// string containing sensitive info, wrap with Hide()
|
// // string containing sensitive info, wrap with Hide()
|
||||||
"user_name", clues.Hide("your_user_name@microsoft.example"),
|
// "user_name", clues.Hide("your_user_name@microsoft.example"),
|
||||||
// string partially concealed by a managed concealer.
|
// // string partially concealed by a managed concealer.
|
||||||
"request_url", graph.LoggableURL("https://corsobackup.io"),
|
// "request_url", graph.LoggableURL("https://corsobackup.io"),
|
||||||
// a concealer-compliant struct, safe to add plainly
|
// // a concealer-compliant struct, safe to add plainly
|
||||||
"storage_path", itemPath)
|
// "storage_path", itemPath)
|
||||||
}
|
}
|
||||||
|
|||||||
@ -5,8 +5,10 @@ import (
|
|||||||
|
|
||||||
"github.com/spf13/cobra"
|
"github.com/spf13/cobra"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
"github.com/stretchr/testify/suite"
|
"github.com/stretchr/testify/suite"
|
||||||
|
|
||||||
|
"github.com/alcionai/clues"
|
||||||
"github.com/alcionai/corso/src/internal/tester"
|
"github.com/alcionai/corso/src/internal/tester"
|
||||||
"github.com/alcionai/corso/src/pkg/logger"
|
"github.com/alcionai/corso/src/pkg/logger"
|
||||||
)
|
)
|
||||||
@ -53,75 +55,69 @@ func (suite *LoggerUnitSuite) TestAddLoggingFlags() {
|
|||||||
"--" + logger.MaskSensitiveDataFN,
|
"--" + logger.MaskSensitiveDataFN,
|
||||||
})
|
})
|
||||||
|
|
||||||
// err := cmd.Execute()
|
err := cmd.Execute()
|
||||||
// require.NoError(t, err, clues.ToCore(err))
|
require.NoError(t, err, clues.ToCore(err))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite *LoggerUnitSuite) TestPreloadLoggingFlags() {
|
func (suite *LoggerUnitSuite) TestPreloadLoggingFlags() {
|
||||||
t := suite.T()
|
t := suite.T()
|
||||||
|
|
||||||
_ = t
|
logger.DebugAPIFV = false
|
||||||
|
logger.ReadableLogsFV = false
|
||||||
|
|
||||||
// logger.DebugAPIFV = false
|
args := []string{
|
||||||
// logger.ReadableLogsFV = false
|
"--" + logger.DebugAPIFN,
|
||||||
|
"--" + logger.LogFileFN, "log-file",
|
||||||
|
"--" + logger.LogLevelFN, string(logger.LLError),
|
||||||
|
"--" + logger.LogFormatFN, string(logger.LFText),
|
||||||
|
"--" + logger.ReadableLogsFN,
|
||||||
|
"--" + logger.MaskSensitiveDataFN,
|
||||||
|
}
|
||||||
|
|
||||||
// args := []string{
|
settings := logger.PreloadLoggingFlags(args)
|
||||||
// "--" + logger.DebugAPIFN,
|
|
||||||
// "--" + logger.LogFileFN, "log-file",
|
|
||||||
// "--" + logger.LogLevelFN, string(logger.LLError),
|
|
||||||
// "--" + logger.LogFormatFN, string(logger.LFText),
|
|
||||||
// "--" + logger.ReadableLogsFN,
|
|
||||||
// "--" + logger.MaskSensitiveDataFN,
|
|
||||||
// }
|
|
||||||
|
|
||||||
// settings := logger.PreloadLoggingFlags(args)
|
assert.True(t, logger.DebugAPIFV, logger.DebugAPIFN)
|
||||||
|
assert.True(t, logger.ReadableLogsFV, logger.ReadableLogsFN)
|
||||||
// assert.True(t, logger.DebugAPIFV, logger.DebugAPIFN)
|
assert.Equal(t, "log-file", settings.File, "settings.File")
|
||||||
// assert.True(t, logger.ReadableLogsFV, logger.ReadableLogsFN)
|
assert.Equal(t, logger.LLError, settings.Level, "settings.Level")
|
||||||
// assert.Equal(t, "log-file", settings.File, "settings.File")
|
assert.Equal(t, logger.LFText, settings.Format, "settings.Format")
|
||||||
// assert.Equal(t, logger.LLError, settings.Level, "settings.Level")
|
assert.Equal(t, logger.PIIHash, settings.PIIHandling, "settings.PIIHandling")
|
||||||
// assert.Equal(t, logger.LFText, settings.Format, "settings.Format")
|
|
||||||
// assert.Equal(t, logger.PIIHash, settings.PIIHandling, "settings.PIIHandling")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite *LoggerUnitSuite) TestPreloadLoggingFlags_badArgsEnsureDefault() {
|
func (suite *LoggerUnitSuite) TestPreloadLoggingFlags_badArgsEnsureDefault() {
|
||||||
t := suite.T()
|
t := suite.T()
|
||||||
|
|
||||||
_ = t
|
logger.DebugAPIFV = false
|
||||||
|
logger.ReadableLogsFV = false
|
||||||
|
|
||||||
// logger.DebugAPIFV = false
|
args := []string{
|
||||||
// logger.ReadableLogsFV = false
|
"--" + logger.DebugAPIFN,
|
||||||
|
"--" + logger.LogFileFN, "log-file",
|
||||||
|
"--" + logger.LogLevelFN, "not-a-level",
|
||||||
|
"--" + logger.LogFormatFN, "not-a-format",
|
||||||
|
"--" + logger.ReadableLogsFN,
|
||||||
|
"--" + logger.MaskSensitiveDataFN,
|
||||||
|
}
|
||||||
|
|
||||||
// args := []string{
|
settings := logger.PreloadLoggingFlags(args)
|
||||||
// "--" + logger.DebugAPIFN,
|
settings = settings.EnsureDefaults()
|
||||||
// "--" + logger.LogFileFN, "log-file",
|
|
||||||
// "--" + logger.LogLevelFN, "not-a-level",
|
|
||||||
// "--" + logger.LogFormatFN, "not-a-format",
|
|
||||||
// "--" + logger.ReadableLogsFN,
|
|
||||||
// "--" + logger.MaskSensitiveDataFN,
|
|
||||||
// }
|
|
||||||
|
|
||||||
// settings := logger.PreloadLoggingFlags(args)
|
assert.Equal(t, logger.LLInfo, settings.Level, "settings.Level")
|
||||||
// settings = settings.EnsureDefaults()
|
assert.Equal(t, logger.LFText, settings.Format, "settings.Format")
|
||||||
|
|
||||||
// assert.Equal(t, logger.LLInfo, settings.Level, "settings.Level")
|
|
||||||
// assert.Equal(t, logger.LFText, settings.Format, "settings.Format")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite *LoggerUnitSuite) TestSettings_ensureDefaults() {
|
func (suite *LoggerUnitSuite) TestSettings_ensureDefaults() {
|
||||||
t := suite.T()
|
t := suite.T()
|
||||||
|
|
||||||
_ = t
|
s := logger.Settings{}
|
||||||
|
require.Empty(t, s.File, "file")
|
||||||
|
require.Empty(t, s.Level, "level")
|
||||||
|
require.Empty(t, s.Format, "format")
|
||||||
|
require.Empty(t, s.PIIHandling, "piialg")
|
||||||
|
|
||||||
// s := logger.Settings{}
|
s = s.EnsureDefaults()
|
||||||
// require.Empty(t, s.File, "file")
|
require.NotEmpty(t, s.File, "file")
|
||||||
// require.Empty(t, s.Level, "level")
|
require.NotEmpty(t, s.Level, "level")
|
||||||
// require.Empty(t, s.Format, "format")
|
require.NotEmpty(t, s.Format, "format")
|
||||||
// require.Empty(t, s.PIIHandling, "piialg")
|
require.NotEmpty(t, s.PIIHandling, "piialg")
|
||||||
|
|
||||||
// s = s.EnsureDefaults()
|
|
||||||
// require.NotEmpty(t, s.File, "file")
|
|
||||||
// require.NotEmpty(t, s.Level, "level")
|
|
||||||
// require.NotEmpty(t, s.Format, "format")
|
|
||||||
// require.NotEmpty(t, s.PIIHandling, "piialg")
|
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user