Custom histogram metric buckets (#456)

cretz · web-flow · commit 464b8952b5b3 · 2025-04-17T15:20:56.000-05:00
Fixes #416
diff --git a/src/Temporalio/Bridge/Interop/Interop.cs b/src/Temporalio/Bridge/Interop/Interop.cs
@@ -331,6 +331,9 @@ internal partial struct OpenTelemetryOptions
 
         [NativeTypeName("enum OpenTelemetryProtocol")]
         public OpenTelemetryProtocol protocol;
+
+        [NativeTypeName("MetadataRef")]
+        public ByteArrayRef histogram_bucket_overrides;
     }
 
     internal partial struct PrometheusOptions
@@ -346,6 +349,9 @@ internal partial struct PrometheusOptions
 
         [NativeTypeName("bool")]
         public byte durations_as_seconds;
+
+        [NativeTypeName("MetadataRef")]
+        public ByteArrayRef histogram_bucket_overrides;
     }
 
     [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
@@ -646,7 +652,7 @@ internal unsafe partial struct SlotInfo
     {
         public SlotInfo_Tag tag;
 
-        [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L443_C3")]
+        [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L453_C3")]
         public _Anonymous_e__Union Anonymous;
 
         internal ref WorkflowSlotInfo_Body workflow_slot_info
@@ -775,7 +781,7 @@ internal unsafe partial struct SlotSupplier
     {
         public SlotSupplier_Tag tag;
 
-        [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L494_C3")]
+        [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L504_C3")]
         public _Anonymous_e__Union Anonymous;
 
         internal ref FixedSizeSlotSupplier fixed_size
@@ -815,15 +821,15 @@ internal ref CustomSlotSupplierCallbacksImpl custom
         internal unsafe partial struct _Anonymous_e__Union
         {
             [FieldOffset(0)]
-            [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L495_C5")]
+            [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L505_C5")]
             public _Anonymous1_e__Struct Anonymous1;
 
             [FieldOffset(0)]
-            [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L498_C5")]
+            [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L508_C5")]
             public _Anonymous2_e__Struct Anonymous2;
 
             [FieldOffset(0)]
-            [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L501_C5")]
+            [NativeTypeName("__AnonymousRecord_temporal-sdk-bridge_L511_C5")]
             public _Anonymous3_e__Struct Anonymous3;
 
             internal partial struct _Anonymous1_e__Struct
diff --git a/src/Temporalio/Bridge/OptionsExtensions.cs b/src/Temporalio/Bridge/OptionsExtensions.cs
@@ -108,6 +108,8 @@ public static unsafe Interop.OpenTelemetryOptions ToInteropOptions(
                 metric_temporality = temporality,
                 durations_as_seconds = (byte)(options.UseSecondsForDuration ? 1 : 0),
                 protocol = protocol,
+                histogram_bucket_overrides = ToHistogramBucketOverrides(
+                    options.HistogramBucketOverrides, scope),
             };
         }
 
@@ -131,6 +133,8 @@ public static unsafe Interop.PrometheusOptions ToInteropOptions(
                 counters_total_suffix = (byte)(options.HasCounterTotalSuffix ? 1 : 0),
                 unit_suffix = (byte)(options.HasUnitSuffix ? 1 : 0),
                 durations_as_seconds = (byte)(options.UseSecondsForDuration ? 1 : 0),
+                histogram_bucket_overrides = ToHistogramBucketOverrides(
+                    options.HistogramBucketOverrides, scope),
             };
         }
 
@@ -180,11 +184,7 @@ public static unsafe Interop.MetricsOptions ToInteropOptions(
                 {
                     throw new ArgumentException("Prometheus options must have bind address");
                 }
-                prometheus = scope.Pointer(
-                    new Interop.PrometheusOptions()
-                    {
-                        bind_address = scope.ByteArray(options.Prometheus.BindAddress),
-                    });
+                prometheus = scope.Pointer(options.Prometheus.ToInteropOptions(scope));
             }
             else if (options.OpenTelemetry != null)
             {
@@ -554,6 +554,12 @@ public static Interop.WorkerOptions ToInteropOptions(
             };
         }
 
+        private static Interop.ByteArrayRef ToHistogramBucketOverrides(
+            IReadOnlyDictionary<string, IReadOnlyCollection<double>>? overrides, Scope scope) =>
+            scope.Metadata(overrides?.Select(kvp =>
+                new KeyValuePair<string, string>(
+                    kvp.Key, string.Join(",", kvp.Value.Select(v => v.ToString("0.###"))))));
+
         private static Interop.TunerHolder ToInteropTuner(
             this Temporalio.Worker.Tuning.WorkerTuner tuner,
             Scope scope,
diff --git a/src/Temporalio/Bridge/include/temporal-sdk-bridge.h b/src/Temporalio/Bridge/include/temporal-sdk-bridge.h
@@ -223,13 +223,23 @@ typedef struct OpenTelemetryOptions {
   enum OpenTelemetryMetricTemporality metric_temporality;
   bool durations_as_seconds;
   enum OpenTelemetryProtocol protocol;
+  /**
+   * Histogram bucket overrides in form of
+   * <metric1>\n<float>,<float>,<float>\n<metric2>\n<float>,<float>,<float>
+   */
+  MetadataRef histogram_bucket_overrides;
 } OpenTelemetryOptions;
 
 typedef struct PrometheusOptions {
   struct ByteArrayRef bind_address;
   bool counters_total_suffix;
   bool unit_suffix;
   bool durations_as_seconds;
+  /**
+   * Histogram bucket overrides in form of
+   * <metric1>\n<float>,<float>,<float>\n<metric2>\n<float>,<float>,<float>
+   */
+  MetadataRef histogram_bucket_overrides;
 } PrometheusOptions;
 
 typedef const void *(*CustomMetricMeterMetricNewCallback)(struct ByteArrayRef name,
diff --git a/src/Temporalio/Bridge/src/runtime.rs b/src/Temporalio/Bridge/src/runtime.rs
@@ -5,6 +5,7 @@ use crate::ByteArrayRef;
 use crate::MetadataRef;
 
 use serde_json::json;
+use std::collections::HashMap;
 use std::fmt;
 use std::net::SocketAddr;
 use std::str::FromStr;
@@ -17,6 +18,7 @@ use temporal_sdk_core::telemetry::{build_otlp_metric_exporter, start_prometheus_
 use temporal_sdk_core::CoreRuntime;
 use temporal_sdk_core::TokioRuntimeBuilder;
 use temporal_sdk_core_api::telemetry::metrics::CoreMeter;
+use temporal_sdk_core_api::telemetry::HistogramBucketOverrides;
 use temporal_sdk_core_api::telemetry::MetricTemporality;
 use temporal_sdk_core_api::telemetry::{CoreLog, CoreLogConsumer};
 use temporal_sdk_core_api::telemetry::{
@@ -85,6 +87,9 @@ pub struct OpenTelemetryOptions {
     metric_temporality: OpenTelemetryMetricTemporality,
     durations_as_seconds: bool,
     protocol: OpenTelemetryProtocol,
+    /// Histogram bucket overrides in form of
+    /// <metric1>\n<float>,<float>,<float>\n<metric2>\n<float>,<float>,<float>
+    histogram_bucket_overrides: MetadataRef,
 }
 
 #[repr(C)]
@@ -105,6 +110,9 @@ pub struct PrometheusOptions {
     counters_total_suffix: bool,
     unit_suffix: bool,
     durations_as_seconds: bool,
+    /// Histogram bucket overrides in form of
+    /// <metric1>\n<float>,<float>,<float>\n<metric2>\n<float>,<float>,<float>
+    histogram_bucket_overrides: MetadataRef,
 }
 
 #[derive(Clone)]
@@ -378,7 +386,12 @@ fn create_meter(
                 OpenTelemetryMetricTemporality::Delta => MetricTemporality::Delta,
             })
             .global_tags(options.global_tags.to_string_map_on_newlines())
-            .use_seconds_for_durations(otel_options.durations_as_seconds);
+            .use_seconds_for_durations(otel_options.durations_as_seconds)
+            .histogram_bucket_overrides(HistogramBucketOverrides {
+                overrides: parse_histogram_bucket_overrides(
+                    &otel_options.histogram_bucket_overrides,
+                )?,
+            });
         if otel_options.metric_periodicity_millis > 0 {
             build.metric_periodicity(Duration::from_millis(
                 otel_options.metric_periodicity_millis.into(),
@@ -398,7 +411,12 @@ fn create_meter(
             .global_tags(options.global_tags.to_string_map_on_newlines())
             .counters_total_suffix(prom_options.counters_total_suffix)
             .unit_suffix(prom_options.unit_suffix)
-            .use_seconds_for_durations(prom_options.durations_as_seconds);
+            .use_seconds_for_durations(prom_options.durations_as_seconds)
+            .histogram_bucket_overrides(HistogramBucketOverrides {
+                overrides: parse_histogram_bucket_overrides(
+                    &prom_options.histogram_bucket_overrides,
+                )?,
+            });
         Ok(start_prometheus_metric_exporter(build.build()?)?.meter)
     } else if let Some(custom_meter) = custom_meter {
         Ok(Arc::new(custom_meter))
@@ -408,3 +426,19 @@ fn create_meter(
         ))
     }
 }
+
+fn parse_histogram_bucket_overrides(
+    raw: &MetadataRef,
+) -> anyhow::Result<HashMap<String, Vec<f64>>> {
+    raw.to_string_map_on_newlines()
+        .into_iter()
+        .map(|(k, v)| {
+            let vals: anyhow::Result<Vec<f64>> = v
+                .split(',')
+                .map(str::parse::<f64>)
+                .collect::<Result<_, _>>() // Result<Vec<f64>, ParseFloatError>
+                .map_err(Into::into);
+            vals.map(|vals| (k, vals))
+        })
+        .collect()
+}
diff --git a/src/Temporalio/Runtime/OpenTelemetryOptions.cs b/src/Temporalio/Runtime/OpenTelemetryOptions.cs
@@ -62,6 +62,12 @@ public OpenTelemetryOptions(string url)
         /// </summary>
         public OpenTelemetryProtocol Protocol { get; set; } = OpenTelemetryProtocol.Grpc;
 
+        /// <summary>
+        /// Gets or sets the histogram bucket overrides. The dictionary key is the metric name and
+        /// the value is the collection of buckets as floats.
+        /// </summary>
+        public IReadOnlyDictionary<string, IReadOnlyCollection<double>>? HistogramBucketOverrides { get; set; }
+
         /// <summary>
         /// Create a shallow copy of these options.
         /// </summary>
diff --git a/src/Temporalio/Runtime/PrometheusOptions.cs b/src/Temporalio/Runtime/PrometheusOptions.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 
 namespace Temporalio.Runtime
 {
@@ -41,6 +42,12 @@ public PrometheusOptions()
         /// </summary>
         public bool UseSecondsForDuration { get; set; }
 
+        /// <summary>
+        /// Gets or sets the histogram bucket overrides. The dictionary key is the metric name and
+        /// the value is the collection of buckets as floats.
+        /// </summary>
+        public IReadOnlyDictionary<string, IReadOnlyCollection<double>>? HistogramBucketOverrides { get; set; }
+
         /// <summary>
         /// Create a shallow copy of these options.
         /// </summary>
diff --git a/tests/Temporalio.Tests/.editorconfig b/tests/Temporalio.Tests/.editorconfig
@@ -52,6 +52,10 @@ dotnet_diagnostic.SA1118.severity = none
 # To test lambda attributes, lambda and attribute share line
 dotnet_diagnostic.SA1134.severity = none
 
+# We want to have classes near their tests
+dotnet_diagnostic.SA1201.severity = none
+dotnet_diagnostic.SA1204.severity = none
+
 # We allow underscores as names and Rosyln devs won't fix
 # https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2599
 dotnet_diagnostic.SA1313.severity = none
diff --git a/tests/Temporalio.Tests/Client/TemporalClientWorkflowTests.cs b/tests/Temporalio.Tests/Client/TemporalClientWorkflowTests.cs
@@ -1,4 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
 namespace Temporalio.Tests.Client;
 
 using Temporalio.Api.Enums.V1;
diff --git a/tests/Temporalio.Tests/Extensions/DiagnosticSource/CustomMetricMeterTests.cs b/tests/Temporalio.Tests/Extensions/DiagnosticSource/CustomMetricMeterTests.cs
@@ -1,4 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
 namespace Temporalio.Tests.Extensions.DiagnosticSource;
 
 using System.Collections.Concurrent;
diff --git a/tests/Temporalio.Tests/Extensions/Hosting/ActivityScopeTests.cs b/tests/Temporalio.Tests/Extensions/Hosting/ActivityScopeTests.cs
@@ -1,4 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
 namespace Temporalio.Tests.Extensions.Hosting;
 
 using System.Threading.Tasks;
diff --git a/tests/Temporalio.Tests/Extensions/Hosting/TemporalWorkerServiceTests.cs b/tests/Temporalio.Tests/Extensions/Hosting/TemporalWorkerServiceTests.cs
@@ -1,4 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
 namespace Temporalio.Tests.Extensions.Hosting;
 
 using Microsoft.Extensions.DependencyInjection;
diff --git a/tests/Temporalio.Tests/Runtime/TemporalRuntimeTests.cs b/tests/Temporalio.Tests/Runtime/TemporalRuntimeTests.cs
@@ -273,4 +273,62 @@ public class TaskFailWorkflow
         [WorkflowRun]
         public async Task RunAsync() => throw new InvalidOperationException("Intentional error");
     }
+
+    [Fact]
+    public async Task Runtime_HistogramBucketOverrides_WorksProperly()
+    {
+        // Prom metrics with custom histogram buckets
+        var promAddr = $"127.0.0.1:{TestUtils.FreePort()}";
+        var client = await TemporalClient.ConnectAsync(
+            new()
+            {
+                TargetHost = Client.Connection.Options.TargetHost,
+                Namespace = Client.Options.Namespace,
+                Runtime = new(new()
+                {
+                    Telemetry = new()
+                    {
+                        Metrics = new()
+                        {
+                            Prometheus = new(promAddr)
+                            {
+                                HistogramBucketOverrides = new Dictionary<string, IReadOnlyCollection<double>>
+                                {
+                                    ["temporal_request_latency"] = new[] { 123.4, 567.89 },
+                                    ["custom_histogram"] = new[] { 5d, 6, 7 },
+                                },
+                            },
+                        },
+                    },
+                }),
+            });
+
+        // Generate metrics
+        await client.WorkflowService.GetSystemInfoAsync(new());
+        var hist = client.Connection.Options.Runtime!.MetricMeter.CreateHistogram<float>("custom_histogram");
+        hist.Record(4.5f);
+        hist.Record(5.5f);
+        hist.Record(6.5f);
+        hist.Record(7.5f);
+
+        // Check metrics
+        using var httpClient = new HttpClient();
+        var resp = await httpClient.GetAsync(new Uri($"http://{promAddr}/metrics"));
+        var respLines = (await resp.Content.ReadAsStringAsync()).Split('\n');
+        Assert.Contains(
+            respLines,
+            line => line.StartsWith("temporal_request_latency_bucket{") && line.Contains("le=\"123.4\"}"));
+        Assert.Contains(
+            respLines,
+            line => line.StartsWith("temporal_request_latency_bucket{") && line.Contains("le=\"567.89\"}"));
+        Assert.Contains(
+            respLines,
+            line => line.StartsWith("custom_histogram_bucket{") && line.EndsWith("le=\"5\"} 1"));
+        Assert.Contains(
+            respLines,
+            line => line.StartsWith("custom_histogram_bucket{") && line.EndsWith("le=\"6\"} 2"));
+        Assert.Contains(
+            respLines,
+            line => line.StartsWith("custom_histogram_bucket{") && line.EndsWith("le=\"7\"} 3"));
+    }
 }
diff --git a/tests/Temporalio.Tests/Testing/WorkflowEnvironmentTests.cs b/tests/Temporalio.Tests/Testing/WorkflowEnvironmentTests.cs
@@ -1,5 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
-
 namespace Temporalio.Tests.Testing;
 
 using System.Diagnostics;
diff --git a/tests/Temporalio.Tests/Worker/ActivityWorkerTests.cs b/tests/Temporalio.Tests/Worker/ActivityWorkerTests.cs
@@ -1,4 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes/methods near their tests
 #pragma warning disable xUnit1013 // We want instance methods as activities sometimes
 namespace Temporalio.Tests.Worker;
 
diff --git a/tests/Temporalio.Tests/Worker/WorkerTuningTests.cs b/tests/Temporalio.Tests/Worker/WorkerTuningTests.cs
@@ -1,5 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
-
 using Temporalio.Activities;
 using Temporalio.Client;
 using Temporalio.Worker;
diff --git a/tests/Temporalio.Tests/Worker/WorkerVersioningTests.cs b/tests/Temporalio.Tests/Worker/WorkerVersioningTests.cs
@@ -1,5 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
-
 using Temporalio.Client;
 using Temporalio.Worker;
 using Temporalio.Workflows;
diff --git a/tests/Temporalio.Tests/Worker/WorkflowReplayerTests.cs b/tests/Temporalio.Tests/Worker/WorkflowReplayerTests.cs
@@ -1,5 +1,3 @@
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
-
 namespace Temporalio.Tests.Worker;
 
 using Temporalio.Activities;
diff --git a/tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs b/tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs
@@ -1,6 +1,4 @@
 #pragma warning disable CA1724 // Don't care about name conflicts
-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests
-
 namespace Temporalio.Tests.Worker;
 
 using System.Collections.Concurrent;

Original file line number	Diff line number	Diff line change
`@@ -108,6 +108,8 @@ public static unsafe Interop.OpenTelemetryOptions ToInteropOptions(`
`108`	`108`	`metric_temporality = temporality,`
`109`	`109`	`durations_as_seconds = (byte)(options.UseSecondsForDuration ? 1 : 0),`
`110`	`110`	`protocol = protocol,`
	`111`	`+ histogram_bucket_overrides = ToHistogramBucketOverrides(`
	`112`	`+ options.HistogramBucketOverrides, scope),`
`111`	`113`	`};`
`112`	`114`	`}`
`113`	`115`
`@@ -131,6 +133,8 @@ public static unsafe Interop.PrometheusOptions ToInteropOptions(`
`131`	`133`	`counters_total_suffix = (byte)(options.HasCounterTotalSuffix ? 1 : 0),`
`132`	`134`	`unit_suffix = (byte)(options.HasUnitSuffix ? 1 : 0),`
`133`	`135`	`durations_as_seconds = (byte)(options.UseSecondsForDuration ? 1 : 0),`
	`136`	`+ histogram_bucket_overrides = ToHistogramBucketOverrides(`
	`137`	`+ options.HistogramBucketOverrides, scope),`
`134`	`138`	`};`
`135`	`139`	`}`
`136`	`140`
`@@ -180,11 +184,7 @@ public static unsafe Interop.MetricsOptions ToInteropOptions(`
`180`	`184`	`{`
`181`	`185`	`throw new ArgumentException("Prometheus options must have bind address");`
`182`	`186`	`}`
`183`		`- prometheus = scope.Pointer(`
`184`		`- new Interop.PrometheusOptions()`
`185`		`- {`
`186`		`- bind_address = scope.ByteArray(options.Prometheus.BindAddress),`
`187`		`- });`
	`187`	`+ prometheus = scope.Pointer(options.Prometheus.ToInteropOptions(scope));`
`188`	`188`	`}`
`189`	`189`	`else if (options.OpenTelemetry != null)`
`190`	`190`	`{`
`@@ -554,6 +554,12 @@ public static Interop.WorkerOptions ToInteropOptions(`
`554`	`554`	`};`
`555`	`555`	`}`
`556`	`556`
	`557`	`+ private static Interop.ByteArrayRef ToHistogramBucketOverrides(`
	`558`	`+ IReadOnlyDictionary<string, IReadOnlyCollection<double>>? overrides, Scope scope) =>`
	`559`	`+ scope.Metadata(overrides?.Select(kvp =>`
	`560`	`+ new KeyValuePair<string, string>(`
	`561`	`+ kvp.Key, string.Join(",", kvp.Value.Select(v => v.ToString("0.###"))))));`
	`562`	`+`
`557`	`563`	`private static Interop.TunerHolder ToInteropTuner(`
`558`	`564`	`this Temporalio.Worker.Tuning.WorkerTuner tuner,`
`559`	`565`	`Scope scope,`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests`
`2`	`1`	`namespace Temporalio.Tests.Client;`
`3`	`2`
`4`	`3`	`using Temporalio.Api.Enums.V1;`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests`
`2`	`1`	`namespace Temporalio.Tests.Extensions.DiagnosticSource;`
`3`	`2`
`4`	`3`	`using System.Collections.Concurrent;`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-#pragma warning disable SA1201, SA1204 // We want to have classes near their tests`
`2`	`1`	`namespace Temporalio.Tests.Extensions.Hosting;`
`3`	`2`
`4`	`3`	`using System.Threading.Tasks;`