This repository has been archived by the owner on Dec 6, 2024. It is now read-only.
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.
Versioning and Stability for OpenTelemetry Clients #143
Versioning and Stability for OpenTelemetry Clients #143
Changes from 38 commits
f427d74
50db1aa
a4d6432
a001ee6
40d4dae
a848e3a
680d1ff
f880b88
70786fe
b5b9255
7a62f27
7f24f1d
bdb150a
f818d11
d1624dc
0ee804e
a3384e0
9669b23
48d7f4f
0d03cf8
ccb0cc6
f87ee1f
2d52f5c
6f7fe0f
55ddbfb
9e0284b
53df880
a5e9192
bacf18d
1fbaf30
c335a51
02fa9c7
e90aec5
d69fc1a
4a13c0a
fe35fdb
f7a2c04
4337fd8
86cbe2b
2f7396b
8e04de1
6a2a78f
594e380
1da65ef
a71d8b5
ca647dc
66e58e4
eb39b89
00b57c7
a602397
04a3f15
a4e027a
8184d15
e560725
0919fa7
5b93dc7
92335ad
31a2cfa
93622e5
9938dd5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I would consider a "library developer" to include libraries that implement SDK extensions, such as custom exporters. Perhaps this could be simplified to just "instrumentation should never directly reference SDK packages" which I believe is the intent.
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.
Thanks, I clarified this further.
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.
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.
If the API can make breaking changes when it increases its major version, presumably the SDK must also have a breaking change, which isn't mentioned here.
I think you could simplify this section by talking about "Code Stability" vs "Semantic Conventions Stability". The same rules for backwards-compatibility should apply to all kinds of code.
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.
🤷♀️ People keep asking what guarantees specifically apply to semantic conventions.
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.
Why isn't ABI compatibility required? Do we expect some level of dependency hell to reduce the maintenance burden of contrib a little?
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.
After some discussion, I've changed ABI compatibility into a language-specific concern. It is too difficult to make a broad pronouncement on this nuanced subject.
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.
There isn't any time-period described here between deprecation and removal, which makes deprecation somewhat moot. I can simply replace old with new, and cut a new major version without deprecating. If this is binding in some form (e.g. through a minimum "deprecation period"), we should spell that out. If not, make it clearer that this is an optional, be-a-nice-community sort of thing.
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.
So, I don't want to arbitrarily pick a minimum deprecation period, and no one has pointed to any kind of convention or standard to follow. The current deprecation period should be considered "infinite" for the time being.
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.
I'm not quite following. IIUC, an infinite deprecation period would imply we can't remove signals, even with a new major version, which doesn't align with the rest of the proposal.
Here is what I would suggest as a definition for deprecation:
"A deprecated signal is a signal which will be removed in the next major version, but is identical to a stable signal in all other respects. Marking a signal deprecated does not imply a new major version must be cut, but guarantees that it will be removed if/when a new major version is cut. All signals which are removed in a new major version must be marked deprecated in the old major version before releasing the new major version--no cherrypicking deprecations after-the-fact, and no removing signals which are not deprecated. Marking a signal deprecated is intended to smooth the transition to a new major version by encouraging users to move off of the deprecated signal prior to upgrading to the next major version. A user who has moved off of all deprecated signals will not encounter any compilation errors when upgrading to the new major version."
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.
This implies there is always a replacement for deprecated functionality. Are we saying there will never be a signal we want to remove without replacement?
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.
I think the idea is to actually remove those components way down the road, just not right away (we did this in OpenTracing, removing APIs after deprecating them, and there were a few complains).
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.
Yeah, I can't see a reason for us removing stable features just because we decide we don't like them any more. In those cases, we just leave them alone and let them be.
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.
Agreed. The only process by which we should remove signals is by introducing a new major version. We should also put a high price on new major versions, which pushes removal of signals way down the road.
I just don't want to imply that major version releases, which can remove signals, will have feature-parity with previous major versions.
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.
In the hypothetical world in which we cut a new major version, and there was a security vulnerability, I think we would want to patch the latest minor version in both major versions. It would be hard to force customers to upgrade to a new major version to get a security fix, as major version upgrades will be disruptive by definition.
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.
So, I agree that back-porting across major versions seems both reasonable and likely. But how many major versions? I would rather not pick something arbitrary, and leave it open for the future when we come to it. We can always add this clarification later.
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.
In my doc I had proposed all major versions that have had a minor release in the past year, but starts to get into the LTS discussion, which may be too much for this proposal. It might be easier to start with something optional, like "it is up to maintainers to decide whether to support additional minor versions, or to support minor versions from previous major versions"