open-telemetry / weaver

OTel Weaver lets you easily develop, validate, document, and deploy semantic conventions
Apache License 2.0
52 stars 19 forks source link

When resolving to stdout do not display status #184

Open MadVikingGod opened 4 months ago

MadVikingGod commented 4 months ago

Describe the bug When the output is stdout, the status lines (example below) will break any kind of stream processing, such as passing it to jq.

example:

✔ SemConv registry loaded (111 files)
✔ No policy found
✔ SemConv registry resolved

To Reproduce Steps to reproduce the behavior:

  1. Run weaver registry resolve -f json | jq .
  2. watch it fail because of the status printed at the beginning

Expected behavior The resolve output should be in the correct format, free of any status outputs.

Additional context These statuses are nice if we are outputting to a file or if they were on stderr.

lquerel commented 4 months ago

It's not ideal but weaver --quiet registry resolve -f json | jq . works.

However, I agree we should turn automatically quiet mode in this specific context. Thanks for reporting.

lquerel commented 4 months ago

--quiet is no longer required with #187 (not yet approved at this point of time).

MadVikingGod commented 4 months ago

Is there any way to promote this option to A) show up in the weaver registry resolve --help, and B) Be usable from the end of the command?

For B) weaver --quiet registry resolve works, but weaver registry resolve --quiet does not.

lquerel commented 4 months ago

Yes, it's definitely something to improve in the user experience. We use Clap for command-line parsing, and while there are some limitations, there are workarounds to ensure that both A and B are feasible.

lquerel commented 4 months ago

@MadVikingGod with #187 merged --quiet is no longer required.

However, I'd like to keep this issue open as I will make some changes to accept the --quiet argument in a more discoverable position.

lquerel commented 4 months ago

Fixed in #196 (pending approval).