PatternJsonProvider: Inconsistent handling of configuration error

[brenuart]

The behaviour of the PatternJsonProvider when it is given an invalid pattern differs depending on the type of error. Here are a few examples:

(1) Invalid conversion word in Pattern layout

The following JSON pattern refers to the log message using the %message conversion word instead of %msg:

{ "message: "%message" } 

The following error status is emitted when the provider is started but no exception is thrown:

09:58:13,556 |-ERROR in ch.qos.logback.core.pattern.parser.Compiler@4a87761d - There is no conversion class registered for conversion word [message]
09:58:13,556 |-ERROR in ch.qos.logback.core.pattern.parser.Compiler@4a87761d - [message] is not a valid conversion word

The provider is started and produces the following JSON output when serialising log events:

{"message":"%PARSER_ERROR[message]"}

(2) Syntax error in Pattern Layout

The following JSON pattern contains a syntax error (conversion word missing after %):

{ "message": "%" }

This is causing an ERROR status with an exception when the provider is started:

10:29:04,107 |-ERROR in ch.qos.logback.classic.PatternLayout("%") - Failed to parse pattern "%". ch.qos.logback.core.spi.ScanException: Unexpected end of pattern string
	at ch.qos.logback.core.spi.ScanException: Unexpected end of pattern string
	at 	at ch.qos.logback.core.pattern.parser.TokenStream.tokenize(TokenStream.java:119)
	at 	at ch.qos.logback.core.pattern.parser.Parser.<init>(Parser.java:68)
...

Unlike the previous case, the message field is now populated at runtime with an empty string instead of a "%PARSE_ERROR" special value:

{"message": ""}

(3) Invalid JSON pattern

The PatternJsonProvider accepts a valid JSON string for its "pattern" property. If the supplied value is not a valid JSON, an ERROR status is logged and the provider defaults to an empty pattern, producing nothing at runtime.
Example (missing closing quote):

{ "message": "%message }

Produces the following ERROR when the provider is started:

10:32:55,479 |-ERROR in net.logstash.logback.composite.loggingevent.LoggingEventPatternJsonProvider@536aaa8d - [pattern] is not a valid JSON object

And the following JSON is produced at runtime:

{}

According to me, replacing the pattern with an error marker like %PARSE_ERROR is not a good option. This will produce a valid JSON output at runtime in most cases making it difficult to notice something went wrong with the configuration. Even worst if it is wrapped with a #asLong operation like this: { "age": "#asLong(%relative)" }. The operation will receive the "%PARSE_ERROR" string and will fail to convert it to a long defaulting to null.

IMHO, the best strategy is to log an ERROR status in all cases and don't produce anything at runtime. This is the behaviour adopted by most of the other JsonProvider when their configuration is wrong: they simply revert producing nothing.

@philsttr What's your opinion?

[philsttr]

the best strategy is to log an ERROR status in all cases and don't produce anything at runtime.

This works for me as long as the implementation isn't too complex, and logstash-logback-encoder can easily detect errors. logback itself doesn't complain about invalid patterns until they are used, so I'm not sure how easy it will be to detect them.