Open Telemetry Console Exporter Json output

[meijeran]

Fixes #5036
Design discussion issue #

Changes

Please provide a brief description of the changes here.

This is an attempt to solve #5036, I switched the current output to a json variant.
I am not sure if we want to keep the original output as well and add the json variant as an extra option. According the comments in #4548 only one output format is highly favored.

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[CodeBlanch]

I definitely think some options/levers are going to be needed.

• The primary use case today for ConsoleExporter is dev/debug/test. The main thing we need for this is readability. I don't think JSON is too bad but some have suggested YAML might be better. Or a very simple text format. We could have options to pick one with the most simple/easy to read as the default. We don't have to start with the options or multiple formats those could be added later but I would be interested to see a comparison for the change her vs what we have today.

• The use case users seem to be asking for on the issue is for production output to use with scraping of stdout. This is a very different thing. I think for this we need the JSON to be compact (single-line per entry). Also it should probably use JSON Protobuf Encoding (example: trace.json). We probably also need to improve the performance to make it production ready but that doesn't need to happen on this PR. Do we even want the ConsoleExporter to be this thing? Or perhaps could we have a StandardOutputExporter in contrib or something which is purpose built for this scenario? We could document ConsoleExporter is only supported for dev/debug/test and link to the other thing for users who want to scrape. Just an idea 🤔

So my question is really which scenario is this for? Does this really fix/resolve the issue?

[cijothomas]

ConsoleExporter is explicitly not recommended for production scenarios. There is no spec written for ensuring consistent output. And yes, performance is another concern.

For scraping the stdout, a better option is to use collector, and have it output to stdout.
Edit : collectors output is also not fixed format.

[cijothomas]

Can you share couple of screenshots of the new behavior as well?

[meijeran]

So currently it looks like this

[reyang]

Is this JSON format the same as the proto JSON format or a custom one? https://opentelemetry.io/docs/specs/otlp/#json-protobuf-encoding

[codecov]

Codecov Report

Attention: Patch coverage is 73.68421% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 85.85%. Comparing base (6250307) to head (f5bd25b).
Report is 222 commits behind head on main.

❗ Current head f5bd25b differs from pull request most recent head 55fa273. Consider uploading reports for the commit 55fa273 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5588      +/-   ##
==========================================
+ Coverage   83.38%   85.85%   +2.47%     
==========================================
  Files         297      265      -32     
  Lines       12531    11321    -1210     
==========================================
- Hits        10449     9720     -729     
+ Misses       2082     1601     -481     

Flag	Coverage Δ
unittests	?
unittests-Solution-Experimental	85.86% <73.68%> (?)
unittests-Solution-Stable	85.69% <73.68%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...emetry.Exporter.Console/ConsoleActivityExporter.cs	75.75% <73.68%> (+27.48%)	⬆️

... and 112 files with indirect coverage changes

[meijeran]

It is not the proto json format is it just a serialized Activity

[cijothomas]

I definitely think some options/levers are going to be needed.

• The primary use case today for ConsoleExporter is dev/debug/test. The main thing we need for this is readability. I don't think JSON is too bad but some have suggested YAML might be better. Or a very simple text format. We could have options to pick one with the most simple/easy to read as the default. We don't have to start with the options or multiple formats those could be added later but I would be interested to see a comparison for the change her vs what we have today.
• The use case users seem to be asking for on the issue is for production output to use with scraping of stdout. This is a very different thing. I think for this we need the JSON to be compact (single-line per entry). Also it should probably use JSON Protobuf Encoding (example: trace.json). We probably also need to improve the performance to make it production ready but that doesn't need to happen on this PR. Do we even want the ConsoleExporter to be this thing? Or perhaps could we have a StandardOutputExporter in contrib or something which is purpose built for this scenario? We could document ConsoleExporter is only supported for dev/debug/test and link to the other thing for users who want to scrape. Just an idea 🤔

So my question is really which scenario is this for? Does this really fix/resolve the issue?

@meijeran Could you reply to the above comment? We are trying to see the issues attempted to be resolved with this PR, so as to better review/guide!

[meijeran]

@cijothomas Great questions!

The main reason I started this change is because of issue #5036 and some comments from this discussion. There were a few requests to standardize the console output, so this proposal is just an idea to see if we can make that work.

If the goal is to scrape this data, I think having a dedicated exporter in contrib would be the way to go instead of relying on this. But I'm also curious why the other exporter options aren't enough for that.

So, to answer your question, I'm not entirely sure if this fixes the issue completely, but it's something people have been asking for. Would love to hear more thoughts on this!