Merge pull request #49 from SimonRichardson/inherited-labels

Inherited labels

config.go

@@ -47,19 +47,19 @@ func parseConfigValue(value string) (string, Level, error) {
 		return "", UNSPECIFIED, fmt.Errorf("config value %q has missing module name", value)
 	}
 
-	if label := extractConfigLabel(name); label != "" {
-		if strings.Contains(label, ".") {
+	if tag := extractConfigTag(name); tag != "" {
+		if strings.Contains(tag, ".") {
 			// Show the original name and not text potentially extracted config
-			// label.
-			return "", UNSPECIFIED, fmt.Errorf("config label should not contain '.', found %q", name)
+			// tag.
+			return "", UNSPECIFIED, fmt.Errorf("config tag should not contain '.', found %q", name)
 		}
 		// Ensure once the normalised extraction has happened, we put the prefix
-		// back on, so that we don't loose the fact that the config is a label.
+		// back on, so that we don't loose the fact that the config is a tag.
 		//
 		// Ideally we would change Config from map[string]Level to
 		// map[string]ConfigEntry and then we wouldn't need this step, but that
 		// causes lots of issues in Juju directly.
-		name = fmt.Sprintf("#%s", label)
+		name = fmt.Sprintf("#%s", tag)
 	}
 
 	levelStr := strings.TrimSpace(pair[1])
@@ -87,8 +87,9 @@ func parseConfigValue(value string) (string, Level, error) {
 // so "DEBUG" is equivalent to `<root>=DEBUG`
 //
 // An example specification:
+//
 //	`<root>=ERROR; foo.bar=WARNING`
-//	`[LABEL]=ERROR`
+//	`[TAG]=ERROR`
 func ParseConfigString(specification string) (Config, error) {
 	specification = strings.TrimSpace(specification)
 	if specification == "" {
@@ -111,7 +112,7 @@ func ParseConfigString(specification string) (Config, error) {
 	return cfg, nil
 }
 
-func extractConfigLabel(s string) string {
+func extractConfigTag(s string) string {
 	name := strings.TrimSpace(s)
 	if len(s) < 2 {
 		return ""

config_test.go

@@ -38,16 +38,16 @@ func (*ConfigSuite) TestParseConfigValue(c *gc.C) {
 		module: "",
 		level:  INFO,
 	}, {
-		value:  "#label = info",
-		module: "#label",
+		value:  "#tag = info",
+		module: "#tag",
 		level:  INFO,
 	}, {
-		value:  "#LABEL = info",
-		module: "#label",
+		value:  "#TAG = info",
+		module: "#tag",
 		level:  INFO,
 	}, {
-		value: "#label.1 = info",
-		err:   `config label should not contain '.', found "#label.1"`,
+		value: "#tag.1 = info",
+		err:   `config tag should not contain '.', found "#tag.1"`,
 	}} {
 		c.Logf("%d: %s", i, test.value)
 		module, level, err := parseConfigValue(test.value)

context.go

@@ -17,9 +17,9 @@ type Context struct {
 
 	// Perhaps have one mutex?
 	// All `modules` variables are managed by the one mutex.
-	modulesMutex       sync.Mutex
-	modules            map[string]*module
-	modulesLabelConfig map[string]Level
+	modulesMutex     sync.Mutex
+	modules          map[string]*module
+	modulesTagConfig map[string]Level
 
 	writersMutex sync.Mutex
 	writers      map[string]Writer
@@ -35,9 +35,9 @@ func NewContext(rootLevel Level) *Context {
 		rootLevel = WARNING
 	}
 	context := &Context{
-		modules:            make(map[string]*module),
-		modulesLabelConfig: make(map[string]Level),
-		writers:            make(map[string]Writer),
+		modules:          make(map[string]*module),
+		modulesTagConfig: make(map[string]Level),
+		writers:          make(map[string]Writer),
 	}
 	context.root = &module{
 		level:   rootLevel,
@@ -50,26 +50,26 @@ 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, labels ...string) Logger {
+func (c *Context) GetLogger(name string, tags ...string) Logger {
 	name = strings.TrimSpace(strings.ToLower(name))
 
 	c.modulesMutex.Lock()
 	defer c.modulesMutex.Unlock()
 
 	return Logger{
-		impl: c.getLoggerModule(name, labels),
+		impl: c.getLoggerModule(name, tags),
 	}
 }
 
-// GetAllLoggerLabels returns all the logger labels for a given context. The
+// GetAllLoggerTags returns all the logger tags for a given context. The
 // names are unique and sorted before returned, to improve consistency.
-func (c *Context) GetAllLoggerLabels() []string {
+func (c *Context) GetAllLoggerTags() []string {
 	c.modulesMutex.Lock()
 	defer c.modulesMutex.Unlock()
 
 	names := make(map[string]struct{})
 	for _, module := range c.modules {
-		for k, v := range module.labelsLookup {
+		for k, v := range module.tagsLookup {
 			names[k] = v
 		}
 	}
@@ -108,17 +108,27 @@ func (c *Context) getLoggerModule(name string, tags []string) *module {
 		// First tag wins when setting the logger tag from the config tag
 		// level cache. If there are no tag configs, then fallback to
 		// UNSPECIFIED and inherit the level correctly.
-		if configLevel, ok := c.modulesLabelConfig[tag]; ok && level == UNSPECIFIED {
+		if configLevel, ok := c.modulesTagConfig[tag]; ok && level == UNSPECIFIED {
 			level = configLevel
 		}
 	}
+
+	// As it's not possible to modify the parent's labels, it's safe to copy
+	// them at the time of creation. Otherwise we have to walk the parent chain
+	// to get the full set of labels for every log message.
+	labels := make(Labels)
+	for k, v := range parent.labels {
+		labels[k] = v
+	}
+
 	impl = &module{
-		name:         name,
-		level:        level,
-		parent:       parent,
-		context:      c,
-		tags:         tags,
-		labelsLookup: labelMap,
+		name:       name,
+		level:      level,
+		parent:     parent,
+		context:    c,
+		tags:       tags,
+		tagsLookup: labelMap,
+		labels:     parent.labels,
 	}
 	c.modules[name] = impl
 	return impl
@@ -132,7 +142,7 @@ func (c *Context) getLoggerModulesByTag(label string) []*module {
 			continue
 		}
 
-		if _, ok := mod.labelsLookup[label]; ok {
+		if _, ok := mod.tagsLookup[label]; ok {
 			modules = append(modules, mod)
 		}
 	}
@@ -172,18 +182,18 @@ func (c *Context) ApplyConfig(config Config) {
 	c.modulesMutex.Lock()
 	defer c.modulesMutex.Unlock()
 	for name, level := range config {
-		label := extractConfigLabel(name)
-		if label == "" {
+		tag := extractConfigTag(name)
+		if tag == "" {
 			module := c.getLoggerModule(name, nil)
 			module.setLevel(level)
 			continue
 		}
 
 		// Ensure that we save the config for lazy loggers to pick up correctly.
-		c.modulesLabelConfig[label] = level
+		c.modulesTagConfig[tag] = level
 
-		// Config contains a named label, use that for selecting the loggers.
-		modules := c.getLoggerModulesByTag(label)
+		// Config contains a named tag, use that for selecting the loggers.
+		modules := c.getLoggerModulesByTag(tag)
 		for _, module := range modules {
 			module.setLevel(level)
 		}
@@ -200,7 +210,7 @@ func (c *Context) ResetLoggerLevels() {
 		module.setLevel(UNSPECIFIED)
 	}
 	// We can safely just wipe everything here.
-	c.modulesLabelConfig = make(map[string]Level)
+	c.modulesTagConfig = make(map[string]Level)
 }
 
 func (c *Context) write(entry Entry) {

context_test.go

@@ -216,25 +216,25 @@ func (*ContextSuite) TestApplyConfigAdditive(c *gc.C) {
 		})
 }
 
-func (*ContextSuite) TestGetAllLoggerLabels(c *gc.C) {
+func (*ContextSuite) TestGetAllLoggerTags(c *gc.C) {
 	context := loggo.NewContext(loggo.WARNING)
 	context.GetLogger("a.b", "one")
 	context.GetLogger("c.d", "one")
 	context.GetLogger("e", "two")
 
-	labels := context.GetAllLoggerLabels()
+	labels := context.GetAllLoggerTags()
 	c.Assert(labels, gc.DeepEquals, []string{"one", "two"})
 }
 
-func (*ContextSuite) TestGetAllLoggerLabelsWithApplyConfig(c *gc.C) {
+func (*ContextSuite) TestGetAllLoggerTagsWithApplyConfig(c *gc.C) {
 	context := loggo.NewContext(loggo.WARNING)
 	context.ApplyConfig(loggo.Config{"#one": loggo.TRACE})
 
-	labels := context.GetAllLoggerLabels()
+	labels := context.GetAllLoggerTags()
 	c.Assert(labels, gc.DeepEquals, []string{})
 }
 
-func (*ContextSuite) TestApplyConfigLabels(c *gc.C) {
+func (*ContextSuite) TestApplyConfigTags(c *gc.C) {
 	context := loggo.NewContext(loggo.WARNING)
 	context.GetLogger("a.b", "one")
 	context.GetLogger("c.d", "one")
@@ -339,7 +339,7 @@ func (*ContextSuite) TestApplyConfigLabelsResetLoggerLevels(c *gc.C) {
 		})
 }
 
-func (*ContextSuite) TestApplyConfigLabelsAddative(c *gc.C) {
+func (*ContextSuite) TestApplyConfigTagsAddative(c *gc.C) {
 	context := loggo.NewContext(loggo.WARNING)
 	context.ApplyConfig(loggo.Config{"#one": loggo.TRACE})
 	context.ApplyConfig(loggo.Config{"#two": loggo.DEBUG})
@@ -353,7 +353,7 @@ func (*ContextSuite) TestApplyConfigLabelsAddative(c *gc.C) {
 		})
 }
 
-func (*ContextSuite) TestApplyConfigWithMalformedLabel(c *gc.C) {
+func (*ContextSuite) TestApplyConfigWithMalformedTag(c *gc.C) {
 	context := loggo.NewContext(loggo.WARNING)
 	context.GetLogger("a.b", "one")
 
@@ -371,7 +371,7 @@ func (*ContextSuite) TestApplyConfigWithMalformedLabel(c *gc.C) {
 		})
 }
 
-func (*ContextSuite) TestResetLoggerLevels(c *gc.C) {
+func (*ContextSuite) TestResetLoggerTags(c *gc.C) {
 	context := loggo.NewContext(loggo.DEBUG)
 	context.ApplyConfig(loggo.Config{"first.second": loggo.TRACE})
 	context.ResetLoggerLevels()

logger.go

@@ -22,13 +22,16 @@ type Logger struct {
 	labels Labels
 }
 
-// WithLabels returns a logger whose module is the same
-// as this logger and the returned logger will add the
-// specified labels to each log entry.
+// WithLabels returns a logger whose module is the same as this logger and
+// the returned logger will add the specified labels to each log entry.
+// WithLabels only target a specific logger with labels. Children of the logger
+// will not inherit the labels.
+// To add labels to all child loggers, use ChildWithLabels.
 func (logger Logger) WithLabels(labels Labels) Logger {
 	if len(labels) == 0 {
 		return logger
 	}
+
 	result := logger
 	result.labels = make(Labels)
 	for k, v := range labels {
@@ -85,6 +88,34 @@ func (logger Logger) ChildWithTags(name string, tags ...string) Logger {
 	return module.context.GetLogger(path, tags...)
 }
 
+// ChildWithLabels returns the Logger whose module name is the composed of this
+// Logger's name and the specified name with the correct associated labels.
+// Adding labels to the child logger will cause all child loggers to also
+// inherit the labels of the parent(s) loggers.
+// For targeting a singular logger with labels, use WithLabels which are not
+// inherited by child loggers.
+func (logger Logger) ChildWithLabels(name string, labels Labels) Logger {
+	module := logger.getModule()
+	path := module.name
+	if path == "" {
+		path = name
+	} else {
+		path += "." + name
+	}
+
+	merged := make(Labels)
+	for k, v := range logger.impl.labels {
+		merged[k] = v
+	}
+	for k, v := range labels {
+		merged[k] = v
+	}
+
+	result := module.context.GetLogger(path)
+	result.impl.labels = merged
+	return result
+}
+
 // Name returns the logger's module name.
 func (logger Logger) Name() string {
 	return logger.getModule().Name()
@@ -188,9 +219,11 @@ func (logger Logger) logCallf(calldepth int, level Level, message string, extraL
 	}
 	entry.Labels = make(Labels)
 	if len(module.tags) > 0 {
-		entry.Labels = make(Labels)
 		entry.Labels[LoggerTags] = strings.Join(module.tags, ",")
 	}
+	for k, v := range logger.impl.labels {
+		entry.Labels[k] = v
+	}
 	for k, v := range logger.labels {
 		entry.Labels[k] = v
 	}