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

Update status of stdout exporter for logs to stable #3922

Merged
merged 6 commits into from
Mar 12, 2024

Conversation

cijothomas
Copy link
Member

https://github.com/open-telemetry/opentelemetry-specification/pull/3741/files and https://github.com/open-telemetry/opentelemetry-specification/pull/3740/files were added together, and it is not clear why one is experimental, and the other is stable.

This PR is marking the logging stdout exporter spec as stable. I couldn't find any reason why it was non-stable in the first place, so I assume it was just an oversight only. Happy to be corrected, if I missed something

Originally part of #3901

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Reiley Yang <[email protected]>
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need to clarify the output format of this exporter unspecified before we stabilize it?

@cijothomas
Copy link
Member Author

Do you think we need to clarify the output format of this exporter unspecified before we stabilize it?

I don't see the need, as this is already stable for other signals, and none of them has a output format specified.

However, I do support the idea of spec-ing out the output format, and apply it for all signals. We should be able to do it in back-compat way (like opt-in to the new format) so don't have to block this.
(or since output format is not specified, it maybe okay to specify one without breaking spec's back-compat, and leave it upto individual implementations to decide if they want to opt-in to new format, etc.)

@MrAlias
Copy link
Contributor

MrAlias commented Mar 11, 2024

However, I do support the idea of spec-ing out the output format, and apply it for all signals. We should be able to do it in back-compat way (like opt-in to the new format) so don't have to block this.
(or since output format is not specified, it maybe okay to specify one without breaking spec's back-compat, and leave it upto individual implementations to decide if they want to opt-in to new format, etc.)

Hmm, this seems to be highlighting my underlying concern. We need to be transparent up-front that the output format is unspecified and that we won't ever try to standardize this format.

If we standardize our output format we will be specifying a data structure additional to the OTLP. We should instead require that user depend on that protocol if they want a stable and standardized data format.

I think we should go as far to say that language implementations are required to document their exporters as not providing a interoperable output and that that output itself is not guaranteed to be stable.

@cijothomas
Copy link
Member Author

However, I do support the idea of spec-ing out the output format, and apply it for all signals. We should be able to do it in back-compat way (like opt-in to the new format) so don't have to block this.
(or since output format is not specified, it maybe okay to specify one without breaking spec's back-compat, and leave it upto individual implementations to decide if they want to opt-in to new format, etc.)

Hmm, this seems to be highlighting my underlying concern. We need to be transparent up-front that the output format is unspecified and that we won't ever try to standardize this format.

If we standardize our output format we will be specifying a data structure additional to the OTLP. We should instead require that user depend on that protocol if they want a stable and standardized data format.

I think we should go as far to say that language implementations are required to document their exporters as not providing a interoperable output and that that output itself is not guaranteed to be stable.

Does any of these need to block this PR, given span,metrics stdout exporter spec is already stable ?

@MrAlias
Copy link
Contributor

MrAlias commented Mar 12, 2024

Does any of these need to block this PR, given span,metrics stdout exporter spec is already stable ?

If omitting this kind of information was intentional in the trace and metric exporters, than I think we could proceed. However, I'm guessing not having this for those two signals was unintentional and just an oversight. We should try to address this prior to stabilizing so we don't make the same mistake a third time.

@cijothomas
Copy link
Member Author

Does any of these need to block this PR, given span,metrics stdout exporter spec is already stable ?

If omitting this kind of information was intentional in the trace and metric exporters, than I think we could proceed. However, I'm guessing not having this for those two signals was unintentional and just an oversight. We should try to address this prior to stabilizing so we don't make the same mistake a third time.

I don't think it was an oversight, as #3565 existed well before https://github.com/open-telemetry/opentelemetry-specification/pull/3740/files was merged (as stable).

@MrAlias
Copy link
Contributor

MrAlias commented Mar 12, 2024

I don't think it was an oversight, as #3565 existed well before https://github.com/open-telemetry/opentelemetry-specification/pull/3740/files was merged (as stable).

Gotcha 👍, SGTM

@reyang reyang merged commit 6cd1112 into open-telemetry:main Mar 12, 2024
7 checks passed
@cijothomas cijothomas deleted the cijothomas/log-stdout-stable branch March 12, 2024 21:56
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants