-
Notifications
You must be signed in to change notification settings - Fork 274
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
Declare OTLP/JSON Stable #436
Declare OTLP/JSON Stable #436
Conversation
@tigrannajaryan I take for granted #363 is not a hard requirement for this? |
No, it is not because we prohibit using enum value names in JSON. We require using numeric values. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tigrannajaryan this should wait first for the specification to be released per our guidance correct? Anything that is specified in the specification repo becomes the rule once it is released.
Once we are ready (both PRs have enough approvals) we can merge both PRs at the same time. |
Since I don't feel that strong. But we have a rule that "unrealeased" specification should not be used :))
After open-telemetry/opentelemetry-specification#2930 is merged the "protocol" part of the protobufs is stable, but what about the helpers (flag values (or mask values, unclear though)) that are not defined by the protocol, are these stable? What does it mean for JSON encoding? Update: If we keep them, what are the guarantees for these fields since they are not put on the wire, so definitely they will always be "wire compatible". |
Helpers are not visible anywhere on the wire (we intentionally disallowed enum values as strings to make sure enum names don't end on wire), so I think the fact that we have helpers or don't have helpers has no impact for JSON encoding. Do you see this differently? |
@bogdandrutu does this answer your question? Any other concerns or we can move forward with this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bogdandrutu does this answer your question? Any other concerns or we can move forward with this?
@tigrannajaryan you are correct, my comment is important for 1.0 but not for JSON stability.
This is ready to be merged after 1.18 spec release. |
See also open-telemetry/opentelemetry-proto#436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
Merging as 1.18.0 was released and open-telemetry/opentelemetry-specification#2930 was merged. |
See also open-telemetry#436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
See also open-telemetry#436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
See also #436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
See also open-telemetry/opentelemetry-proto#436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
See also open-telemetry#436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
See also open-telemetry#436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
See also open-telemetry#436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
See also open-telemetry#436 Co-authored-by: Carlos Alberto Cortez <[email protected]>
See also open-telemetry/opentelemetry-specification#2930