Logger: Label loggers #40
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The following allows loggers to be tagged with labels, these labels can
be used when applying the configuration. With this change users of the
logging library can change the level of a series of loggers using the
tagged label.
The driver behind this is to better expose loggers with in Juju. If a
user wanted to see all http client connections, then a logger will
be tagged with `GetLogger('???', HTTP)`. Using the model-config
within Juju, it will be possible to set `[HTTP]=TRACE` and all HTTP
loggers will be set to the appropriate level.
---
Concerns:
1. No validation on namespaces for loggers are performed, other than
ensuring that ";" and "." are split. Meaning that "[]" for label syntax
might already exist in the wild?
2. What do we do about malformed "[", or "]" labels?
---
Alternative syntax for labels:
1. "<>"
2. "{}"
SimonRichardson
force-pushed
the
labelled-logger
branch
from
d39340f
to
30c7ae1
Compare
|
I feel like if we annotate loggers with labels, those labels should end up part of the log messages that they generate, so that it can be filtered early or late with the normal filtering mechanisms. (Like we do with |
To allow debug-log to see a label, we need to add Labels to loggo.Entry. When logging using Logf we just get the module and grab the labels if there are any.
|
@jameinel I've added labels to LogEntry, which will then allow |
achilleasa
reviewed
Jun 25, 2021
Parsing the label from the namespace was susceptible of a slight edge case, where the validation and the extraction might differ. This fixes that problem by using the same method for each.
|
!!build!! |
SimonRichardson
force-pushed
the
labelled-logger
branch
from
debcef2
to
813b4dd
Compare
jameinel
reviewed
Jun 28, 2021
The following changes move a label from `[LABEL]` to `#label`, which removes the syntax ambiguity. The old format looks like a slice/array so users might be inclined to do `[LABEL,OTHER]` and that becomes very confusing.
|
@jameinel Changed to |
To aid with discoverability we should be able to log all the labels for a given context.
When calling Child on a given logger, the method should pass through the labels so that any new logger is also created with the appropriate labels. Exposing Labels on a logger now allows us to ensure that the labels are indeed set correctly.
To prevent churn on the version of loggo, add a method so we keep backwards compatible.
SimonRichardson
force-pushed
the
labelled-logger
branch
from
e5ccd08
to
e360c26
Compare
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The following allows loggers to be tagged with labels, these labels can
be used when applying the configuration. With this change users of the
logging library can change the level of a series of loggers using the
tagged label.
The driver behind this is to better expose loggers within Juju. If a
user wanted to see all HTTP client connections, then a logger will
be tagged with
GetLogger('a.b.c', HTTP). Using the model-configwithin Juju, it will be possible to set
[HTTP]=TRACEand all HTTPloggers will be set to the appropriate level.
Concerns:
ensuring that ";" and "." are split. Meaning that "[]" for label syntax
might already exist in the wild?
Alternative syntax for labels:
See: #39