LokI Exporter - Adding a feature for loki exporter to encode JSON for log entry #3874

crearys · 2021-06-24T15:57:17Z

Description:

The PR creates a feature for the Loki Exporter that allows you to use a JSON representation of the LogRecord as described by https://developers.google.com/protocol-buffers/docs/proto3#json.

It creates a Loki LogEntry using JSON instead of the current body string.

The format option allows you to choose between json or the body string, the default is the body string.

This feature allows us to handle structured logging the Loki exporter and use structured logging in our logging pipeline.

Link to tracking Issue:
This may fix the issue: #2529

Testing:
TestConvert - Ensuring the json encoding is providing expected responses.

TestJsonLoadConfig - Ensuring the format config option is set correctly.

TestExporter_convertLogtoJsonEntry - Ensuring the Log Entry is converted to a JSON Entry.

Documentation:
Comments added to the functions and README options updated.

bogdandrutu · 2021-07-02T00:11:37Z

@gramidt please review

github-actions · 2021-07-09T05:18:54Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

gramidt · 2021-07-09T16:52:10Z

I apologize for the delay, @crearys and @bogdandrutu. I was out of the office on vacation and returned to a pile of things to catch up on. I will try to review this PR over the weekend.

crearys · 2021-07-12T10:18:34Z

@gramidt thanks

github-actions · 2021-07-20T05:18:24Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

gramidt · 2021-07-20T16:35:23Z

exporter/lokiexporter/exporter.go

@@ -135,7 +135,19 @@ func (l *lokiExporter) logDataToLoki(ld pdata.Logs) (pr *logproto.PushRequest, n
 					continue
 				}
 				labels := mergedLabels.String()
-				entry := convertLogToLokiEntry(log)
+				var entry *logproto.Entry
+				if l.config.Format == "json" {


Since the decision on format is not made at runtime, but rather at config time, what are your thoughts on making this decision early on and mapping the appropriate method ahead of time? This would remove the additional check per record.

@gramidt I have just pushed a new commit following your comments.

gramidt · 2021-07-28T13:03:32Z

@crearys - Looks like there's some linting and unit test issues. Could you address those?

gillg · 2021-07-28T13:15:03Z

Does anyone seen my comments ? Mostly about nested structures supported or not by loki with pipe search / grafana in vizualization panel. Are you sure it's ok ?

At otel-collector side can we map atttributes.attr1 to a label ?

EDIT: Sorry my review was never submitted ! 😅

gillg · 2021-06-29T23:06:25Z

exporter/lokiexporter/encode_json.go

+		TraceID:    lr.TraceID().HexString(),
+		SpanID:     lr.SpanID().HexString(),
+		Severity:   lr.SeverityText(),
+		Attributes: tracetranslator.AttributeMapToMap(lr.Attributes())}


As you created a full structure, why not add "Resources" ? Maybe conditionaly ?

gillg · 2021-06-29T23:11:58Z

exporter/lokiexporter/encode_json_test.go

+}
+
+func exampleJson() string {
+	jsonExample := `{"name":"name","body":"Example log","traceid":"01020304000000000000000000000000","spanid":"0506070800000000","severity":"error","attributes":{"attr1":"1","attr2":"2"}}`


To follow some logic and to be more explicit on loki side, why not prefix your arbitrary keys (name, body, traceid...) by someting like "otel_collector_lokiexporter_" ?
It can avoid confusion with keys in logs received by another channel than otel collector for exemple.

I would prefer use "otel.collector.lokiexporter." to be more consistent with OTEL but if you apply | json on it, loki replace all points by underscores anyway at the end...

gillg · 2021-06-29T23:15:21Z

exporter/lokiexporter/README.md

@@ -44,6 +44,8 @@ The following settings can be optionally configured:

 - `headers` (no default): Name/value pairs added to the HTTP request headers.

+- `format` (default = body): Set the log entry line format. This can be set to 'json' (the entire json encoded log record) or 'body' (the log record body field as a string).


I like it, finaly it's not too intrusive as potential big change !

gillg · 2021-06-29T23:19:52Z

exporter/lokiexporter/encode_json.go

+		TraceID:    lr.TraceID().HexString(),
+		SpanID:     lr.SpanID().HexString(),
+		Severity:   lr.SeverityText(),
+		Attributes: tracetranslator.AttributeMapToMap(lr.Attributes())}


What about nested structures on Grafana / Loki ?
In my memories I tested it and it was not working as expected. I'm not sure if loki is able to do something like {} | json | attributes.attr1 == "xxx" and on Grafana log pannel I think nested objects are not correctly rendered.

Does anyone can confirm ?
Do you think attributes could be flatened at json root (maybe with a key prefix) ?

Working on logs enriched by a resourcedetector processor, I confirm the Resources should be converted to structured logs data !
In theory some keys could be the same in attributes and resources (even if it doesn't makes sense...) so convertLogToLokiEntry should prefix them by something in my opinion.

crearys · 2021-07-29T12:05:41Z

@gillg To respond to all your comments. In grafana you can query the nested structures using {} | json | attributes_attr1="debug" . Around the flat JSON structure i think there needs to be a consensus on what json format we want to use. For this PR we had only considered encoding the log record, creating a flat json structure will need a different encoding method and could possibly be added as another format.

gillg · 2021-07-29T12:43:36Z

@gillg To respond to all your comments. In grafana you can query the nested structures using {} | json | attributes_attr1="debug" .

Ok, if I understand, loki makes a kind of native flattening on your nested JSON structure. So it "saves" the problem, but maybe it consumes more resources at query time ? @gramidt do you know if there is a notable impact ? Are we safe doing nothing at otel collector side and let's loki parse/flatten the json ?
Can we use the same notation on otel config ?

loki:
  labels:
    attributes:
      # Allowing 'severity' attribute and not providing a mapping, since the attribute name is a valid Loki label name.
      severity: ""
      http.status_code: "http_status_code" # here http.status_code is not a nested object but just a key

Around the flat JSON structure i think there needs to be a consensus on what json format we want to use. For this PR we had only considered encoding the log record, creating a flat json structure will need a different encoding method and could possibly be added as another format

As Loki takes keys of each nested structures and creates a flat structure with underscores we could do the same thing to help its work. It's just an idea.

And about "unprefixed" keys like name, body, severity, etc, it's less a problem if we keep them as it without a consensus.

github-actions · 2021-08-10T05:52:24Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-09-02T05:15:33Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-09-14T05:15:20Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-10-01T05:15:26Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

crearys · 2021-10-06T11:07:53Z

@gramidt updated for 0.36.0

gramidt · 2021-10-06T14:52:54Z

@slim-bean @crearys - Would either of you be interested in becoming the new owner of the Loki exporter? My priorities have shifted and I'm unable to allocate the necessary time to be a good maintainer.

gillg · 2021-10-11T15:17:28Z

A Loki exporter maintener is really needed :/
Probably a Loki core team member would be great to make the good choices according to Loki architecture.

Moreover with this PR we are probably not so far from a final good V1.

github-actions · 2021-10-19T05:15:22Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

bogdandrutu · 2021-10-19T15:47:56Z

@crearys @gillg @gramidt closing this since we don't have an owner of the component. Please find a good new owner and then reopen the PR.

crearys added 10 commits May 21, 2021 14:59

initial commit

491e6e1

revert un-needed changes

18d1808

test json encode as entry

403b6a1

add encoding files

4c695b7

merge loki encoding with the resource atttribute branch

caa4507

add format option to loki exporter config

071a13c

add test for loki/json test data

bb37818

improve variable names and comments

728f4bf

pull from upstream main and update loki exporter files to fix tests

bf30cc5

Create feature for loki exporter to encode json

bb253b3

crearys requested review from a team and Aneurysm9 June 24, 2021 15:57

github-actions bot assigned tigrannajaryan Jun 24, 2021

tigrannajaryan requested a review from gramidt June 28, 2021 17:04

tigrannajaryan assigned gramidt and unassigned tigrannajaryan Jun 28, 2021

github-actions bot added the Stale label Jul 9, 2021

github-actions bot removed the Stale label Jul 10, 2021

github-actions bot added the Stale label Jul 20, 2021

gramidt reviewed Jul 20, 2021

View reviewed changes

mapping convert functions

11d5439

gillg reviewed Jul 28, 2021

View reviewed changes

crearys added 3 commits July 29, 2021 11:30

Fix linting errors

fcdb38b

Merge remote-tracking branch 'upstream/main' into lokiencoding-pr

e01ed0f

update pdata package

ebdcccb

github-actions bot removed the Stale label Aug 1, 2021

github-actions bot added the Stale label Aug 10, 2021

bogdandrutu removed the Stale label Aug 16, 2021

github-actions bot added the Stale label Sep 2, 2021

bogdandrutu removed the Stale label Sep 3, 2021

github-actions bot added the Stale label Sep 14, 2021

bogdandrutu removed the Stale label Sep 22, 2021

crearys added 2 commits September 23, 2021 15:02

pull upstream

45a52eb

update to use otel 0.36

e80de58

github-actions bot added the Stale label Oct 1, 2021

bogdandrutu removed the Stale label Oct 6, 2021

github-actions bot added the Stale label Oct 19, 2021

bogdandrutu removed the Stale label Oct 19, 2021

bogdandrutu closed this Oct 19, 2021

gillg mentioned this pull request Oct 20, 2021

Loki Exporter - Adding a feature for loki exporter to encode JSON for log entry #5846

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LokI Exporter - Adding a feature for loki exporter to encode JSON for log entry #3874

LokI Exporter - Adding a feature for loki exporter to encode JSON for log entry #3874

crearys commented Jun 24, 2021

bogdandrutu commented Jul 2, 2021

github-actions bot commented Jul 9, 2021

gramidt commented Jul 9, 2021

crearys commented Jul 12, 2021

github-actions bot commented Jul 20, 2021

gramidt Jul 20, 2021

crearys Jul 22, 2021

gramidt commented Jul 28, 2021

gillg commented Jul 28, 2021 •

edited

Loading

gillg Jun 29, 2021

gillg Jun 29, 2021

gillg Jun 29, 2021

gillg Jun 29, 2021

gillg Oct 11, 2021

crearys commented Jul 29, 2021

gillg commented Jul 29, 2021

github-actions bot commented Aug 10, 2021

github-actions bot commented Sep 2, 2021

github-actions bot commented Sep 14, 2021

github-actions bot commented Oct 1, 2021

crearys commented Oct 6, 2021

gramidt commented Oct 6, 2021 •

edited

Loading

gillg commented Oct 11, 2021

github-actions bot commented Oct 19, 2021

bogdandrutu commented Oct 19, 2021

		@@ -44,6 +44,8 @@ The following settings can be optionally configured:

		- `headers` (no default): Name/value pairs added to the HTTP request headers.

		- `format` (default = body): Set the log entry line format. This can be set to 'json' (the entire json encoded log record) or 'body' (the log record body field as a string).

LokI Exporter - Adding a feature for loki exporter to encode JSON for log entry #3874

LokI Exporter - Adding a feature for loki exporter to encode JSON for log entry #3874

Conversation

crearys commented Jun 24, 2021

bogdandrutu commented Jul 2, 2021

github-actions bot commented Jul 9, 2021

gramidt commented Jul 9, 2021

crearys commented Jul 12, 2021

github-actions bot commented Jul 20, 2021

gramidt Jul 20, 2021

Choose a reason for hiding this comment

crearys Jul 22, 2021

Choose a reason for hiding this comment

gramidt commented Jul 28, 2021

gillg commented Jul 28, 2021 • edited Loading

gillg Jun 29, 2021

Choose a reason for hiding this comment

gillg Jun 29, 2021

Choose a reason for hiding this comment

gillg Jun 29, 2021

Choose a reason for hiding this comment

gillg Jun 29, 2021

Choose a reason for hiding this comment

gillg Oct 11, 2021

Choose a reason for hiding this comment

crearys commented Jul 29, 2021

gillg commented Jul 29, 2021

github-actions bot commented Aug 10, 2021

github-actions bot commented Sep 2, 2021

github-actions bot commented Sep 14, 2021

github-actions bot commented Oct 1, 2021

crearys commented Oct 6, 2021

gramidt commented Oct 6, 2021 • edited Loading

gillg commented Oct 11, 2021

github-actions bot commented Oct 19, 2021

bogdandrutu commented Oct 19, 2021

gillg commented Jul 28, 2021 •

edited

Loading

gramidt commented Oct 6, 2021 •

edited

Loading