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.

Sign up for GitHub

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

Jump to bottom

Organize API into Go modules based on stability and dependencies #1467

Closed
punya opened this issue Jan 14, 2021 · 4 comments · Fixed by #1528
Closed

Organize API into Go modules based on stability and dependencies #1467

punya opened this issue Jan 14, 2021 · 4 comments · Fixed by #1528
Labels
pkg:API Related to an API package
Milestone
RC1

Comments

@punya
Copy link
Member

punya commented Jan 14, 2021 •
edited
Loading

In order to release a stable v1.0 of the Trace API and SDK without forcing the same stability on work that is still experimental (e.g. Metrics), we need to separate the Trace API into an independent Go module. While performing this separation, we must respect the following constraints:
1. Stable modules must not depend on unstable modules.
2. The global API must be a stable module.
3. There must not be cyclic dependencies between modules (not just between packages).
@MrAlias MrAlias added pkg:API Related to an API package priority:p1 labels Jan 14, 2021
@MrAlias MrAlias added this to the RC1 milestone Jan 14, 2021
@MrAlias
Copy link
Contributor

MrAlias commented Jan 14, 2021

Related to #1446

@punya

This comment has been minimized.

Sign in to view
@punya punya mentioned this issue Jan 21, 2021
Closed
@punya punya mentioned this issue Feb 2, 2021
Closed
@punya
Copy link
Member Author

punya commented Feb 2, 2021

Today I learned that Go modules can have cyclic dependencies (golang/go#24098), so I'm going to see if this can be solved much more simply with fewer (perhaps no) changes to the package structure.

@punya punya mentioned this issue Feb 2, 2021
Closed
@punya
Copy link
Member Author

punya commented Feb 8, 2021 •
edited
Loading

Goals

  1. Identify the parts of opentelemetry-go that need to have a stable public API as part of the Trace release.
  2. For each package, determine if it can be declared stable (put into a v1.x module), declared unstable (put into a v0.x module), or needs to be split.
  3. Minimize disruption to existing pull requests and ecosystem (e.g. contrib).
  4. Minimize future disruption to code when we stabilize packages.

Non-goals

  1. Keeping metrics-related code out of v1.0 modules. It’s ok for a v1.0 module to have metrics code in an internal/ sub-package where it can be exposed by a v0.x module located at the right place in the hierarchy.
  2. Avoiding dependencies from stable modules to unstable ones. It's ok for a stable module's implementation to depend on an unstable one, as long as the API is unaffected.
  3. Avoiding cyclic module dependencies.

Proposal

  1. Create modules at otel/metric, otel/trace, otel/oteltest, otel/sdk/export/metric, otel/sdk/metric (lots of cyclic dependencies).
  2. Move GetMeterProvider, Meter and SetMeterProvider from otel to otel/metric/global.
  3. Mark otel, otel/trace, otel/sdk as v1.0.
  4. In future when we release Metrics as a stable signal, leave the meter-related functions in otel/metric - at most, add funcs in otel that forward to them.

@Aneurysm9 @MrAlias @rakyll I would appreciate all of your feedback on this approach.

@punya punya mentioned this issue Feb 10, 2021
Closed
6 tasks
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 10, 2021
@punya
Automate maintenance of replace directives
d84c34e 
Relates to open-telemetry#1446 and open-telemetry#1467.

The replace directives are hand-written today, which will become harder
as we create more modules and *especially* as we add cyclic
dependencies. This generates them automatically instead.
@punya punya mentioned this issue Feb 10, 2021
Closed
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 10, 2021
@punya
Automate maintenance of replace directives
bf23df7 
Relates to open-telemetry#1446 and open-telemetry#1467.

The replace directives are hand-written today, which will become harder
as we create more modules and *especially* as we add cyclic
dependencies. This generates them automatically instead.
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 10, 2021
@punya
Automate maintenance of replace directives
f742beb 
Relates to open-telemetry#1446 and open-telemetry#1467.

The replace directives are hand-written today, which will become harder
as we create more modules and *especially* as we add cyclic
dependencies. This generates them automatically instead.
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 10, 2021
@punya
Automate maintenance of replace directives
ede57d8 
Relates to open-telemetry#1446 and open-telemetry#1467.

The replace directives are hand-written today, which will become harder
as we create more modules and *especially* as we add cyclic
dependencies. This generates them automatically instead.
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 10, 2021
@punya
Create new modules required for trace release
798bf7f 
* otel/metric
* otel/trace
* otel/oteltest
* otel/sdk/export/metric
* otel/sdk/metric

Relates to open-telemetry#1467
@punya punya mentioned this issue Feb 10, 2021
Closed
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 10, 2021
@punya
Create new modules required for trace release
85f15fc 
* otel/metric
* otel/trace
* otel/oteltest
* otel/sdk/export/metric
* otel/sdk/metric

Relates to open-telemetry#1467
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 11, 2021
@punya
Create new modules required for trace release
84d400f 
* otel/metric
* otel/trace
* otel/oteltest
* otel/sdk/export/metric
* otel/sdk/metric

Relates to open-telemetry#1467
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 11, 2021
@punya
Create new modules required for trace release
59ac2a7 
* otel/metric
* otel/trace
* otel/oteltest
* otel/sdk/export/metric
* otel/sdk/metric

Relates to open-telemetry#1467
@punya punya mentioned this issue Feb 11, 2021
Merged
punya added a commit to punya/opentelemetry-go that referenced this issue Feb 11, 2021
@punya
Create new modules required for trace release
eb31490 
* otel/metric
* otel/trace
* otel/oteltest
* otel/sdk/export/metric
* otel/sdk/metric

Relates to open-telemetry#1467
MrAlias added a commit that referenced this issue Feb 12, 2021
@punya @MrAlias
Organize API into Go modules based on stability and dependencies (#1528)
9b242bc 
* Add a tool to auto-generate replace directives

* Auto-generated changes from previous commit

* Create new modules required for trace release

* otel/metric
* otel/trace
* otel/oteltest
* otel/sdk/export/metric
* otel/sdk/metric

Relates to #1467

* Move metric-related public API out of otel/ pkg

Move GetMeterProvider, Meter and SetMeterProvider to new package
otel/metric/global in the otel/metric module.

This will allow otel/ module to be released as v1.0.

* Include PR references in CHANGELOG

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Check for root dir

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
ldelossa pushed a commit to ldelossa/opentelemetry-go that referenced this issue Mar 5, 2021
@punya @MrAlias @ldelossa
Organize API into Go modules based on stability and dependencies (ope…
cce1492 
…n-telemetry#1528)

* Add a tool to auto-generate replace directives

* Auto-generated changes from previous commit

* Create new modules required for trace release

* otel/metric
* otel/trace
* otel/oteltest
* otel/sdk/export/metric
* otel/sdk/metric

Relates to open-telemetry#1467

* Move metric-related public API out of otel/ pkg

Move GetMeterProvider, Meter and SetMeterProvider to new package
otel/metric/global in the otel/metric module.

This will allow otel/ module to be released as v1.0.

* Include PR references in CHANGELOG

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Check for root dir

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@punya punya mentioned this issue Mar 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
pkg:API Related to an API package
Projects
None yet
Milestone
RC1
Development

Successfully merging a pull request may close this issue.

Organize API into Go modules based on stability and dependencies punya/opentelemetry-go
2 participants
@punya @MrAlias