Support duration and float metrics (#223)

Fixes #209

Author: cretz

src/Temporalio.Extensions.DiagnosticSource/CustomMetricMeter.cs

@@ -13,6 +13,12 @@ namespace Temporalio.Extensions.DiagnosticSource
     /// Implementation of <see cref="ICustomMetricMeter" /> for a <see cref="Meter" /> that can be
     /// set on <see cref="MetricsOptions.CustomMetricMeter" /> to record metrics to the meter.
     /// </summary>
+    /// <remarks>
+    /// By default all histograms are set as a <c>long</c> of milliseconds unless
+    /// <see cref="MetricsOptions.CustomMetricMeterOptions"/> is set to <c>FloatSeconds</c>.
+    /// Similarly, if the unit for a histogram is "duration", it is changed to "ms" unless that same
+    /// setting is set, at which point the unit is changed to "s".
+    /// </remarks>
     public class CustomMetricMeter : ICustomMetricMeter
     {
         /// <summary>
@@ -35,8 +41,22 @@ public ICustomMetricCounter<T> CreateCounter<T>(
         /// <inheritdoc />
         public ICustomMetricHistogram<T> CreateHistogram<T>(
             string name, string? unit, string? description)
-            where T : struct =>
-            new CustomMetricHistogram<T>(Meter.CreateHistogram<T>(name, unit, description));
+            where T : struct
+        {
+            // Have to convert TimeSpan to something .NET meter can work with. For this to even
+            // happen, a user would have had to set custom options to report as time span.
+            if (typeof(T) == typeof(TimeSpan))
+            {
+                // If unit is "duration", change to "ms since we're converting here
+                if (unit == "duration")
+                {
+                    unit = "ms";
+                }
+                return (new CustomMetricHistogramTimeSpan(
+                    Meter.CreateHistogram<long>(name, unit, description)) as ICustomMetricHistogram<T>)!;
+            }
+            return new CustomMetricHistogram<T>(Meter.CreateHistogram<T>(name, unit, description));
+        }
 
         /// <inheritdoc />
         public ICustomMetricGauge<T> CreateGauge<T>(
@@ -75,6 +95,17 @@ public void Record(T value, object tags) =>
                 underlying.Record(value, ((Tags)tags).TagList);
         }
 
+        private sealed class CustomMetricHistogramTimeSpan : ICustomMetricHistogram<TimeSpan>
+        {
+            private readonly Histogram<long> underlying;
+
+            internal CustomMetricHistogramTimeSpan(Histogram<long> underlying) =>
+                this.underlying = underlying;
+
+            public void Record(TimeSpan value, object tags) =>
+                underlying.Record((long)value.TotalMilliseconds, ((Tags)tags).TagList);
+        }
+
 #pragma warning disable CA1001 // We are disposing the lock on destruction since this can't be disposable
         private sealed class CustomMetricGauge<T> : ICustomMetricGauge<T>
 #pragma warning restore CA1001

src/Temporalio/Bridge/Api/WorkflowActivation/WorkflowActivation.cs

@@ -193,6 +193,7 @@ static WorkflowActivationReflection() {
   /// * Signal and update handlers should be invoked before workflow routines are iterated. That is to
   ///   say before the users' main workflow function and anything spawned by it is allowed to continue.
   /// * Queries always go last (and, in fact, always come in their own activation)
+  /// * Evictions also always come in their own activation
   ///
   /// The downside of this reordering is that a signal or update handler may not observe that some
   /// other event had already happened (ex: an activity completed) when it is first invoked, though it
@@ -204,11 +205,9 @@ static WorkflowActivationReflection() {
   ///
   /// ## Evictions
   ///
-  /// Activations that contain only a `remove_from_cache` job should not cause the workflow code
-  /// to be invoked and may be responded to with an empty command list. Eviction jobs may also
-  /// appear with other jobs, but will always appear last in the job list. In this case it is
-  /// expected that the workflow code will be invoked, and the response produced as normal, but
-  /// the caller should evict the run after doing so.
+  /// Evictions appear as an activations that contains only a `remove_from_cache` job. Such activations
+  /// should not cause the workflow code to be invoked and may be responded to with an empty command
+  /// list.
   /// </summary>
   internal sealed partial class WorkflowActivation : pb::IMessage<WorkflowActivation>
   #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE
@@ -853,7 +852,8 @@ public WorkflowActivationJob Clone() {
     /// <summary>Field number for the "query_workflow" field.</summary>
     public const int QueryWorkflowFieldNumber = 5;
     /// <summary>
-    /// A request to query the workflow was received.
+    /// A request to query the workflow was received. It is guaranteed that queries (one or more)
+    /// always come in their own activation after other mutating jobs.
     /// </summary>
     [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
     [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
@@ -1005,11 +1005,9 @@ public WorkflowActivationJob Clone() {
     /// <summary>Field number for the "remove_from_cache" field.</summary>
     public const int RemoveFromCacheFieldNumber = 50;
     /// <summary>
-    /// Remove the workflow identified by the [WorkflowActivation] containing this job from the cache
-    /// after performing the activation.
-    ///
-    /// If other job variant are present in the list, this variant will be the last job in the
-    /// job list. The string value is a reason for eviction.
+    /// Remove the workflow identified by the [WorkflowActivation] containing this job from the
+    /// cache after performing the activation. It is guaranteed that this will be the only job
+    /// in the activation if present.
     /// </summary>
     [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
     [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]

src/Temporalio/Bridge/Cargo.lock

@@ -10,22 +10,29 @@ namespace Temporalio.Bridge
     internal class CustomMetricMeter
     {
         private readonly Temporalio.Runtime.ICustomMetricMeter meter;
+        private readonly Temporalio.Runtime.CustomMetricMeterOptions options;
         private readonly List<GCHandle> handles = new();
 
         /// <summary>
         /// Initializes a new instance of the <see cref="CustomMetricMeter" /> class.
         /// </summary>
         /// <param name="meter">Meter implementation.</param>
-        public unsafe CustomMetricMeter(Temporalio.Runtime.ICustomMetricMeter meter)
+        /// <param name="options">Options.</param>
+        public unsafe CustomMetricMeter(
+            Temporalio.Runtime.ICustomMetricMeter meter,
+            Temporalio.Runtime.CustomMetricMeterOptions options)
         {
             this.meter = meter;
+            this.options = options;
 
             // Create metric meter struct
             var interopMeter = new Interop.CustomMetricMeter()
             {
-                metric_integer_new = FunctionPointer<Interop.CustomMetricMeterMetricIntegerNewCallback>(CreateMetric),
-                metric_integer_free = FunctionPointer<Interop.CustomMetricMeterMetricIntegerFreeCallback>(FreeMetric),
-                metric_integer_update = FunctionPointer<Interop.CustomMetricMeterMetricIntegerUpdateCallback>(UpdateMetric),
+                metric_new = FunctionPointer<Interop.CustomMetricMeterMetricNewCallback>(CreateMetric),
+                metric_free = FunctionPointer<Interop.CustomMetricMeterMetricFreeCallback>(FreeMetric),
+                metric_record_integer = FunctionPointer<Interop.CustomMetricMeterMetricRecordIntegerCallback>(RecordMetricInteger),
+                metric_record_float = FunctionPointer<Interop.CustomMetricMeterMetricRecordFloatCallback>(RecordMetricFloat),
+                metric_record_duration = FunctionPointer<Interop.CustomMetricMeterMetricRecordDurationCallback>(RecordMetricDuration),
                 attributes_new = FunctionPointer<Interop.CustomMetricMeterAttributesNewCallback>(CreateAttributes),
                 attributes_free = FunctionPointer<Interop.CustomMetricMeterAttributesFreeCallback>(FreeAttributes),
                 meter_free = FunctionPointer<Interop.CustomMetricMeterMeterFreeCallback>(Free),
@@ -58,38 +65,69 @@ private static unsafe string GetString(byte* bytes, UIntPtr size) =>
             Interop.ByteArrayRef name,
             Interop.ByteArrayRef description,
             Interop.ByteArrayRef unit,
-            Interop.MetricIntegerKind kind)
+            Interop.MetricKind kind)
         {
-            Temporalio.Runtime.ICustomMetric<long> metric;
+            GCHandle metric;
             var nameStr = GetString(name);
             var unitStr = GetStringOrNull(unit);
             var descStr = GetStringOrNull(description);
             switch (kind)
             {
-                case Interop.MetricIntegerKind.Counter:
-                    metric = meter.CreateCounter<long>(nameStr, unitStr, descStr);
+                case Interop.MetricKind.CounterInteger:
+                    metric = GCHandle.Alloc(meter.CreateCounter<long>(nameStr, unitStr, descStr));
                     break;
-                case Interop.MetricIntegerKind.Histogram:
-                    metric = meter.CreateHistogram<long>(nameStr, unitStr, descStr);
+                case Interop.MetricKind.HistogramInteger:
+                    metric = GCHandle.Alloc(meter.CreateHistogram<long>(nameStr, unitStr, descStr));
                     break;
-                case Interop.MetricIntegerKind.Gauge:
-                    metric = meter.CreateGauge<long>(nameStr, unitStr, descStr);
+                case Interop.MetricKind.HistogramFloat:
+                    metric = GCHandle.Alloc(meter.CreateHistogram<double>(nameStr, unitStr, descStr));
+                    break;
+                case Interop.MetricKind.HistogramDuration:
+                    switch (options.HistogramDurationFormat)
+                    {
+                        case Temporalio.Runtime.CustomMetricMeterOptions.DurationFormat.IntegerMilliseconds:
+                            // Change unit from "duration" to "ms" since we're converting to ms
+                            if (unitStr == "duration")
+                            {
+                                unitStr = "ms";
+                            }
+                            metric = GCHandle.Alloc(meter.CreateHistogram<long>(nameStr, unitStr, descStr));
+                            break;
+                        case Temporalio.Runtime.CustomMetricMeterOptions.DurationFormat.FloatSeconds:
+                            // Change unit from "duration" to "s" since we're converting to s
+                            if (unitStr == "duration")
+                            {
+                                unitStr = "s";
+                            }
+                            metric = GCHandle.Alloc(meter.CreateHistogram<double>(nameStr, unitStr, descStr));
+                            break;
+                        case Temporalio.Runtime.CustomMetricMeterOptions.DurationFormat.TimeSpan:
+                            metric = GCHandle.Alloc(meter.CreateHistogram<TimeSpan>(nameStr, unitStr, descStr));
+                            break;
+                        default:
+                            throw new InvalidOperationException($"Unknown format: {options.HistogramDurationFormat}");
+                    }
+                    break;
+                case Interop.MetricKind.GaugeInteger:
+                    metric = GCHandle.Alloc(meter.CreateGauge<long>(nameStr, unitStr, descStr));
+                    break;
+                case Interop.MetricKind.GaugeFloat:
+                    metric = GCHandle.Alloc(meter.CreateGauge<double>(nameStr, unitStr, descStr));
                     break;
                 default:
                     throw new InvalidOperationException($"Unknown kind: {kind}");
             }
             // Return pointer
-            return GCHandle.ToIntPtr(GCHandle.Alloc(metric)).ToPointer();
+            return GCHandle.ToIntPtr(metric).ToPointer();
         }
 
         private unsafe void FreeMetric(void* metric) => GCHandle.FromIntPtr(new(metric)).Free();
 
-        private unsafe void UpdateMetric(void* metric, ulong value, void* attributes)
+        private unsafe void RecordMetricInteger(void* metric, ulong value, void* attributes)
         {
             var metricObject = (Temporalio.Runtime.ICustomMetric<long>)GCHandle.FromIntPtr(new(metric)).Target!;
             var tags = GCHandle.FromIntPtr(new(attributes)).Target!;
-            // We trust that value will never be over Int64.MaxValue
-            var metricValue = unchecked((long)value);
+            var metricValue = value > long.MaxValue ? long.MaxValue : unchecked((long)value);
             switch (metricObject)
             {
                 case Temporalio.Runtime.ICustomMetricCounter<long> counter:
@@ -104,6 +142,51 @@ private unsafe void UpdateMetric(void* metric, ulong value, void* attributes)
             }
         }
 
+        private unsafe void RecordMetricFloat(void* metric, double value, void* attributes)
+        {
+            var metricObject = (Temporalio.Runtime.ICustomMetric<double>)GCHandle.FromIntPtr(new(metric)).Target!;
+            var tags = GCHandle.FromIntPtr(new(attributes)).Target!;
+            switch (metricObject)
+            {
+                case Temporalio.Runtime.ICustomMetricHistogram<double> histogram:
+                    histogram.Record(value, tags);
+                    break;
+                case Temporalio.Runtime.ICustomMetricGauge<double> gauge:
+                    gauge.Set(value, tags);
+                    break;
+            }
+        }
+
+        private unsafe void RecordMetricDuration(void* metric, ulong valueMs, void* attributes)
+        {
+            var metricObject = GCHandle.FromIntPtr(new(metric)).Target!;
+            var tags = GCHandle.FromIntPtr(new(attributes)).Target!;
+            var metricValue = valueMs > long.MaxValue ? long.MaxValue : unchecked((long)valueMs);
+            // We don't want to throw out of here, so we just fall through if anything doesn't match
+            // expected types (which should never happen since we controlled creation)
+            switch (options.HistogramDurationFormat)
+            {
+                case Temporalio.Runtime.CustomMetricMeterOptions.DurationFormat.IntegerMilliseconds:
+                    if (metricObject is Temporalio.Runtime.ICustomMetricHistogram<long> histLong)
+                    {
+                        histLong.Record(metricValue, tags);
+                    }
+                    break;
+                case Temporalio.Runtime.CustomMetricMeterOptions.DurationFormat.FloatSeconds:
+                    if (metricObject is Temporalio.Runtime.ICustomMetricHistogram<double> histDouble)
+                    {
+                        histDouble.Record(metricValue / 1000.0, tags);
+                    }
+                    break;
+                case Temporalio.Runtime.CustomMetricMeterOptions.DurationFormat.TimeSpan:
+                    if (metricObject is Temporalio.Runtime.ICustomMetricHistogram<TimeSpan> histTimeSpan)
+                    {
+                        histTimeSpan.Record(TimeSpan.FromMilliseconds(metricValue), tags);
+                    }
+                    break;
+            }
+        }
+
         private unsafe void* CreateAttributes(
             void* appendFrom, Interop.CustomMetricAttribute* attributes, UIntPtr attributesSize)
         {