search

elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
1.22k stars 24.85k forks source link

`Elasticsearch exited unexpectedly` is not JSON #96996

Open DaveCTurner opened 1 year ago

DaveCTurner commented 1 year ago

When running in certain environments we expect that the output to stderr and stdout comprises JSON logs, with timestamps and so on. But if Elasticsearch exits then we log the bare string ERROR: Elasticsearch exited unexpectedly. Can we make this JSON? Can we include a timestamp? It'd be awfully useful to know when exactly the exit occurred.

elasticsearchmachine commented 1 year ago

Pinging @elastic/es-core-infra (Team:Core/Infra)

mosche commented 10 months ago

Can we make this JSON?

The output format for stdout / stderr is defined based on the log4j2 layout used for the console appender (log4j2.properties), this can obviously vary between environments. Cli tools are currently not using log4j2, instead we're directly writing to stout / stderr (based on a Terminal implementation). No matter how we change the output format in cli tools, it'll always be inconsistent in some environments unless based on the same configuration.

If we consider this to be really important, we could think about a terminal that writes to stdout / stderr through log4j. Though, cli tools / the launcher were designed with minimal dependencies in mind. That would obviously change then.

Can we include a timestamp?

Just including a timestamp is certainly trivial and easy to do independent of the previous point.

mark-vieira commented 10 months ago

If we consider this to be really important, we could think about a terminal that writes to stdout / stderr through log4j. Though, cli tools / the launcher were designed with minimal dependencies in mind. That would obviously change then.

We should consider if this is possible. Doing something ad-hoc is going to mean trying to detect if JSON output for stdout/stderr is enabled. Since this is currently controlled by log4j properties, there's no way the CLI tools will know this without a) parsing those properties or b) introducing some other signal (system prop, environment variable, etc). If we go with (a) we might as well just use log4j in the CLI tools and (b) will undoubtedly get out of sync given all the different packaging types we have and logging config overrides.

rjernst commented 10 months ago

I think it's a little more complicated than just using log4j within the CLI tools. It's not just about the messages the CLI outputs directly. The CLI has to capture and pass through stderr currently to look for special signals from the ES process (eg ready signal). The JDK can sometimes output things to stderr (maybe stdout too?) that are plain text. The CLI already sees that plain text, but just passes it through unaware. But other output from the process may already be in json, so we can't just jsonify everything output to stderr/stdout.

As far as outputting json from the cli, while we do already configure logging, I'm not sure we want to complicate Terminal with log4j. Terminal has it's own concepts for log levels, which are not the same as general logging. We also don't need full featured json, we have very limited needs here.

For detecting when to output in JSON, we already know when we are in the Docker packaging, which is when we output logs to JSON, so we can use the same thing in selecting which Terminal output type to use in CliToolLauncher.

mark-vieira commented 10 months ago

For detecting when to output in JSON, we already know when we are in the Docker packaging, which is when we output logs to JSON, so we can use the same thing in selecting which Terminal output type to use in CliToolLauncher.

Right now this is the case and 1:1 but that's not necessarily guaranteed to always be so. Probably "good enough" if going full log4j is impractical though.

mosche commented 10 months ago

@rjernst I've pushed a draft PR (see above), let me know if the general approach makes sense to you. The new output obviously breaks a lot of assumptions in the packaging tests. If you're good to proceed that way I'll tackle those next.

pgomulka commented 9 months ago

could we use System.Logger in the CLI tools? In CLI it would default to console logger (and perhaps create a json style console System.Logger) but if log4j2 configuration is present it woudl return log4j2 impl? this would only fix the dependency problem. Not sure how to work around the problem with log4j configuration being loaded later..