temporalio / sdk-core

Core Temporal SDK that can be used as a base for language specific Temporal SDKs
MIT License
278 stars 77 forks source link

[Feature Request] Update to tonic 0.10 #626

Closed cretz closed 3 months ago

cretz commented 1 year ago

Describe the solution you'd like

Update tonic in workspace to 0.10 and fix anything needed. In Python I saw something around the proxier! macro and IntoRequest, but I didn't investigate and it may be unique to Python's macro.

Sushisource commented 1 year ago

As usual this needs https://github.com/open-telemetry/opentelemetry-rust/tree/main/opentelemetry-otlp to update to newer tonic first which may take a while

cretz commented 1 year ago

I see. https://github.com/open-telemetry/opentelemetry-rust/pull/1310 is in PR but unsure when it'll move through. I'll come back to this when I see that (or similar) move.

h7kanna commented 12 months ago

Ideally, this crate's tonic dependency can (possibly) be updated independently of that of Opentelemetry's ? I mean like this

core/Cargo.toml

opentelemetry-tonic = { version = "0.9", package = "tonic", features = ["tls", "tls-roots"] }

core/src/telemetry/metrics.rs

use opentelemetry_tonic::metadata::MetadataMap;
//use tonic::metadata::MetadataMap;

And update tonic = "0.10" and prost-* to "0.12" like this https://github.com/temporalio/sdk-core/compare/master...h7kanna:sdk-core:update-tonic

This can even be applied to https://github.com/temporalio/sdk-core/pull/628 (Upgrade OTel to 0.21)

Sushisource commented 11 months ago

@h7kanna It can be, for sure, but I kinda wanted to avoid having multiple copies of tonic which is a rather large dep. Maybe that's not such a big deal but, so far the need to update as fast as possible hasn't been too important.

h7kanna commented 11 months ago

Yes ofcourse, I just wanted to point out that it is possible if needed.