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

chore: Link to spec versioning doc #1737

Merged
merged 2 commits into from
Jun 3, 2021

Conversation

dyladan
Copy link
Member

@dyladan dyladan commented Dec 9, 2020

Creating a JS versioning proposal as requested in open-telemetry/opentelemetry-specification#1267

/cc @tedsuo

@codecov
Copy link

codecov bot commented Dec 9, 2020

Codecov Report

Merging #1737 (e3104ab) into main (975abda) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1737      +/-   ##
==========================================
+ Coverage   92.79%   92.81%   +0.01%     
==========================================
  Files         144      144              
  Lines        5178     5178              
  Branches     1062     1062              
==========================================
+ Hits         4805     4806       +1     
+ Misses        373      372       -1     
Impacted Files Coverage Δ
...emetry-core/src/platform/node/RandomIdGenerator.ts 93.75% <0.00%> (+6.25%) ⬆️

@obecny
Copy link
Member

obecny commented Dec 9, 2020

lgtm.
One thing - I'm missing some information about how long certain version will be supported - minimum period. For example what if we release version 2.x how long will the version 1.x be still supported. Perhaps this will be taken from "general agreement" from all Sigs ?

@dyladan
Copy link
Member Author

dyladan commented Dec 9, 2020

lgtm.
One thing - I'm missing some information about how long certain version will be supported - minimum period. For example what if we release version 2.x how long will the version 1.x be still supported. Perhaps this will be taken from "general agreement" from all Sigs ?

The only word we have from @tedsuo is "years". They don't expect to go to 2.0 any time soon.

RELEASING.md Outdated

- The `examples/` and `getting-started/` folders are not part of lerna packages, we need to manually bump the version in `package.json`.

## Versioning
Copy link
Member

Choose a reason for hiding this comment

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

overall i'm fine (plus the modification discussed about the SIG), could we add a link to this inside the readme ?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, remove experiential, use version 0, new package for new (significant?) experimental changes, don't rev the API version for every change (unless the API explicitly changes)

Copy link
Member Author

Choose a reason for hiding this comment

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

link to this inside the readme

yes

plus the modification discussed about the SIG

114d83c (#1737)

RELEASING.md Outdated

#### V1.0.0 Release (tracing, baggage, propagators, context)

- `@opentelemetry/[email protected]`
Copy link
Member Author

Choose a reason for hiding this comment

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

According to the versioning OTEP, they want each signal to have its own API package. This package would then just depend on the stable API packages and re-export them. What do we think of that idea?

@open-telemetry/javascript-maintainers @open-telemetry/javascript-approvers

Copy link
Member

Choose a reason for hiding this comment

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

I don't really see the advantages of doing this apart for more work when releasing ?

Copy link
Member

@obecny obecny Dec 10, 2020

Choose a reason for hiding this comment

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

for sure I would split api, about the rest hard to say, for using on daily basis I would rather have them together in one repo and keep the version aligned (all except api).

Copy link
Member Author

Choose a reason for hiding this comment

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

for sure I would split api

@obecny you would have a separate package for each trace, context, metrics, etc? Or do you just mean metrics would be split?

What if we only have separate API packages and remove the "main" API package?

Copy link
Member

Choose a reason for hiding this comment

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

if we can have only api seperate and the rest that would be the easiest, otherwise I would have:

  • api
  • metrics (until it is stable)
  • all the rest

RELEASING.md Outdated

#### Immature or experimental signals

API modules for experimental signals will not be included in the `@opentelemetry/api` module, and must be installed manually. API modules will be named with an `-experimental` suffix to make it abundantly clear that depending on them is at your own risk. For example, the `@opentelemetry/metrics-api-experimental` module will provide experimental access to the unfinished metrics API. NO STABILITY GUARANTEES ARE MADE.
Copy link
Member

Choose a reason for hiding this comment

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

I thought we agreed to remove the -experimental suffix at the SIG meeting. It looks like the "versioning" section was updated, but this might have been missed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@MSNev
Copy link
Contributor

MSNev commented Dec 10, 2020

Based on comments and against the main OTEP and looking at what Java is doing, it doesn't appear to be that the usage of additional "suffixes" 1.0.0-alpha is explicitly "banned" but rather the issue was called out based on language specific "tools" that don't fully support semver 2.0.

While I agree with the general sentiment of using 0.x.y for experimental (for the initial situation we are in), I think we should also consider including the "-alpha" / "-beta" suffixes especially moving forward for adding additional changes to existing packages, rather than creating a brand new and starting @ v 0.x.y (for adding future changes) as I believe this will get us back into the same situation and reason that we are dropping the "-expermential" suffix on the package name.

Thoughts?

@dyladan
Copy link
Member Author

dyladan commented Dec 10, 2020

Based on comments and against the main OTEP and looking at what Java is doing, it doesn't appear to be that the usage of additional "suffixes" 1.0.0-alpha is explicitly "banned" but rather the issue was called out based on language specific "tools" that don't fully support semver 2.0.

While I agree with the general sentiment of using 0.x.y for experimental (for the initial situation we are in), I think we should also consider including the "-alpha" / "-beta" suffixes especially moving forward for adding additional changes to existing packages, rather than creating a brand new and starting @ v 0.x.y (for adding future changes) as I believe this will get us back into the same situation and reason that we are dropping the "-expermential" suffix on the package name.

Thoughts?

Can you give an example of a time in which we would have the choice between a -alpha version tag and introducing a new package with 0.x.y version?

The idea seems to be predicated on new features relating to mature signals, which the spec and TC have explicitly stated will not happen.

@MSNev
Copy link
Contributor

MSNev commented Dec 10, 2020

Assuming that we are going to be able to release a "perfect" version across all packages is quite simply not realistic...

We WILL need to add new (non-breaking) features to existing packages. And I believe that the simple act of creating the next version violates your statement, whether that is a minor bump or major.

Or are we declaring that we are only EVER going to release v1.0.x and NEVER release any "pre" (alpha/beta) package for future versions (containing new integrations or features)? Yes, I am specifically ignoring this statement "We currently have no plans for deprecating signals or creating a major version past v1.0." as this is a short-term view and in my view will get violated not long after we GA (or possible before as part of considering adding packages)

So I'm going to flip your question around and ask "Why would you want to create a new package for the next version of an existing package?" (as I believe this assertion takes us back to the -experiential situation again)

Example from Trasks's comment on the main OTEP.
"for experimental (non-GA) instrumentation artifacts, the preference is to use version suffix, e.g. 1.15.0-alpha for any experimental artifacts released as part of 1.15.0 release train."

@MSNev
Copy link
Contributor

MSNev commented Dec 16, 2020

Based on the updated OTEP, the usage of suffixes is now the suggestion, unless not supported by the language/build environments, with GO the only called out instance. Do we have anyone in the JS community that is using a build environment that would barf if we used suffixes?

@dyladan
Copy link
Member Author

dyladan commented Dec 17, 2020

Do we have anyone in the JS community that is using a build environment that would barf if we used suffixes?

Not that I'm aware of.

Base automatically changed from master to main January 25, 2021 19:26
@vmarchaud
Copy link
Member

Is that still required @dyladan ?

@dyladan
Copy link
Member Author

dyladan commented Mar 22, 2021

AH I thought this had been merged. Yes this is still a requirement. We will also need one on the API repo. I'll make sure it's compatible with the latest spec versioning doc and update it today. thanks for the heads up

@vmarchaud
Copy link
Member

@dyladan ping ;)

@dyladan
Copy link
Member Author

dyladan commented Apr 20, 2021

@vmarchaud pong :)

I'll do this today

@obecny
Copy link
Member

obecny commented May 17, 2021

@dyladan ping

@dyladan
Copy link
Member Author

dyladan commented May 17, 2021

image

image

Yikes. I'm in the middle of something at the moment but I actually have this mostly done. Just need to make a final pass and i'll update.

@dyladan dyladan changed the title chore: add versioning proposal chore: Link to spec versioning doc Jun 2, 2021
@dyladan dyladan merged commit 287098d into open-telemetry:main Jun 3, 2021
@dyladan dyladan deleted the versioning branch June 3, 2021 15:43
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
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.

5 participants