[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I'm supportive of this effort, but if we're removing the plaintext format, I think it's worth sending this out as an RFC.

In D107079#2913760 <https://reviews.llvm.org/D107079#2913760>, @teemperor wrote:

> Some general notes:
>
> - Why JSON? In LLVM we have pretty much just JSON and YAML as formats available (well, and the bitcode format I guess). YAML is a much more complicated format and the main idea here is to let people write tools, so JSON seemed like an obvious pick.

The YAML I/O library is also relatively slow, which would hurt the overhead of enabling logging (even more).

> - Will there be an upstream log viewer? I don't know, but we could easily put a simple one on the website that just formats a log as a simple HTML table. That is easily done in vanilla JavaScript. But that should be a follow up patch.

Personally I'd also want a script to view the logs in the terminal. If we write it in Python we can ship it with lldb.

> - Do we want to keep the old plain text format too? To be honest, I don't really see much

Me neither on the condition that users can convert the logs to some kind of plain text format (part of the motivation for my previous comment.)

> - What about logs that are printed to stdin and we want to parse? I think you can easily make a tool that filters out the non-JSON output. Logs are guaranteed to have every object on its own line, so you can just go line-by-line and filter them based on that.
>
> Some planned follow ups:
>
> - Enable more logging meta-information by default. Things such as pid/tid seems like obvious cheap choices.

+1, if the data is structured we should always emit it (unless there's some large associated cost)

> - For people that really like the old format, it should be a simple patch to add a converter that just takes a JSON file and outputs the old plaintext.
>
> - I want to remove the ability to turn off thread-safe logging. It seems like it could potentially crash LLDB but it would certainly break the JSON parser. It's usually (and luckily) enabled by default anyway.
>
> - Some more meta information would be actually helpful sometimes and there isn't any reason to not add them. E.g., classifying messages as error/warning/info.

+1 on everything here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107079/new/

https://reviews.llvm.org/D107079