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

chore: release proposal v1.0.1 / v0.27.0 #2611

Merged
merged 7 commits into from
Nov 11, 2021
Merged

chore: release proposal v1.0.1 / v0.27.0 #2611

merged 7 commits into from
Nov 11, 2021

Conversation

dyladan
Copy link
Member

@dyladan dyladan commented Nov 10, 2021 •
edited
Loading

Alternative of #2609 which releases both stable and experimental

💥 Breaking Change

  • Other
    • #2566 feat!(metrics): remove batch observer (@dyladan)
    • #2485 feat!: Split metric and trace exporters into new experimental packages (@willarmiros)
    • #2540 fix(sdk-metrics-base): remove metric kind BATCH_OBSERVER (@legendecas)
    • #2496 feat(api-metrics): rename metric instruments to match feature-freeze API specification (@legendecas)
  • opentelemetry-core

🚀 (Enhancement)

  • Other
    • #2523 feat: Rename Labels to Attributes (@pirgeo)
    • #2559 feat(api-metrics): remove bind/unbind and bound instruments (@legendecas)
    • #2563 feat(sdk-metrics-base): remove per-meter config on MeterProvider.getMeter (@legendecas)
  • opentelemetry-core
    • #2465 fix: prefer globalThis instead of window to support webworkers (@legendecas)
  • opentelemetry-semantic-conventions
    • #2532 feat(@opentelemetry/semantic-conventions): change enum to object literals (@echoontheway)
    • #2528 feat: upgrade semantic-conventions to latest v1.7.0 spec (@weyert)
  • opentelemetry-core, opentelemetry-sdk-trace-base

🐛 (Bug Fix)

  • Other
    • #2610 fix: preventing double enable for instrumentation that has been already enabled (@obecny)
    • #2581 feat: lazy initialization of the gzip stream (@fungiboletus)
    • #2584 fix: fixing compatibility versions for detectors (@obecny)
    • #2558 fix(@opentelemetry/exporter-prometheus): unref prometheus server to prevent process running indefinitely (@mothershipper)
    • #2495 fix(sdk-metrics-base): metrics name should be in the max length of 63 (@legendecas)
    • #2497 feat(@opentelemetry-instrumentation-fetch): support reading response body from the hook applyCustomAttributesOnSpan (@echoontheway)
  • opentelemetry-core
    • #2560 fix(core): support regex global flag in urlMatches (@moander)
  • opentelemetry-exporter-zipkin
    • #2519 fix(exporter-zipkin): correct status tags names (@t2t2)

📚 (Refine Doc)

🏠 (Internal)

  • Other
  • opentelemetry-sdk-trace-base, opentelemetry-sdk-trace-node, opentelemetry-sdk-trace-web
  • opentelemetry-context-async-hooks, opentelemetry-context-zone-peer-dep, opentelemetry-core, opentelemetry-exporter-jaeger, opentelemetry-exporter-zipkin, opentelemetry-propagator-b3, opentelemetry-propagator-jaeger, opentelemetry-resources, opentelemetry-sdk-trace-base, opentelemetry-sdk-trace-node, opentelemetry-sdk-trace-web, opentelemetry-shim-opentracing
  • opentelemetry-core

Committers: 23

@dyladan dyladan requested a review from a team November 10, 2021 13:16
@codecov
Copy link

codecov bot commented Nov 10, 2021 •
edited
Loading

Codecov Report

Merging #2611 (0332cdb) into main (fdab642) will increase coverage by 0.97%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2611      +/-   ##
==========================================
+ Coverage   93.04%   94.02%   +0.97%     
==========================================
  Files         138       76      -62     
  Lines        5093     2409    -2684     
  Branches     1096      549     -547     
==========================================
- Hits         4739     2265    -2474     
+ Misses        354      144     -210
Impacted Files Coverage Δ
...opentelemetry-api-metrics/src/NoopMeterProvider.ts
...ges/opentelemetry-instrumentation-http/src/http.ts
...emetry-api-metrics/src/platform/node/globalThis.ts
...y-instrumentation-grpc/src/enums/AttributeNames.ts
...metry-instrumentation-grpc/src/grpc/clientUtils.ts
...entelemetry-instrumentation-grpc/src/grpc/index.ts
...s/opentelemetry-instrumentation-fetch/src/fetch.ts
...ackages/opentelemetry-instrumentation/src/utils.ts
.../opentelemetry-api-metrics/src/api/global-utils.ts
...y-sdk-metrics-base/src/export/aggregators/index.ts
... and 52 more

@obecny
Copy link
Member

obecny commented Nov 10, 2021 •
edited
Loading

I see some mixed versions sometimes it is "1.0.0" sometimes "~1.0.0" imho all stable packages (core, @opentelemetry/sdk-trace-base, etc.) we should pin to ~1.0.0 so they will be getting patches automatically WDYT ?

I think we should simply replace it

@dyladan
Copy link
Member Author

dyladan commented Nov 10, 2021

We used to have it like that but we decided to go to pinned versions to allow users to pin if they wanted. If we do ~ or ^ versions, then the users can't effectively pin because the second-level dependencies would be updated.

This way, it is up to users to decide if they want pinned, ~, or ^ dependencies. If they choose to install tracing ^1.0.0 for example, then the tracing package will be upgraded when they run npm install and so will its dependencies but always to the same version. If they do ~, then they will get bugfixes on tracing and its dependencies. Leaving them pinned leaves this choice to the user which is the correct solution IMO.

A few example scenarios:

  1. User pins "1.0.1" exactly

When we release 1.0.2 of our packages, the user does not get any package changes because they have pinned. If we used ^ or ~ for our internal dependencies, some packages might be updated which is not what this user wanted.

  1. User wants bugfix updates, so installs tracing ~1.0.1

When we release 1.0.2, the user gets the new tracing package, which also has updated dependencies so they get 1.0.2 of all the packages they have installed.

  1. User wants latest compatible version always, so installs tracing ^1.0.1

When we release 1.1.0, the user gets the new tracing package, which also has updated dependencies so they get 1.1.0 of all the packages they have installed.

@dyladan
Copy link
Member Author

dyladan commented Nov 10, 2021

Here is the original issue for that #2065

@dyladan
Copy link
Member Author

dyladan commented Nov 10, 2021

The build fails because the experimental dependencies now point to the unreleased stable deps. When I release, I will release the stable packages then I will run the tests again to make sure they pass before releasing the experimental.

@obecny @vmarchaud sound ok to you?

@vmarchaud
Copy link
Member

lgtm @dyladan

@dyladan dyladan merged commit 9cf402e into open-telemetry:main Nov 11, 2021
@dyladan dyladan deleted the proposal-1.0.1-0.27.0 branch November 11, 2021 14:18
@obecny
Copy link
Member

obecny commented Nov 11, 2021

@dyladan
this error
https://github.com/open-telemetry/opentelemetry-js-contrib/runs/4182283826?check_suite_focus=true

is the consequence of using pinned version for stable versions of our sdk "1.0.0" instead of "~1.0.0"

@opentelemetry/sdk-trace-web depends on @opentelemetry/sdk-trace-base ver. "1.0.0" so it is impossible now to use version "1.0.1" of @opentelemetry/sdk-trace-base

I suspect there will be more errors like this sooner or later

@tonivj5
Copy link

tonivj5 commented Nov 12, 2021 •
edited
Loading

I have noticed the same incompatibility between packages. Could contrib's release sync with this repo be a solution? For themoment, I have downgraded to 1.0.0 and 0.26.0 to remain compat with contrib (nest and express instrumentation`)

@vmarchaud
Copy link
Member

vmarchaud commented Nov 12, 2021 •
edited
Loading

I have noticed the same incompatibility between packages

Could you expand on this ? Most breaking changes are for metrics so you shouldn't have issues with contrib i believe

Could contrib's release sync with this repo be a solution

We already discussed this in the past and its quite hard to have them done at the same time (and even harder automatically)

@tonivj5
Copy link

tonivj5 commented Nov 13, 2021 •
edited
Loading

@vmarchaud I have created a repro: https://github.com/tonivj5/otel-deps-issue

You should see these errors with instrumentations and node-sdk (see on vscode or running npm run tscheck)
image

While doing the repro, and following https://github.com/open-telemetry/opentelemetry-js#compatibility-matrix I suppose it's expected. node-sdk is 0.27.0 and has incompat with instrumentation (express, nest, ...) with 0.26.0...

I think no problem then 😅

@vmarchaud
Copy link
Member

@tonivj5 could you try replacing opentelemetry-exporter-otlp-http by opentelemetry-exporter-trace-otlp-http ? We splitted metrics / trace exporter in 0.27

@tonivj5
Copy link

tonivj5 commented Nov 15, 2021 •
edited
Loading

@vmarchaud neither opentelemetry-exporter-trace-otlp-http or opentelemetry-exporter-trace-otlp-grpc are published as packages at npm. are they missing from the release?

My bad 😆, the packages are @opentelemetry/exporter-trace-otlp-grpc & @opentelemetry/exporter-trace-otlp-http

Anyway, I think it was my bad, https://github.com/open-telemetry/opentelemetry-js#compatibility-matrix says explicit that packages at 0.26 are incompatbile with 0.27: node-sdk's version is 0.27, nest/express instrumentation's version is 0.26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vmarchaud vmarchaud vmarchaud approved these changes

@obecny obecny obecny approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dyladan @obecny @vmarchaud @tonivj5