Provide stronger symbolic stability guarantees #432
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@bogdandrutu @Aneurysm9 @dyladan @MrAlias @carlosalberto @ocelotl @aabmass @jsuereth please review. |
bogdandrutu
reviewed
Oct 18, 2022
bogdandrutu
reviewed
Oct 18, 2022
Yes. I updated the PR description to make the goal clear. |
|
I support this definition. I believe it reflects the consensus resulting from discussion on #400 and also what the user community expects from OTLP. |
9 tasks
|
My opinion here is that this extra set of stability restrictions / guarantees on ourselves just alleviates downstream consumption friction. I don't think we need to allow ourselves extra flexibility here that seems to be contested. I do agree that generated code should not be exposed as stable, but instead hidden behind interfaces. HOwever, that's a downstream concern we shouldn't enforce here. I'm in favor of this change and would approve if it wasn't a Draft. |
|
Here is the PR that only adds guarantees necessary for OTLP/JSON: #435 |
tigrannajaryan
added a commit
to tigrannajaryan/opamp-spec
that referenced
this pull request
Jan 5, 2023
The guarantees mostly mirror the proposal for OTLP (open-telemetry/opentelemetry-proto#432), the primary difference being that OpAMP has no gRPC services defined and does not need any guarantees for service definitions.
tigrannajaryan
added a commit
to tigrannajaryan/opamp-spec
that referenced
this pull request
Jan 5, 2023
The guarantees mostly mirror the proposal for OTLP (open-telemetry/opentelemetry-proto#432), the primary difference being that OpAMP has no gRPC services defined and does not need any guarantees for service definitions. Resolves open-telemetry#134
tigrannajaryan
added a commit
to tigrannajaryan/opentelemetry-proto
that referenced
this pull request
Mar 23, 2023
This PR explicitly lists the additive changes allowed and adds a requirement that such additive changes must be accompanied by interoperability explanation when necessary. This is a subset of open-telemetry#432 that contains other guarantees that we did not yet agree to. I believe this particular subset is necessary regardless of what we decide about open-telemetry#432.
tigrannajaryan
added a commit
that referenced
this pull request
Apr 20, 2023
This PR explicitly lists the additive changes allowed and adds a requirement that such additive changes must be accompanied by interoperability explanation when necessary. This is a subset of #432 that contains other guarantees that we did not yet agree to. I believe this particular subset is necessary regardless of what we decide about #432.
tigrannajaryan
force-pushed
the
feature/tigran/define-stability
branch
from
bf866ac
to
217a212
Compare
tigrannajaryan
force-pushed
the
feature/tigran/define-stability
branch
from
217a212
to
9cba87a
Compare
tigrannajaryan
changed the title
|
@open-telemetry/specs-approvers I updated this PR to make it clear what this is about: it provides stronger symbolic stability guarantees (without making any claims about the behavior of the code generator). I think this is one the last major remaining issues that we need to agree on before we can mark this repo 1.0. Please review and comment. Particularly I need opinions whether you think we should provide the stronger symbolic guarantees or not. If we disagree with the idea then the whole PR is pointless and should be discarded. Please bring arguments in favour of your position to help make a decision. |
7 tasks
jsuereth
approved these changes
Apr 20, 2023
Aneurysm9
approved these changes
Apr 21, 2023
mx-psi
approved these changes
Apr 21, 2023
jack-berg
approved these changes
Apr 21, 2023
jack-berg
reviewed
Apr 21, 2023
jmacd
approved these changes
Apr 21, 2023
carlosalberto
approved these changes
Apr 24, 2023
tedsuo
approved these changes
Apr 25, 2023
arminru
approved these changes
Apr 26, 2023
dyladan
approved these changes
Apr 26, 2023
|
The TC re-reviewed this and the final outcome was to go ahead. Thanks for the reviews and sorry for the delay! |
tigrannajaryan
added a commit
to tigrannajaryan/opentelemetry-proto
that referenced
this pull request
May 8, 2023
We merged the open-telemetry#432 without clarifying when exactly the guarantees start applying. The intent was that the guarantees are needed for 1.0 release but we didn't make this explicit. We are now making this explicit and until 1.0 released we should be free to fix the remaining known issues listed here: open-telemetry#456 Once 1.0 release is made we can remove the [from 1.0.0] labels from the README.md.
|
A follow-up PR to make it clear that symbolic guarantees go into effect only when 1.0 is released: #467 |
bogdandrutu
pushed a commit
that referenced
this pull request
May 9, 2023
We merged the #432 without clarifying when exactly the guarantees start applying. The intent was that the guarantees are needed for 1.0 release but we didn't make this explicit. We are now making this explicit and until 1.0 released we should be free to fix the remaining known issues listed here: #456 Once 1.0 release is made we can remove the [from 1.0.0] labels from the README.md.
tigrannajaryan
added a commit
to tigrannajaryan/opamp-spec
that referenced
this pull request
May 15, 2023
The guarantees mostly mirror the proposal for OTLP (open-telemetry/opentelemetry-proto#432), the primary difference being that OpAMP has no gRPC services defined and does not need any guarantees for service definitions. Resolves open-telemetry#134
tigrannajaryan
added a commit
to open-telemetry/opamp-spec
that referenced
this pull request
May 30, 2023
The guarantees mostly mirror the proposal for OTLP (open-telemetry/opentelemetry-proto#432), the primary difference being that OpAMP has no gRPC services defined and does not need any guarantees for service definitions. Resolves #134
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
This PR explicitly lists the additive changes allowed and adds a requirement that such additive changes must be accompanied by interoperability explanation when necessary. This is a subset of open-telemetry#432 that contains other guarantees that we did not yet agree to. I believe this particular subset is necessary regardless of what we decide about open-telemetry#432.
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
…metry#467) We merged the open-telemetry#432 without clarifying when exactly the guarantees start applying. The intent was that the guarantees are needed for 1.0 release but we didn't make this explicit. We are now making this explicit and until 1.0 released we should be free to fix the remaining known issues listed here: open-telemetry#456 Once 1.0 release is made we can remove the [from 1.0.0] labels from the README.md.
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
This PR explicitly lists the additive changes allowed and adds a requirement that such additive changes must be accompanied by interoperability explanation when necessary. This is a subset of open-telemetry#432 that contains other guarantees that we did not yet agree to. I believe this particular subset is necessary regardless of what we decide about open-telemetry#432.
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
…metry#467) We merged the open-telemetry#432 without clarifying when exactly the guarantees start applying. The intent was that the guarantees are needed for 1.0 release but we didn't make this explicit. We are now making this explicit and until 1.0 released we should be free to fix the remaining known issues listed here: open-telemetry#456 Once 1.0 release is made we can remove the [from 1.0.0] labels from the README.md.
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
This PR explicitly lists the additive changes allowed and adds a requirement that such additive changes must be accompanied by interoperability explanation when necessary. This is a subset of open-telemetry#432 that contains other guarantees that we did not yet agree to. I believe this particular subset is necessary regardless of what we decide about open-telemetry#432.
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
…metry#467) We merged the open-telemetry#432 without clarifying when exactly the guarantees start applying. The intent was that the guarantees are needed for 1.0 release but we didn't make this explicit. We are now making this explicit and until 1.0 released we should be free to fix the remaining known issues listed here: open-telemetry#456 Once 1.0 release is made we can remove the [from 1.0.0] labels from the README.md.
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
This PR explicitly lists the additive changes allowed and adds a requirement that such additive changes must be accompanied by interoperability explanation when necessary. This is a subset of open-telemetry#432 that contains other guarantees that we did not yet agree to. I believe this particular subset is necessary regardless of what we decide about open-telemetry#432.
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
VinozzZ
pushed a commit
to VinozzZ/opentelemetry-proto
that referenced
this pull request
Jun 21, 2024
…metry#467) We merged the open-telemetry#432 without clarifying when exactly the guarantees start applying. The intent was that the guarantees are needed for 1.0 release but we didn't make this explicit. We are now making this explicit and until 1.0 released we should be free to fix the remaining known issues listed here: open-telemetry#456 Once 1.0 release is made we can remove the [from 1.0.0] labels from the README.md.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #400
What this PR aims to guarantee stronger backward compatibility of symbols in .proto files. This means that if the same generator is used then most likely the generated code will be backward compatible, i.e. upgrading to a newer version of generated code should not break the build of a codebase that consumes the generated code and should not change the behavior (applies both to senders and receivers).
We do not guarantee the behavior of the generator itself. The notice about this remains in our README.
I think this is the last major remaining issue that we need to agree on before we can mark this repo 1.0.
Please review and comment. Particularly I need opinions whether you think we should provide the stronger symbolic guarantees or not. If we disagree with the idea then the whole PR is pointless and should be discarded.
Please bring arguments in favour of your position to help make a decision.