-
Notifications
You must be signed in to change notification settings - Fork 837
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
feat(otlp-grpc-exporter-base): use statically generated protobuf code #3705
feat(otlp-grpc-exporter-base): use statically generated protobuf code #3705
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3705 +/- ##
==========================================
- Coverage 93.50% 93.19% -0.31%
==========================================
Files 293 296 +3
Lines 8961 8999 +38
Branches 1853 1853
==========================================
+ Hits 8379 8387 +8
- Misses 582 612 +30
|
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.
Looks good. I tried this myself once before and got quite frustrated
experimental/packages/otlp-grpc-exporter-base/src/MetricsExportServiceClient.ts
Show resolved
Hide resolved
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.
Looks good to me. It looks like the precompile
is running npm protos
which itself is running npm submodule
, then after the compile is finished lerna run protos
is run again. i think this isn't required anymore? Seems to only affect grpc exporter base and proto exporter base which both handle it in the precompile step. I think also the submodules are updated once per package instead of globally. Instead of --concurrency 1
it might make sense to just run once? I think the script is not doing anything package-specific.
I'm not sure I understood correctly: did you mean that I should split the I pushed a change that does that but keeps the |
Yes. This just prevents duplicate submodule updates when the whole project is compiled
Yeah it looks right to me |
I'll wait for #3712 to merge first and then add the logs client as well. |
fc01818
to
073b01a
Compare
Added logs too, should be good to go. 🙂 |
…324) <h3>Snyk has created this PR to upgrade @opentelemetry/instrumentation from 0.38.0 to 0.39.1.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **2 versions** ahead of your current version. - The recommended version was released **22 days ago**, on 2023-05-12. <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>@opentelemetry/instrumentation</b></summary> <ul> <li> <b>0.39.1</b> - <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/releases/tag/experimental%2Fv0.39.1">2023-05-12</a></br><h3><g-emoji class="g-emoji" alias="bug" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f41b.png">🐛</g-emoji> (Bug Fix)</h3> <ul> <li>fix(otlp-transformer): move api-logs to dependencies <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3798" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3798/hovercard">#3798</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/pichlermarc">@ pichlermarc</a></li> </ul> </li> <li> <b>0.39.0</b> - <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/releases/tag/experimental%2Fv0.39.0">2023-05-11</a></br><h3><g-emoji class="g-emoji" alias="rocket" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f680.png">🚀</g-emoji> (Enhancement)</h3> <ul> <li>feat(otlp-transformer): support log records. <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3712/" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3712/hovercard">#3712</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/llc1123/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/llc1123">@ llc1123</a></li> <li>feat(otlp-grpc-exporter-base): support log records. <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3712/" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3712/hovercard">#3712</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/llc1123/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/llc1123">@ llc1123</a></li> <li>feat(exporter-logs-otlp-grpc): otlp-grpc exporter for logs. <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3712/" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3712/hovercard">#3712</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/llc1123/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/llc1123">@ llc1123</a></li> <li>feat(otlp-grpc-exporter-base): use statically generated protobuf code <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3705" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3705/hovercard">#3705</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/pichlermarc">@ pichlermarc</a></li> <li>refactor(otlp-transformer): refine metric transformers. <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3770/" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3770/hovercard">#3770</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/llc1123/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/llc1123">@ llc1123</a></li> </ul> <h3><g-emoji class="g-emoji" alias="bug" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f41b.png">🐛</g-emoji> (Bug Fix)</h3> <ul> <li>fix(instrumentation): update <code>require-in-the-middle</code> to v7.1.0 <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3727" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3727/hovercard">#3727</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/trentm/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/trentm">@ trentm</a></li> <li>fix(instrumentation): update <code>require-in-the-middle</code> to v7.0.1 <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3743" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3743/hovercard">#3743</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/trentm/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/trentm">@ trentm</a></li> </ul> <h3><g-emoji class="g-emoji" alias="books" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f4da.png">📚</g-emoji> (Refine Doc)</h3> <ul> <li>doc(instrumentation): add limitiations section to readme <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3786" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3786/hovercard">#3786</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/Flarna/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/Flarna">@ Flarna</a></li> </ul> </li> <li> <b>0.38.0</b> - <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/releases/tag/experimental%2Fv0.38.0">2023-04-13</a></br><h3><g-emoji class="g-emoji" alias="boom" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f4a5.png">💥</g-emoji> Breaking Change</h3> <ul> <li>fix: remove HTTP/HTTPS prefix from span name in instrumentation-xml-http-request <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3672" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3672/hovercard">#3672</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/jufab/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/jufab">@ jufab</a></li> <li>fix(sdk-node)!: remove unused defaultAttributes option <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3724" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3724/hovercard">#3724</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/pichlermarc">@ pichlermarc</a> <ul> <li>Please use <code>NodeSDKConfiguration.resource</code> instead</li> </ul> </li> </ul> <h3><g-emoji class="g-emoji" alias="rocket" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f680.png">🚀</g-emoji> (Enhancement)</h3> <ul> <li>feat(sdk-logs): use logs API 0.38</li> </ul> <h3><g-emoji class="g-emoji" alias="bug" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f41b.png">🐛</g-emoji> (Bug Fix)</h3> <ul> <li>fix(sdk-node): only set DiagConsoleLogger when OTEL_LOG_LEVEL is set <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/pull/3693" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/3693/hovercard">#3693</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://snyk.io/redirect/github/pichlermarc">@ pichlermarc</a></li> </ul> </li> </ul> from <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/releases">@opentelemetry/instrumentation GitHub release notes</a> </details> </details> <details> <summary><b>Commit messages</b></summary> </br> <details> <summary>Package name: <b>@opentelemetry/instrumentation</b></summary> <ul> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/bba09c0e5a6d92fc71ed08028966ae9d56da53db">bba09c0</a> chore: release 0.39.1 (#3800)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/29c46ef5cefe2cc3bf88a64c9ba810aa557f7a06">29c46ef</a> fix(otlp-transformer): move api-logs to dependencies (#3798)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/8fc76896595aac912bf9e15d4f19c167317844c8">8fc7689</a> chore: release 1.13 / 0.39 (#3776)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/32632bd30309621908da8166c6595a23af892f06">32632bd</a> doc(instrumentation): add limitiations section to readme (#3786)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/b8b63083869fb5850fd4830ded0fc509a2cd0f0a">b8b6308</a> chore: remove "opentelemetry" prefix from opencensus-shim package directory (#3784)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/7255da994fe79f115ceb9b1260fb171cf9313706">7255da9</a> feat(opencensus-shim) add mapping logic and propagation shim (#3751)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/32ae641ad13be8786cce9ffc3942efa385c0ab33">32ae641</a> fix(deps): update dependency require-in-the-middle to v7.1.0 for types and named export (#3727)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/2f1e31648402b310448d226ae9632b115e6ad5c4">2f1e316</a> feat(otlp-grpc-exporter-base): use statically generated protobuf code (#3705)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/abfb1bb68e1ec8a183c01644ab9673cd6ac74841">abfb1bb</a> refactor(otlp-transformer): refine metrics transformer (#3770)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/a96116db88c7c0a0d1185601942b379c6118bd6d">a96116d</a> deps: remove `rimraf` (#3769)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/98e4e821bcd5b3d74f95db26439a93c3abb9f467">98e4e82</a> feat(exporter-logs-otlp-grpc): implements otlp-grpc exporters for logs (#3712)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/9347ca6a3ea5f40b9fff33d1a9a3ae2bea7e6ba8">9347ca6</a> fix(http): stop listening to `request`'s `close` event once it has emitted `response` (#3625)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/6676414505f6ffc75e151fa162e351fa1de7204e">6676414</a> feat(opencensus-shim): add OpenCensus shim package boilerplate (#3750)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/cd0d8806e0ed4c9dc68be9773770349c712e5e76">cd0d880</a> fix(sdk-node): lazy require @ opentelemetry/exporter-jaeger (#3739)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/de354dbbdd24bb9f93edef6d098fe8484c7bc3ff">de354db</a> fix: VisibilityState type for typescript >4.6 (#3741)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/61aae103f39ffa20f4e01f23f4bfd6b1cc6f7e9e">61aae10</a> Fix changelog link (#3744)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/3e99e13a4ec55a94926fdb1c1c6f5955783b3ab3">3e99e13</a> fix(instrumentation): update dep require-in-the-middle to 7.0.1 (#3743)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/1b50f91850d0d503feb3a2aeb5c83d63598e8180">1b50f91</a> deps(sdk-logs): remove unused rimraf dev dependency (#3738)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/5b9534bd47deb2a15f7414ba7df05057f6ab9b1e">5b9534b</a> deps: remove unused mkdirp and rimraf dependencies (#3737)</li> <li><a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/commit/053acb604c9d2b735bb267ff47724bb3af1ffd62">053acb6</a> Logs version fixup (#3729)</li> </ul> <a href="https://snyk.io/redirect/github/open-telemetry/opentelemetry-js/compare/a04090010ee18e17487b449984807cc2b7b6e3e6...bba09c0e5a6d92fc71ed08028966ae9d56da53db">Compare</a> </details> </details> <hr/> **Note:** *You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs.* For more information: <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiI2Y2I5ODY1ZC00NzEyLTQ2NTEtYmQ5Mi0zZThlODVlMGU2MWYiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjZjYjk4NjVkLTQ3MTItNDY1MS1iZDkyLTNlOGU4NWUwZTYxZiJ9fQ==" width="0" height="0"/> 🧐 [View latest project report](https://app.snyk.io/org/sullivanpj/project/61dfb01d-4c48-4d8b-a40c-a3939907a630?utm_source=github&utm_medium=referral&page=upgrade-pr) 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/sullivanpj/project/61dfb01d-4c48-4d8b-a40c-a3939907a630/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr) 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/sullivanpj/project/61dfb01d-4c48-4d8b-a40c-a3939907a630/settings/integration?pkg=@opentelemetry/instrumentation&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades)
Which problem is this PR solving?
grpc_tools_node_protoc
can generate service and protobuf definitions, however, the protobuf types generated by that tool are not compatible with types from our existing@opentelemetry/otlp-transformer
package.This PR adds support for using the gRPC exporter with bundling tools by mimicking what the code generated by gRPC's static code generator is doing, while still keeping the
@opentelemetry/otlp-transformer
package as-is. This is accomplished by hand-rolling a service definition and using code that is statically generated usingprotobufjs-cli
instead.As we already use
protobuf-js-cli
, in the http/protobuf exporter package, I re-used the existing script and moved it to thescripts
directory in the repository root.Fixes #2786, #3632, #3656
Type of change
How Has This Been Tested?
Checklist: