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

chore: fix grpc and proto-loader deps #3337

Merged
merged 6 commits into from
Oct 21, 2022
Merged

chore: fix grpc and proto-loader deps #3337

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9741406
chore: fix grpc and proto-looader deps
seemk Oct 14, 2022
c75cbc4
doc: update changelog
seemk Oct 14, 2022
15ab7b3
Merge branch 'main' into grpc-update-1
seemk Oct 17, 2022
c313fb1
Merge branch 'main' into grpc-update-1
seemk Oct 18, 2022
19df1d0
Merge branch 'main' into grpc-update-1
legendecas Oct 21, 2022
93b51e3
Update CHANGELOG.md
legendecas Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ All notable changes to experimental packages in this project will be documented
### :rocket: (Enhancement)

* feat(sdk-node): configure trace exporter with environment variables [#3143](https://github.com/open-telemetry/opentelemetry-js/pull/3143) @svetlanabrennan
* deps: remove unused proto-loader dependencies and update grpc-js and proto-loader versions [#3337](https://github.com/open-telemetry/opentelemetry-js/pull/3337) @seemk
* feat(prometheus): serialize resource as target_info gauge [#3300](https://github.com/open-telemetry/opentelemetry-js/pull/3300) @pichlermarc

### :bug: (Bug Fix)
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/exporter-trace-otlp-grpc/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
},
"devDependencies": {
"@babel/core": "7.16.0",
"@grpc/proto-loader": "^0.7.3",
Copy link
Member

Choose a reason for hiding this comment

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

Is @grpc/proto-loader needed in the dev dependencies even @grpc/grpc-js is a dependency?
I thought @grpc/grpc-js ensures that the correct/matching version of @grpc/proto-loader is installed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since test code requires it explicitly, can we be sure @grpc/grpc-js will depend on it in the future? It might introduce potential breakage 🤔

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 depend on a package directly then it should be in the package.json. Even if it is transitively required by another package npm handles the dedupe when required.

Copy link
Member

Choose a reason for hiding this comment

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

If it is within our dependency dedupe might fail if ranges don't match. Usually we pin our dev deps (otherwise renovate bot complains) which doesn't match to what grpc-js does.

Anyhow I agree it's better to add what we need and update in sync with grpc-js. Maybe renovate bot can support here but I guess it's not so easy to configure it for our needs.

"@opentelemetry/api": "^1.0.0",
"@opentelemetry/otlp-exporter-base": "0.33.0",
"@types/mocha": "10.0.0",
Expand All @@ -67,8 +68,7 @@
"@opentelemetry/api": "^1.0.0"
},
"dependencies": {
"@grpc/grpc-js": "^1.5.9",
"@grpc/proto-loader": "^0.6.9",
"@grpc/grpc-js": "^1.7.1",
"@opentelemetry/core": "1.7.0",
"@opentelemetry/otlp-grpc-exporter-base": "0.33.0",
"@opentelemetry/otlp-transformer": "0.33.0",
Expand Down
1 change: 0 additions & 1 deletion experimental/packages/exporter-trace-otlp-proto/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
"@opentelemetry/api": "^1.0.0"
},
"dependencies": {
"@grpc/proto-loader": "^0.6.9",
"@opentelemetry/core": "1.7.0",
"@opentelemetry/otlp-exporter-base": "0.33.0",
"@opentelemetry/otlp-proto-exporter-base": "0.33.0",
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
},
"devDependencies": {
"@babel/core": "7.16.0",
"@grpc/proto-loader": "^0.7.3",
"@opentelemetry/api": "^1.0.0",
"@opentelemetry/api-metrics": "0.33.0",
"@types/mocha": "10.0.0",
Expand All @@ -67,8 +68,7 @@
"@opentelemetry/api": "^1.0.0"
},
"dependencies": {
"@grpc/grpc-js": "^1.5.9",
"@grpc/proto-loader": "^0.6.9",
"@grpc/grpc-js": "^1.7.1",
"@opentelemetry/core": "1.7.0",
"@opentelemetry/exporter-metrics-otlp-http": "0.33.0",
"@opentelemetry/otlp-grpc-exporter-base": "0.33.0",
Expand Down
1 change: 0 additions & 1 deletion experimental/packages/opentelemetry-exporter-metrics-otlp-proto/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
"@opentelemetry/api": "^1.0.0"
},
"dependencies": {
"@grpc/proto-loader": "0.6.9",
"@opentelemetry/core": "1.7.0",
"@opentelemetry/exporter-metrics-otlp-http": "0.33.0",
"@opentelemetry/otlp-exporter-base": "0.33.0",
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/opentelemetry-instrumentation-grpc/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@
"access": "public"
},
"devDependencies": {
"@grpc/grpc-js": "1.5.9",
"@grpc/proto-loader": "0.6.9",
"@grpc/grpc-js": "^1.7.1",
"@grpc/proto-loader": "^0.7.3",
"@opentelemetry/api": "^1.0.0",
"@opentelemetry/context-async-hooks": "1.7.0",
"@opentelemetry/core": "1.7.0",
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/otlp-grpc-exporter-base/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@
"@opentelemetry/api": "^1.0.0"
},
"dependencies": {
"@grpc/grpc-js": "^1.5.9",
"@grpc/proto-loader": "^0.6.9",
"@grpc/grpc-js": "^1.7.1",
"@grpc/proto-loader": "^0.7.3",
"@opentelemetry/core": "1.7.0",
"@opentelemetry/otlp-exporter-base": "0.33.0"
},
Expand Down
1 change: 0 additions & 1 deletion experimental/packages/otlp-proto-exporter-base/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
"@opentelemetry/api": "^1.0.0"
},
"dependencies": {
"@grpc/proto-loader": "^0.6.9",
"@opentelemetry/core": "1.7.0",
"@opentelemetry/otlp-exporter-base": "0.33.0",
"protobufjs": "7.1.1"
Expand Down