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

Clarify maturity guarantees #225

Conversation

tigrannajaryan
Copy link
Member

This clarifies what is and is not part of the promises.

This clarifies what is and is not part of the promises.
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-approvers please review.

@arminru arminru requested review from a team October 19, 2020 16:56
Copy link
Member

@Oberon00 Oberon00 left a comment

Choose a reason for hiding this comment

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

I'm only a specs-trace-approver, so not sure if my approval formally counts, but LGTM!

README.md Outdated Show resolved Hide resolved
field, nor enum names of Protobuf messages are visible on the wire and are not
considered part of the guarantees. We are free to make change to the names.

In the future when OTLP/JSON is declared stable, field names will also become part of
Copy link
Member

Choose a reason for hiding this comment

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

I think OTLP/JSON should just always be left unstable. I think the cost/benefit ratio to declaring this stable is just not right. Are there any strong reasons for supporting JSON at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

Short answer: yes, we want it, but I am not sure we want to open this discussion here in this PR. OTLP/JSON is already part of the specification: https:/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/otlp.md#request

Copy link
Member

Choose a reason for hiding this comment

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

I think @Oberon00 has good arguments (probably more in his mind). Would be good to have this discussion, also may be good to discuss if JSON should be based on these protos or we defined it separately.

@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/field-name-guarantees branch from 8947d65 to b85e351 Compare October 20, 2020 15:27
@bogdandrutu bogdandrutu merged commit 69d265b into open-telemetry:master Oct 20, 2020
@tigrannajaryan tigrannajaryan deleted the feature/tigran/field-name-guarantees branch October 21, 2020 13:36
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.

7 participants