Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logger: Label loggers #40

Merged
merged 8 commits into from
Jul 2, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (c Config) String() string {
return ""
}
// output in alphabetical order.
names := []string{}
var names []string
for name := range c {
names = append(names, name)
}
Expand Down Expand Up @@ -47,6 +47,10 @@ func parseConfigValue(value string) (string, Level, error) {
return "", UNSPECIFIED, fmt.Errorf("config value %q has missing module name", value)
}

if extractConfigLabel(name) != "" && strings.Contains(name, ".") {
return "", UNSPECIFIED, fmt.Errorf("config label should not contain '.', found %q", name)
SimonRichardson marked this conversation as resolved.
Show resolved Hide resolved
}

levelStr := strings.TrimSpace(pair[1])
level, ok := ParseLevel(levelStr)
if !ok {
Expand All @@ -73,6 +77,7 @@ func parseConfigValue(value string) (string, Level, error) {
//
// An example specification:
// `<root>=ERROR; foo.bar=WARNING`
// `[LABEL]=ERROR`
func ParseConfigString(specification string) (Config, error) {
specification = strings.TrimSpace(specification)
if specification == "" {
Expand All @@ -94,3 +99,14 @@ func ParseConfigString(specification string) (Config, error) {
}
return cfg, nil
}

func extractConfigLabel(s string) string {
name := strings.TrimSpace(s)
if len(s) < 3 {
return ""
}
if name[0] == '[' && name[len(name)-1] == ']' {
return name[1 : len(name)-1]
}
return ""
}
10 changes: 10 additions & 0 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ func (*ConfigSuite) TestParseConfigValue(c *gc.C) {
value: "<root> = info",
module: "",
level: INFO,
}, {
value: "[LABEL] = info",
module: "[LABEL]",
level: INFO,
}, {
value: "[LABEL.1] = info",
err: `config label should not contain '.', found "[LABEL.1]"`,
}} {
c.Logf("%d: %s", i, test.value)
module, level, err := parseConfigValue(test.value)
Expand Down Expand Up @@ -72,6 +79,9 @@ func (*ConfigSuite) TestPaarseConfigurationString(c *gc.C) {
}, {
configuration: "<root>=DEBUG",
expected: Config{"": DEBUG},
}, {
configuration: "[LABEL]=DEBUG",
expected: Config{"[LABEL]": DEBUG},
}, {
configuration: "test.module=debug",
expected: Config{"test.module": DEBUG},
Expand Down
60 changes: 53 additions & 7 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,18 @@ func NewContext(rootLevel Level) *Context {

// GetLogger returns a Logger for the given module name, creating it and
// its parents if necessary.
func (c *Context) GetLogger(name string) Logger {
func (c *Context) GetLogger(name string, labels ...string) Logger {
name = strings.TrimSpace(strings.ToLower(name))

c.modulesMutex.Lock()
defer c.modulesMutex.Unlock()
return Logger{c.getLoggerModule(name)}

return Logger{
impl: c.getLoggerModule(name, labels),
}
}

func (c *Context) getLoggerModule(name string) *module {
func (c *Context) getLoggerModule(name string, labels []string) *module {
if name == rootString {
name = ""
}
Expand All @@ -65,12 +69,44 @@ func (c *Context) getLoggerModule(name string) *module {
if i := strings.LastIndex(name, "."); i >= 0 {
parentName = name[0:i]
}
parent := c.getLoggerModule(parentName)
impl = &module{name, UNSPECIFIED, parent, c}
// Labels don't apply to the parent, otherwise <root> would have all labels.
// Selection of the label would give you all loggers again, which isn't what
// you want.
parent := c.getLoggerModule(parentName, nil)

// Ensure that we create a new logger module for the name, that includes the
// label.
labelMap := map[string]struct{}{}
for _, label := range labels {
labelMap[label] = struct{}{}
}
impl = &module{
name: name,
level: UNSPECIFIED,
parent: parent,
context: c,
labels: labels,
labelsLookup: labelMap,
}
c.modules[name] = impl
return impl
}

// getLoggerModulesByLabel returns modules that have the associated label.
func (c *Context) getLoggerModulesByLabel(label string) []*module {
var modules []*module
for _, mod := range c.modules {
if mod.labels == nil {
continue
}

if _, ok := mod.labelsLookup[label]; ok {
modules = append(modules, mod)
}
}
return modules
}

// Config returns the current configuration of the Loggers. Loggers
// with UNSPECIFIED level will not be included.
func (c *Context) Config() Config {
Expand Down Expand Up @@ -104,8 +140,18 @@ func (c *Context) ApplyConfig(config Config) {
c.modulesMutex.Lock()
defer c.modulesMutex.Unlock()
for name, level := range config {
module := c.getLoggerModule(name)
module.setLevel(level)
label := extractConfigLabel(name)
if label == "" {
module := c.getLoggerModule(name, nil)
module.setLevel(level)
continue
}

// Config contains a named label, use that for selecting the loggers.
modules := c.getLoggerModulesByLabel(label)
for _, module := range modules {
module.setLevel(level)
}
}
}

Expand Down
61 changes: 61 additions & 0 deletions context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,67 @@ func (*ContextSuite) TestApplyConfigAdditive(c *gc.C) {
})
}

func (*ContextSuite) TestApplyConfigLabels(c *gc.C) {
context := loggo.NewContext(loggo.WARNING)
context.GetLogger("a.b", "ONE")
context.GetLogger("c.d", "ONE")
context.GetLogger("e", "TWO")

context.ApplyConfig(loggo.Config{"[ONE]": loggo.TRACE})
context.ApplyConfig(loggo.Config{"[TWO]": loggo.DEBUG})

c.Assert(context.Config(), gc.DeepEquals,
loggo.Config{
"": loggo.WARNING,
"a.b": loggo.TRACE,
"c.d": loggo.TRACE,
"e": loggo.DEBUG,
})
c.Assert(context.CompleteConfig(), gc.DeepEquals,
loggo.Config{
"": loggo.WARNING,
"a": loggo.UNSPECIFIED,
"a.b": loggo.TRACE,
"c": loggo.UNSPECIFIED,
"c.d": loggo.TRACE,
"e": loggo.DEBUG,
})
}

func (*ContextSuite) TestApplyConfigLabelsAddative(c *gc.C) {
context := loggo.NewContext(loggo.WARNING)
context.ApplyConfig(loggo.Config{"[ONE]": loggo.TRACE})
SimonRichardson marked this conversation as resolved.
Show resolved Hide resolved
context.ApplyConfig(loggo.Config{"[TWO]": loggo.DEBUG})
c.Assert(context.Config(), gc.DeepEquals,
loggo.Config{
"": loggo.WARNING,
})
c.Assert(context.CompleteConfig(), gc.DeepEquals,
loggo.Config{
"": loggo.WARNING,
})
}

func (*ContextSuite) TestApplyConfigWithMalformedLabel(c *gc.C) {
context := loggo.NewContext(loggo.WARNING)
context.GetLogger("a.b", "ONE")

context.ApplyConfig(loggo.Config{"[ONE": loggo.TRACE})

c.Assert(context.Config(), gc.DeepEquals,
loggo.Config{
"": loggo.WARNING,
"[ONE": loggo.TRACE,
})
c.Assert(context.CompleteConfig(), gc.DeepEquals,
loggo.Config{
"": loggo.WARNING,
"a": loggo.UNSPECIFIED,
"a.b": loggo.UNSPECIFIED,
"[ONE": loggo.TRACE,
})
}

func (*ContextSuite) TestResetLoggerLevels(c *gc.C) {
context := loggo.NewContext(loggo.DEBUG)
context.ApplyConfig(loggo.Config{"first.second": loggo.TRACE})
Expand Down
2 changes: 2 additions & 0 deletions entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,6 @@ type Entry struct {
Timestamp time.Time
// Message is the formatted string from teh log call.
Message string
// Labels is the label associated with the log message.
Labels []string
}
4 changes: 0 additions & 4 deletions formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,3 @@ func initTimeFormat() string {
}
return "15:04:05"
}

func formatTime(ts time.Time) string {
return ts.Format(TimeFormat)
}
2 changes: 2 additions & 0 deletions logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,14 @@ func (logger Logger) LogCallf(calldepth int, level Level, message string, args .
if len(args) > 0 {
formattedMessage = fmt.Sprintf(message, args...)
}

module.write(Entry{
Level: level,
Filename: file,
Line: line,
Timestamp: now,
Message: formattedMessage,
Labels: module.labels,
})
}

Expand Down
20 changes: 12 additions & 8 deletions logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,19 @@ type LoggingSuite struct {
context *loggo.Context
writer *writer
logger loggo.Logger

// Test that labels get outputted to loggo.Entry
Labels []string
}

var _ = gc.Suite(&LoggingSuite{})
var _ = gc.Suite(&LoggingSuite{Labels: []string{"ONE", "TWO"}})

func (s *LoggingSuite) SetUpTest(c *gc.C) {
s.writer = &writer{}
s.context = loggo.NewContext(loggo.TRACE)
s.context.AddWriter("test", s.writer)
s.logger = s.context.GetLogger("test")
s.logger = s.context.GetLogger("test", s.Labels...)
}

func (s *LoggingSuite) TestLoggingStrings(c *gc.C) {
Expand All @@ -33,10 +37,10 @@ func (s *LoggingSuite) TestLoggingStrings(c *gc.C) {
s.logger.Infof("missing %s")

checkLogEntries(c, s.writer.Log(), []loggo.Entry{
{Level: loggo.INFO, Module: "test", Message: "simple"},
{Level: loggo.INFO, Module: "test", Message: "with args 42"},
{Level: loggo.INFO, Module: "test", Message: "working 100%"},
{Level: loggo.INFO, Module: "test", Message: "missing %s"},
{Level: loggo.INFO, Module: "test", Message: "simple", Labels: s.Labels},
{Level: loggo.INFO, Module: "test", Message: "with args 42", Labels: s.Labels},
{Level: loggo.INFO, Module: "test", Message: "working 100%", Labels: s.Labels},
{Level: loggo.INFO, Module: "test", Message: "missing %s", Labels: s.Labels},
})
}

Expand All @@ -47,9 +51,9 @@ func (s *LoggingSuite) TestLoggingLimitWarning(c *gc.C) {
end := time.Now()
entries := s.writer.Log()
checkLogEntries(c, entries, []loggo.Entry{
{Level: loggo.CRITICAL, Module: "test", Message: "something critical"},
{Level: loggo.ERROR, Module: "test", Message: "an error"},
{Level: loggo.WARNING, Module: "test", Message: "a warning message"},
{Level: loggo.CRITICAL, Module: "test", Message: "something critical", Labels: s.Labels},
{Level: loggo.ERROR, Module: "test", Message: "an error", Labels: s.Labels},
{Level: loggo.WARNING, Module: "test", Message: "a warning message", Labels: s.Labels},
})

for _, entry := range entries {
Expand Down
22 changes: 12 additions & 10 deletions module.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ type module struct {
level Level
parent *module
context *Context

labels []string
labelsLookup map[string]struct{}
}

// Name returns the module's name.
func (module *module) Name() string {
if module.name == "" {
func (m *module) Name() string {
if m.name == "" {
return rootString
}
return module.name
return m.name
}

func (m *module) willWrite(level Level) bool {
Expand All @@ -32,27 +35,26 @@ func (m *module) willWrite(level Level) bool {
return level >= m.getEffectiveLogLevel()
}

func (module *module) getEffectiveLogLevel() Level {
func (m *module) getEffectiveLogLevel() Level {
// Note: the root module is guaranteed to have a
// specified logging level, so acts as a suitable sentinel
// for this loop.
for {
if level := module.level.get(); level != UNSPECIFIED {
if level := m.level.get(); level != UNSPECIFIED {
return level
}
module = module.parent
m = m.parent
}
panic("unreachable")
}

// setLevel sets the severity level of the given module.
// The root module cannot be set to UNSPECIFIED level.
func (module *module) setLevel(level Level) {
func (m *module) setLevel(level Level) {
// The root module can't be unspecified.
if module.name == "" && level == UNSPECIFIED {
if m.name == "" && level == UNSPECIFIED {
level = WARNING
}
module.level.set(level)
m.level.set(level)
}

func (m *module) write(entry Entry) {
Expand Down
22 changes: 22 additions & 0 deletions writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,28 @@ func (s *SimpleWriterSuite) TestNewSimpleWriter(c *gc.C) {
Line: 12,
Timestamp: now,
Message: "a message",
Labels: nil,
})

c.Check(buf.String(), gc.Equals, "<< a message >>\n")
}

func (s *SimpleWriterSuite) TestNewSimpleWriterWithLabels(c *gc.C) {
now := time.Now()
formatter := func(entry loggo.Entry) string {
return "<< " + entry.Message + " >>"
}
buf := &bytes.Buffer{}

writer := loggo.NewSimpleWriter(buf, formatter)
writer.Write(loggo.Entry{
Level: loggo.INFO,
Module: "test",
Filename: "somefile.go",
Line: 12,
Timestamp: now,
Message: "a message",
Labels: []string{"ONE", "TWO"},
})

c.Check(buf.String(), gc.Equals, "<< a message >>\n")
Expand Down