Conversation
GlenDC
left a comment
There was a problem hiding this comment.
Some initial feedback. Once this is handled we can start looking into details. Even though I must say beyond this feedback it does look good. So don't think there will be much left to do (if anything at all) after we resolved these matters.
| [dependencies] | ||
| base64 = { workspace = true } | ||
| flate2 = { workspace = true, optional = true } | ||
| opentelemetry = { workspace = true, optional = true } |
There was a problem hiding this comment.
there should be no need for you to import opentelemetry or opentelemetry-sdk directly. Both are afaik already part of rama-core and can be used via there.
opentelemetry-proto should be dropped (see other comment).
| println!("cargo::rerun-if-changed=proto"); | ||
| } | ||
|
|
||
| #[cfg(feature = "opentelemetry")] |
There was a problem hiding this comment.
this can be replaced with compiling grpc protos
There was a problem hiding this comment.
There should be no need to watch these env variables btw we already watch proto folder, should be enough for this, this is just about building the code from proto so no need to check env variables
The opentelemetry feature used raw prost_build::Config which requires system protoc, unlike the protobuf feature which goes through rama-grpc-build's vendored protoc.
b7be633 to
a800ddd
Compare
|
Let me know @seuros if you need any guidance or help with this one. |
Added Opentelemetry to the gRPC exporter , this mirror the HTTP example.