winston-loki
winston-loki copied to clipboard
Remove default labels
Motivation
Labels in Loki should be added carefully and kept to a minimum, since it directly impacts how logs are stored and performance see here. They shouldn't be dynamically added based on log content by default. Also, in the current implementation labels and level are not added to the log line, which can be an issue when using a custom format.
Description
- Remove the default
level
,label
andlabels
label and change the default format to a json representation of theinfo
object. This enables the user in Loki to then use the| json
parser to extract those labels if needed. This also solves the issue with custom formats. - Fix the tests
Limit
The current drawback is that there is no way to add a label dynamically anymore. If that's a must have feature then I can add another option, to specify a list of keys from the log entry that should be extracted into labels (similarly to the official fluentbit output for example).
Let me know what you think and thanks for putting this adapter together :)
Seems like a good idea to make the adapter be more idiomatic towards Loki! This would become a new major release with some, if not all, changes presented here, together with a fix to https://github.com/JaniAnttonen/winston-loki/issues/90#issuecomment-1036359951 and possibly some other issues :)
Seems like a good idea to make the adapter be more idiomatic towards Loki! This would become a new major release with some, if not all, changes presented here, together with a fix to #90 (comment) and possibly some other issues :)
Let me know how I can support you here ;)
According to current official recommendations, dynamic labels are ok as long as the scope of potential values is limited, but level
is particularly recommended not to put in labels. Having an option to specify an array or a Set (any iterator) of keys from the log entry that should be extracted into labels seems really nice!
UPD: Removing level
from labels may not be a good idea for now for esthetic purposes: with the label, we can easily filter error messages by the level. Without the label, we need to filter by word "ERROR", and for now it looks ugly, and there is nothing we can do with it
I agree with removing the log level, but leaving it in as a default for backward compatibility. This label should not be forced on users as they should want to keep their label cardinality to a minimum.
@IppX
Limit
The current drawback is that there is no way to add a label dynamically anymore. If that's a must have feature then I can add another option, to specify a list of keys from the log entry that should be extracted into labels (similarly to the official fluentbit output for example).
There is a way to add labels dynamically, each log line can have labels added to them individually. Not sure why you would want this unless it was a very small loki deployment in charge of a small amount of apps.
There is a way to add labels dynamically, each log line can have labels added to them individually.
This PR breaks that feature that's why I added it under a limit section.
Not sure why you would want this unless it was a very small loki deployment in charge of a small amount of apps.
I agree, that's why I don't think it's a deal breaker to remove this feature. Plus Loki can do this on the query side.