Repository navigation
[Feature Request] [Rust SDK] Add Exponential/Native Histograms #12222
Replies: 3 comments 10 replies
|
Hi Alex, nice to meet you! Thanks for this; I'll find the right folks to pull in here 👀 |
|
@mckalexee Thanks for the quality writeup! This is something I think we'd be happy to accept. My main concern would be ensuring we keep backwards compatibility, which you've addressed. I'm slightly dubious about the Lastly, it would be good to consider if this is something we'd need/want to port to Go/Java as well. They use different metrics libraries, so the situation may be a bit different there, but it's worth addressing. In any case, I would be fine starting with just Core. |
|
Done in temporalio/sdk-rust#1654 Thanks @mckalexee |
Uh oh!
There was an error while loading. Please reload this page.
Is your feature request related to a problem? Please describe.
Our organization has moved over to native histograms wherever we can due to the increased granularity without blowing up cardinality in our observability platform (Chronosphere). It's also tedious to manage bucket sizes manually when we have activities that take very different amounts of time to complete.
We had a specific request from an engineer to add more buckets to
temporal_activity_succeed_endtoend_latency, but it would be much more convenient if we could do this within the SDK itself.Describe the solution you'd like
OpenTelemetry exponential histograms, which Prometheus-compatible backends store as native histograms, are already supported by the
opentelemetry_sdkcrate pinned in this project.I'm not the biggest rust-head, but through some LLM help I have the interface for the minimum change that should enable this. My goal with this approach is to keep existing metric names consistent, and allow emitting both metric types to allow cutover. Since you cannot export a native and traditional histogram with the same metric name in OpenTelemetry, the native histograms carry a suffix,
_nativeby default, in every mode. The suffix can be changed but not removed, so switching between modes never renames a series.Here is the proposed change to
crates/common/src/telemetry.rs, based off of the pattern established inLoggerFormat.Along with a change to
crates/sdk-core-c-bridge/src/runtime.rsfor .NET.This will also require updates to the various language libraries once this lands here. Happy to flesh this out on this feature request or open new issues on the various repositories.
Enabling these histograms in TypeScript could look something like this.
Additional context
I am happy to drive the implementation here and create the PRs. Our team loves Temporal and this would be a big observability win for us.
Please let me know if I can answer any questions or provide any more detail.
Thanks!
Related: #6633 asks for the same thing across server and SDKs.
All reactions