Added config options to define json fields and line_format for loki logs#1210
Added config options to define json fields and line_format for loki logs#12100x49b wants to merge 8 commits intotektoncd:mainfrom
Conversation
|
|
I need to test this locally with LokiStack operator. It looks like a good feature. |
aThorp96
left a comment
There was a problem hiding this comment.
This seems like a good change to me. Thanks for the contribution!
A couple asks:
- This is configured using a vendor-agnostic variable
LOGGING_PLUGIN_LINE_FORMAT. It makes sense to me that this feature should be implemented for the other logging plugin formats supported byLOGS_TYPEor at least issues created to port support to the other log types (I believe the full list of log types are Loki, GCS, S3, "blob" (same as S3?), and Splunk). - Please also add unit tests and documentation (docs/logging-supprot.md) for the new configuration options. It's worth noting in that documentation that
LOGGING_PLUGIN_JSON_MAPhas no effect on the logging output unless used in conjunction withLOGGING_PLUGIN_LINE_FORMAT.
Along a similar lines, since I am not very familiar with Loki: since there's the json key=value component to the query, does LOGGING_PLUGIN_LINE_FORMAT work when used without LOGGING_PLUGIN_JSON_MAP, or do they have to be used together in all cases?
|
@aThorp96 Thank you for your review. I will add tests and documentation for these two config variables. I can add Feature Requests for the other log formats besides Loki to have that feature for the others too, but I'm not really able to help because I do not know them (GCS, S3, Splunk, etc) well enough. The LOGGING_PLUGIN_LINE_FORMAT only works if there are fields set in LOGGING_PLUGIN_JSON_MAP. With the documentation I will add, this should be clear for users that they work in conjunction. |
…ments' into feature/configurable-logql-statements
|
I tested this and it works fine with operator. |
|
Can you please squash these commits? I will give lgtm then/ |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
chore added documentation for new loki config variables test: added tests for new configuration fields docs: fixed example for LOGGING_PLUGIN_JSON_MAP
…b.com/0x49b/results into feature/configurable-logql-statements
|
@0x49b Can you please sign easy cla? And squash the commits? |
|
EasyCLa for me (0x49b) is done, but the one for @bhanuprasad14 is not. Not a Colleague of mine... Squash I will do |
Changes
Added configuration variables to define a json map to extract fields and to define the line_format of Loki logs. By passing a string with json in
LOGGING_PLUGIN_JSON_MAPlike{"timestamp": "[\"@timestamp\"]"}the json field timestamp gets extracted into timestamp what then can be used to define a custom line_format inLOGGING_PLUGIN_LINE_FORMATlike"{{.timestamp}}: {{.message}}"Mentioned in #1204
/kind feature
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you review them:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes