You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@opentelemetry/exporter-trace-otlp-grpc uses @grpc/proto-loader to load protos at runtime, dynamically producing a package definition that can be passed to grpc.loadPackageDefinition. The relevant code can be found here.
The problem is that @grpc/proto-loader and these external proto files don't play nicely with bundlers like esbuild, webpack, etc. The issue can be hacked around (see #2786 (comment)), but it's quite messy.
Describe the solution you'd like
The protos @opentelemetry/exporter-trace-otlp-grpc uses are static and so, instead of dynamically producing a package definition at runtime, why not statically produce a package definition at compile time?
grpc-tools, from the same people as @grpc/proto-loader, provides a command-line tool, grpc_tools_node_protoc. According to their README.md, it
Generates code that does not require any gRPC library, and instead generates PackageDefinition objects that can be passed to the loadPackageDefinition function provided by both the grpc and @grpc/grpc-js libraries.
I don't have experience with gRPC in Node.js, but I expect that, leveraging grpc-tools as a development dependency, we could stop shipping proto files inside of @opentelemetry/exporter-trace-otlp-grpc. Instead, we would depend on pre-compiled JavaScript (the package definition). Bundlers like esbuild, webpack, etc., would automatically work.
There's one thing to note about grpc-tools: it installs arch × OS-dependent binaries, and it currently lacks arm64 builds for darwin (see grpc/grpc-node#1405). This means it wouldn't be easy to just clone the repo and compile the protos into package definitions on any machine. Instead, if the protos are changing rarely, perhaps the package definitions should be compiled and checked in by a committer with access to an amd64 machine.
Update 1: I started implementing this. As far as I can tell, it is workable generating for grpc_js, but not with generate_package_definition. It also requires a change in the way the protobuf messages are constructed. For example, right now, @opentelemetry/exporter-trace-otlp-grpc depends on @opentelemetry/exporter-trace-otlp-http in order to use these types and toOTLPExportTraceServiceRequest. Instead, I think it would make sense to remove the dependency on @opentelemtry/exporter-trace-otlp-http, since those types can be generated by ts-protoc-gen. Additionally, we'd need to reimplement toOTLPExportTraceServiceRequest in @opentelemetry/exporter-trace-otlp-grpc in terms of low-level, generated methods (setResourceSpans, etc.).
Update 2: Looks like something similar was attempted with pbjs in #2691 and then ultimately hand-rolled in #2746. The new solution will use the JSON instead of binary protobuf encoding (?), although there are some questions around stability of names in the JSON encoding. Inter-package dependencies will change as well. It seems there's active work here, and I should just wait.
Describe alternatives you've considered
Abandon esbuild in order to use @opentelemetry/exporter-trace-otlp-grpc. Uncompressed, my build artifacts would go from a few megabytes to hundreds of megabytes.
Try using @opentelemetry/exporter-trace-otlp-proto instead (it loads proto files differently and so may need fewer hacks to get working in my environment, but still not nice).
Additional context
This change would be relevant to @opentelemetry/exporter-trace-otlp-proto and any other OpenTelemetry JavaScript library that uses (static) protos.
The text was updated successfully, but these errors were encountered:
Similar work is already ongoing here. We created the otlp-transformations package to handle the data manipulation and will be splitting the serialization for protobuf into its own package.
@opentelemetry/exporter-trace-otlp-grpc uses @grpc/proto-loader to load protos at runtime, dynamically producing a package definition that can be passed to
grpc.loadPackageDefinition
. The relevant code can be found here.The problem is that @grpc/proto-loader and these external proto files don't play nicely with bundlers like esbuild, webpack, etc. The issue can be hacked around (see #2786 (comment)), but it's quite messy.
Describe the solution you'd like
The protos @opentelemetry/exporter-trace-otlp-grpc uses are static and so, instead of dynamically producing a package definition at runtime, why not statically produce a package definition at compile time?
grpc-tools, from the same people as @grpc/proto-loader, provides a command-line tool, grpc_tools_node_protoc. According to their README.md, it
I don't have experience with gRPC in Node.js, but I expect that, leveraging grpc-tools as a development dependency, we could stop shipping proto files inside of @opentelemetry/exporter-trace-otlp-grpc. Instead, we would depend on pre-compiled JavaScript (the package definition). Bundlers like esbuild, webpack, etc., would automatically work.
There's one thing to note about grpc-tools: it installs arch × OS-dependent binaries, and it currently lacks arm64 builds for darwin (see grpc/grpc-node#1405). This means it wouldn't be easy to just clone the repo and compile the protos into package definitions on any machine. Instead, if the protos are changing rarely, perhaps the package definitions should be compiled and checked in by a committer with access to an amd64 machine.
Update 1: I started implementing this. As far as I can tell, it is workable generating for
grpc_js
, but not withgenerate_package_definition
. It also requires a change in the way the protobuf messages are constructed. For example, right now, @opentelemetry/exporter-trace-otlp-grpc depends on @opentelemetry/exporter-trace-otlp-http in order to use these types andtoOTLPExportTraceServiceRequest
. Instead, I think it would make sense to remove the dependency on @opentelemtry/exporter-trace-otlp-http, since those types can be generated by ts-protoc-gen. Additionally, we'd need to reimplementtoOTLPExportTraceServiceRequest
in @opentelemetry/exporter-trace-otlp-grpc in terms of low-level, generated methods (setResourceSpans
, etc.).Update 2: Looks like something similar was attempted with
pbjs
in #2691 and then ultimately hand-rolled in #2746. The new solution will use the JSON instead of binary protobuf encoding (?), although there are some questions around stability of names in the JSON encoding. Inter-package dependencies will change as well. It seems there's active work here, and I should just wait.Describe alternatives you've considered
Additional context
This change would be relevant to @opentelemetry/exporter-trace-otlp-proto and any other OpenTelemetry JavaScript library that uses (static) protos.
The text was updated successfully, but these errors were encountered: