elastic / ecs-logging-java

https://www.elastic.co/guide/en/ecs-logging/java/current/intro.html
Apache License 2.0
141 stars 75 forks source link

Markers are not included as key-value pairs #191

Closed MitchelNijdam-Rockstars closed 2 years ago

MitchelNijdam-Rockstars commented 2 years ago

I was trying to add a custom Marker to the JSON output of the EcsEncoder in Spring Boot (using Slf4J & Logback) using the following command:

log.info(
        append("my_custom_metric", incidentCount),
        "Collecting incident metrics, # of incidents: {}",
        incidentCount);

Expecting the following JSON output:

{ "message":"Collecting incident metrics, # of incidents: 0", "tags":[{"my_custom_metric": 0}] }

However, I got this non-useful value instead:

{ "message":"Collecting incident metrics, # of incidents: 0", "tags":["LS_APPEND_OBJECT"] }

Checking the source code, the EcsEncoder.java is using just marker.getName(), which in my eyes is the most useless value to use from the Marker. Isn't the whole idea of the markers that you can add your own, custom values to the Json output? I believe we should be using marker.getFieldName() and marker.getFieldValue() as key value pair in the output, not the name of the marker. Am I missing something?

https://github.com/elastic/ecs-logging-java/blob/2526531d35c9ed583864ddb8fd789262bdf2e1cb/logback-ecs-encoder/src/main/java/co/elastic/logging/logback/EcsEncoder.java#L159-L161

MitchelNijdam-Rockstars commented 2 years ago

After thinking about it over the weekend, I realised that it makes sense that something called a "marker" is not more than a single value - as implemented.

However I still do wonder what the use is for the fieldName and fieldValue is when it gets ignored, especially as the official documentation says one should use it like my example.

felixbarny commented 2 years ago

marker.getFieldName() and marker.getFieldValue() as key value pair in the output, not the name of the marker. Am I missing something?

The thing is that the getFieldName and getFieldValue methods are not part of the org.slf4j.Marker interface but are part of logstash-logback-encoder. But adding support for these markers does sound like a good idea. See also https://github.com/elastic/ecs-logging-java/issues/49

MitchelNijdam-Rockstars commented 2 years ago

Ah indeed, I was looking at the specific implementations of Marker. I think my question is answered then, and #49 should be sufficient to capture my request. Thanks!