opentelemetry-proto
opentelemetry-proto copied to clipboard
open-telemetry
Remove helper masks, not part of the protocol
Fixes https://github.com/open-telemetry/opentelemetry-proto/issues/363
The reason is to keep the proto interface as minimal as possible, and the helper masks are just helpers, and not part of the protocol. They can be defined by the language SIGs in their usages, but don't need to be part of the proto stability.
Signed-off-by: Bogdan Drutu [email protected]
Yeah, I agree - these helper numbers help ensure there isn't language implementation drift. I would vote to keep them.
May help, but not part of the protocol and guarantees. I am definitely not going to accept this as part of our guarantees of stability.
How can FLAG_NO_RECORDED_VALUE not be considered part of a stability guarantee? Are you saying that you'd like to reserve the right to change the name of the symbol so that NO_RECORDED_VALUE could be renamed? The meaning of this field has to be considered stable IMO.
The meaning of this field has to be considered stable IMO.
100% agree
Are you saying that you'd like to reserve the right to change the name of the symbol so that NO_RECORDED_VALUE could be renamed?
YES
I wonder what other @open-telemetry/specs-approvers think about usefulness of these enums.
My preference would be to keep them, but it is not a strong opinion.
Also, we should get rid of the zero values: LOG_RECORD_FLAG_UNSPECIFIED and FLAG_NONE since they encourage poor practice. The correct usage is to always do bitwise AND with the right bit, never compare to the 0 value.
I'm w/ @tigrannajaryan in that I think having them here is generally helpful and helps us avoid (obvious) breaking changes in their definition as well as gives folks "one pane of glass" to look at.
I'm w/ @tigrannajaryan in that I think having them here is generally helpful and helps us avoid (obvious) breaking changes in their definition as well as gives folks "one pane of glass" to look at.
This is a protocol not a "helper" library. I don't think these constants belong to the protocol, similar to a standard like w3c trace context, does not require/define/encourage a constant for 0x1 for the sampling bit, it is who is generating the code responsibility if they want to provide helpers.
I disagree with the effort to protect users from being foolish. I support the idea of removing zero-valued enums, but not removing non-zero values. The FLAG_NO_RECORDED_VALUE has disappeared, which I do not support. Users who use bitmasks should know what they are doing and/or read documents.
Discussed in TC meeting and decided that https://github.com/open-telemetry/opentelemetry-proto/pull/461 should be sufficient and we can close this issue.