open-telemetry / opentelemetry-dotnet

The OpenTelemetry .NET Client
https://opentelemetry.io
Apache License 2.0
3.17k stars 752 forks source link

Console Exporter use YAML by default #5372

Open reyang opened 7 months ago

reyang commented 7 months ago

Feature Request

The console exporter is currently using a custom output format which has the following issues:

  1. It is ad-hoc, each time new things get added, they are added in inconsistent ways.
  2. It is unstable, folks cannot rely on the output format since things can change/break.
  3. It might be confusing, e.g. if the attribute value is a string which has trailing spaces, it is hard to see from the output. The same thing applies to complex data types and multi-line strings.

Describe the solution you'd like:

The output format is YAML by default. The user can opt-in and switch to OTLP JSON format.

YAML is for readability.

OTLP JSON can be used by line based text processing tools.

Additional Context

martinjt commented 6 months ago

Why would YAML be the default and not JSON? Are there examples of other exporters doing YAML as an output?

I can understand potentially having YAML as an option, but from a "pit of success" perspective JSON should definitely be the default.

I am genuinely torn, since no-one should be writing to JSON and reading it using a processing tool. So we're not focusing on performance here and that means JSON as a default is possibly wrong.

Either way, assuming YAML as a default, when other console exporters won't do that I think is wrong. We'd need something custom for things like Auto-instrumentation to turn it on as, although I disagree, some people still want their data written to stdout in k8s.

utpilla commented 6 months ago

Why would YAML be the default and not JSON? Are there examples of other exporters doing YAML as an output?

YAML is a more human-readable. JSON is best suited for programmatic processing of the output. ConsoleExporter is mainly targeted for human developers to easily and quickly verify that their SDK setup is working as expected.

YAML is a commonly used format for printing to console. Check Azure CLI for example.

martinjt commented 6 months ago

I was asking for examples outside of Microsoft, and specifically within the OTel ecosystem.

I'm not sure any of the other SDKs' console exporters have YAML as the default, so I don't think .NET should either. That's an assumption though, so I think there needs to be evidence that this is what all the SDKs are moving to as a default.

I'm not saying that we shouldn't have it as an option. It's the default part that's the issue.

On Fri, 23 Feb 2024, 00:06 Utkarsh Umesan Pillai, @.***> wrote:

Why would YAML be the default and not JSON? Are there examples of other exporters doing YAML as an output?

YAML is a more human-readable. JSON is best suited for programmatic processing of the output. ConsoleExporter is mainly targeted for human developers to easily and quickly verify that their SDK setup is working as expected.

YAML is a commonly used format for printing to console. Check Azure CLI https://learn.microsoft.com/en-us/cli/azure/format-output-azure-cli?tabs=bash for example.

— Reply to this email directly, view it on GitHub https://github.com/open-telemetry/opentelemetry-dotnet/issues/5372#issuecomment-1960540662, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM66A2K2XKYFJSTJ6OG4X3YU7MRVAVCNFSM6AAAAABDRV5LEOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRQGU2DANRWGI . You are receiving this because you commented.Message ID: @.***>

utpilla commented 6 months ago

I was asking for examples outside of Microsoft, and specifically within the OTel ecosystem. I'm not sure any of the other SDKs' console exporters have YAML as the default, so I don't think .NET should either. That's an assumption though, so I think there needs to be evidence that this is what all the SDKs are moving to as a default. I'm not saying that we shouldn't have it as an option. It's the default part that's the issue.

The spec is not particular about the format in which the data has to be printed to console. The decision to choose default format should be based on the primary use case of the exporter. Since the main use case for ConsoleExporter is for human developers to quickly test things, YAML is better suited as the default format than JSON or the existing custom format.

There are examples of using YAML format outside of Microsoft as well. For e.g. AWS CLI.

martinjt commented 6 months ago

I'm asking for examples with OpenTelemetry, or even just telemetry tools. The .NET OpenTelemetry code base shouldn't be breaking from what the other libraries do, whether that's in the spec or not.

A readable, parsed/templated/summarised, output should probably be "the default" but failing that, it should be JSON to match the majority of other languages, and the wider expectations around people piping their telemetry to the console for log parser to pick it up.

I've been through the other languages, Javascript, Python, Rust, C++, PHP, Go, Swift... In fact the ones that don't are .NET, Erlang, and Java. So unless there is an edict coming from the TC that this should be moved to YAML as a default for the languages, this should not be progressed as the default, rather, we should focus on replicating the path the majority of the other languages are doing.

I'd suggest that this is taken by @reyang to the wider TC to provide official guidance, or state here that it's official guidance in their TC member capacity, otherwise, it should be treated as a single user request, not an established and accepted pattern that's been discussed (unless I missed it being brought up on the SIG I missed?)

martinjt commented 6 months ago

In light of the above, I've removed help-wanted for now, until there is an actual decision on whether this would be accepted. The decision should come in the form of a SIG discussion, or a TC/OTEP mandate, not a single person's opinion (and I'm including myself in that, I can't make the decision alone, neither should any single other member of the project).

martinjt commented 5 months ago

This was discussed on the SIG today and the outcome was that JSON should be the default, with the option to use YAML.

We discussed various issues, and although there is no spec guidance on the formats, there is prior art in all the other languages that use JSON right now. Although the structure isn't consistent, they do provide JSON which allows for established use cases like those that are considered "best practice" in kubernetes right now. We do not want to promote this approach, and in the docs, we'll likely set the exporter to YAML.

That said, no one is actively working on this right now.

I'll create a new ticket with then JSON default, close this one and link it to the new ticket along with the other issues in the repo asking for JSON.