Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename logging exporter to 'stdout' or 'debug' exporter in Collector #3524

Closed
alolita opened this issue Jun 29, 2021 · 17 comments
Closed

Rename logging exporter to 'stdout' or 'debug' exporter in Collector #3524

alolita opened this issue Jun 29, 2021 · 17 comments

Comments

@alolita
Copy link
Member

alolita commented Jun 29, 2021

The logging exporter should be renamed to stdout to represent its function. The current name 'logging' is misleading since this is a stdout exporter used mainly for debugging purposes.

Other similar examples of such an exporter can be found in the Go SDK. See https://github.com/open-telemetry/opentelemetry-go/tree/main/exporters/stdout

@alolita alolita added enhancement New feature or request release:required-for-ga Must be resolved before GA release area:exporter labels Jun 29, 2021
@alolita alolita added this to the Phase2-GA-Roadmap milestone Jun 29, 2021
@alolita
Copy link
Member Author

alolita commented Jun 29, 2021

Related to #3474

@bogdandrutu
Copy link
Member

The main difference is that we actually write to the Collector's own logger not to stdout.

@sincejune
Copy link
Contributor

Do we still need to rename the logging exporter? I think I can take it if the answer is yes.

@rakyll
Copy link
Contributor

rakyll commented Jul 8, 2021

I think "debug" is a better name, there is another thread where they are discussions not to write to stdout but to stderr, or give users options.

@alolita
Copy link
Member Author

alolita commented Jul 14, 2021

Action Item: @alolita will reach out to other Go community members to comment on this issue.

@alolita
Copy link
Member Author

alolita commented Jul 22, 2021

@open-telemetry/go-approvers can you please comment on your preference for this renaming of the existing logger exporter to debug exporter?

@MrAlias
Copy link

MrAlias commented Jul 22, 2021

I would recommend not renaming to stdout, we have preserved this name in OTel-go for continuity, but realized that since we provide configuration for where the output can go (i.e. not STDOUT) it is a poor name.

I think debug is a good name if a rename is desired.

@alolita
Copy link
Member Author

alolita commented Jul 29, 2021

@bogdandrutu can we review and merge the outstanding PR and complete this task? Please advise.

@alolita
Copy link
Member Author

alolita commented Aug 6, 2021

@bogdandrutu @tigrannajaryan can you please review and merge this change? There are no objections so far from the community.

@tigrannajaryan
Copy link
Member

I posted my objection here #3609 (review)

I don't see any new information to change my opinion.

@alolita
Copy link
Member Author

alolita commented Sep 2, 2021

@tigrannajaryan this issue was discussed in the Collector SIG and it was agreed upon by the community present there to change to 'debug' for long term maintainability.

@pellared
Copy link
Member

pellared commented Sep 2, 2021

Personally, I do not find debug really better than logging. The same term could be used e.g. as a logging level. I imagine that it can be misleading as well: "debug the collector" may mean to use Delve as well as setting a debug exporter... Maybe internalLogger?

Moreover, it is a breaking change that should be avoided and I do not see any strong reason to do it. I do not see confidence or any strong opinions to change the name. I feel that changing the name can cause more harm than benefits.

@tigrannajaryan
Copy link
Member

@tigrannajaryan this issue was discussed in the Collector SIG and it was agreed upon by the community present there to change to 'debug' for long term maintainability.

@alolita What was the community's justification? I it would be nice to capture it here for posterity. For now I don't see how "logging" is more difficult to maintain than "debug", but probably there are other reasons in favour of the change which I am not aware of.

I definitely do not mind changing if there is a reasonable justification, but I don't see that justification so far. :-)

One argument we should probably use is consistency with SDKs.

Unfortunately the SDKs already are inconsistent:

It may be worth bringing this up in the maintainers or specification SIG and seek for a consistent naming across all language SDKs and Collector. I will be happy to go with whatever consistent name we choose, I don't have a strong preference for a particular name.

@alolita
Copy link
Member Author

alolita commented Sep 9, 2021

@tigrannajaryan responding to your questions:

Community feedback so far has been:

  • Using good practice for Go which is stdout
  • Removing confusion for users since 'logging' is confusing given the project also supports 'logs'
  • Community members voicing 'debug' as a good option

Why not set an example for the Collector for being consistent on naming conventions and help avoid user misunderstanding of the logging component usage :-) I like the idea of discussing this in the maintainers meeting - will add to agenda.

The opportunity is now to make this change before we go stable for the Collector.

@alolita
Copy link
Member Author

alolita commented Sep 15, 2021

@tigrannajaryan @bogdandrutu let's deprioritize this change if you are not in agreement as maintainers. Please advise. Ty!

@tigrannajaryan
Copy link
Member

I submitted an issue on the spec: open-telemetry/opentelemetry-specification#1931
Will also add to Maintainer's SIG agenda.

@alolita alolita added release:allowed-for-ga and removed release:required-for-ga Must be resolved before GA release labels Sep 17, 2021
@tigrannajaryan
Copy link
Member

Maintainer's SIG decided that there it is hard to come up with a consistent name given different naming conventions in different languages. Since we do not expect consistency I believe the name we have is as good as we can have.

I am closing this issue. Feel free to reopen if you have other arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants