OpenSearchJsonLayout logs newlines in json

[svenso]

Describe the bug Using the OpenSearchJsonLayout for logging out of a docker container adds newlines in the generated json-string when a stack-trace is appended. This makes it unnecessarily hard to process the logs by a line based log-agent.

To Reproduce Steps to reproduce the behavior: Use the following log4j2 config:

appender.console.type = Console
appender.console.name = console
appender.console.layout.type = OpenSearchJsonLayout
appender.console.layout.type_name = server

rootLogger.level = info
rootLogger.appenderRef.console.ref = console

which generates logs like this:

{ ... "cluster.uuid": "KK8EloHiQGCbVCUO3RPkmg", "node.id": "HTVcLMDtRLSgVqq2w5l0UA" ,
"stacktrace": ["java.lang.NullPointerException: Cannot invoke \"String.length()\" because \"other\" is null",
"at org.apache.lucene.search.spell.LevenshteinDistance.getDistance(LevenshteinDistance.java:62)"
...
"at java.lang.Thread.run(Thread.java:833) [?:?]"] }

Expected behavior Not adding unnecessary newlines.

The newlines are injected here: https://github.com/opensearch-project/OpenSearch/blob/10bff0c9f5b9ca78b3dc50f5c704dabf41b9d535/server/src/main/java/org/opensearch/common/logging/JsonThrowablePatternConverter.java

Is there any reason for those or can they get removed?

Host/Environment (please complete the following information):

• OS: Ubuntu 22.04 (docker)

[dblock]

I am wild-guessing that this was added for readability of the logs when debugging. Not sure how we can have it both ways 🤔

[svenso]

That's true. But usually, JSON is not the best format, if you plan to read it as is.

Other solutions might be, that we implement flags like in JSONLayout of log4j2 (compact=true for example), but I understand, that this is a quite manual job (due to missing jackson-databind library & security considerations we cannot use JSONLayout directly as far as I understand).

[dblock]

Feel free to propose something non-breaking @svenso. I can relate to the wanting line-by-line readable/processable logs.

[saratvemulapalli]

I was just chatting @dbwiddis, we came up with idea of a configurable setting which spits out logs in machine readable format or human readable format. As @dblock suggested do you have a proposal to move the needle here?

[svenso]

I will have a look at it. But for me, the main question stays: if someone is switching from the default (readable) PatternLayout to OpenSearchJsonLayout, do we really need to keep a "readability"? But I am also not quite happy, how the JSON is generated (string concatenation). Let me test some things and I'll come back.

[svenso]

I made a first draft with some optimizations how it'd be useful for our usecase. It should be back compatible to the old behavior when no configuration is used.

I am not a java developer and it's the first time I am contributing in an opensource project... I hope I didn't do everything wrong...

[dbwiddis]

Hey @svenso you submitted a draft PR #4783 some time ago which looks good. Can you make it ready for review so we can incorproate it?

[minalsha]

@svenso could you please address above comment from @dbwiddis ? Thanks

[svenso]

@dbwiddis Sorry for the delay. To be honest, I do not exactly know, how to proceed concerning the failed checks.. What's my next step? How to make it ready for review?

[dblock]

@svenso Let's have any discussions on the PR in https://github.com/opensearch-project/OpenSearch/pull/4783

[PeterKnopp-ETECTURE]

Hello, is there any ongoing work on this topic?

I would appreciate the requested changes in our environment, too. We are using opensearch in a kubernetes cluster and watch our logs in opensearch-dashboard. Logs with Exception send from the opensearch server are not readable there because the lines are shown in random order.

I agree to the opinion of @svenso that when choosing the JSON-format you don't expect good readability on STDOUT.