-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom metric support #384
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,111 @@ | ||||||||||
"""Metrics using SDK Core. (unstable) | ||||||||||
|
||||||||||
Nothing in this module should be considered stable. The API may change. | ||||||||||
""" | ||||||||||
|
||||||||||
from __future__ import annotations | ||||||||||
|
||||||||||
from typing import Mapping, Optional, Union | ||||||||||
|
||||||||||
import temporalio.bridge.runtime | ||||||||||
import temporalio.bridge.temporal_sdk_bridge | ||||||||||
|
||||||||||
|
||||||||||
class MetricMeter: | ||||||||||
"""Metric meter using SDK Core.""" | ||||||||||
|
||||||||||
@staticmethod | ||||||||||
def create(runtime: temporalio.bridge.runtime.Runtime) -> Optional[MetricMeter]: | ||||||||||
"""Create optional metric meter.""" | ||||||||||
ref = temporalio.bridge.temporal_sdk_bridge.new_metric_meter(runtime._ref) | ||||||||||
if not ref: | ||||||||||
return None | ||||||||||
return MetricMeter(ref) | ||||||||||
Comment on lines
+21
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a personal Python style suggestion, obviously FFTI.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I try to avoid the RHS ternary in some of these cases, but you'll see I use it in others. I may use it here. |
||||||||||
|
||||||||||
def __init__( | ||||||||||
self, ref: temporalio.bridge.temporal_sdk_bridge.MetricMeterRef | ||||||||||
) -> None: | ||||||||||
"""Initialize metric meter.""" | ||||||||||
self._ref = ref | ||||||||||
self._default_attributes = MetricAttributes(ref.default_attributes) | ||||||||||
|
||||||||||
@property | ||||||||||
def default_attributes(self) -> MetricAttributes: | ||||||||||
"""Default attributes for the metric meter.""" | ||||||||||
return self._default_attributes | ||||||||||
|
||||||||||
|
||||||||||
class MetricCounter: | ||||||||||
"""Metric counter using SDK Core.""" | ||||||||||
|
||||||||||
def __init__( | ||||||||||
self, | ||||||||||
meter: MetricMeter, | ||||||||||
name: str, | ||||||||||
description: Optional[str], | ||||||||||
unit: Optional[str], | ||||||||||
) -> None: | ||||||||||
"""Initialize counter metric.""" | ||||||||||
self._ref = meter._ref.new_counter(name, description, unit) | ||||||||||
|
||||||||||
def add(self, value: int, attrs: MetricAttributes) -> None: | ||||||||||
"""Add value to counter.""" | ||||||||||
if value < 0: | ||||||||||
raise ValueError("Metric value must be non-negative value") | ||||||||||
self._ref.add(value, attrs._ref) | ||||||||||
|
||||||||||
|
||||||||||
class MetricHistogram: | ||||||||||
"""Metric histogram using SDK Core.""" | ||||||||||
|
||||||||||
def __init__( | ||||||||||
self, | ||||||||||
meter: MetricMeter, | ||||||||||
name: str, | ||||||||||
description: Optional[str], | ||||||||||
unit: Optional[str], | ||||||||||
) -> None: | ||||||||||
"""Initialize histogram.""" | ||||||||||
self._ref = meter._ref.new_histogram(name, description, unit) | ||||||||||
|
||||||||||
def record(self, value: int, attrs: MetricAttributes) -> None: | ||||||||||
"""Record value on histogram.""" | ||||||||||
if value < 0: | ||||||||||
raise ValueError("Metric value must be non-negative value") | ||||||||||
self._ref.record(value, attrs._ref) | ||||||||||
|
||||||||||
|
||||||||||
class MetricGauge: | ||||||||||
"""Metric gauge using SDK Core.""" | ||||||||||
|
||||||||||
def __init__( | ||||||||||
self, | ||||||||||
meter: MetricMeter, | ||||||||||
name: str, | ||||||||||
description: Optional[str], | ||||||||||
unit: Optional[str], | ||||||||||
) -> None: | ||||||||||
"""Initialize gauge.""" | ||||||||||
self._ref = meter._ref.new_gauge(name, description, unit) | ||||||||||
|
||||||||||
def set(self, value: int, attrs: MetricAttributes) -> None: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't value supposed to be a float here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like otel metrics support both floats and ints for all metric types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Opened an issue in sdk-core temporalio/sdk-core#604 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that should be a blocker for this PR |
||||||||||
"""Set value on gauge.""" | ||||||||||
if value < 0: | ||||||||||
raise ValueError("Metric value must be non-negative value") | ||||||||||
self._ref.set(value, attrs._ref) | ||||||||||
|
||||||||||
|
||||||||||
class MetricAttributes: | ||||||||||
"""Metric attributes using SDK Core.""" | ||||||||||
|
||||||||||
def __init__( | ||||||||||
self, ref: temporalio.bridge.temporal_sdk_bridge.MetricAttributesRef | ||||||||||
) -> None: | ||||||||||
"""Initialize attributes.""" | ||||||||||
self._ref = ref | ||||||||||
|
||||||||||
def with_additional_attributes( | ||||||||||
self, new_attrs: Mapping[str, Union[str, int, float, bool]] | ||||||||||
) -> MetricAttributes: | ||||||||||
"""Create new attributes with new attributes appended.""" | ||||||||||
return MetricAttributes(self._ref.with_additional_attributes(new_attrs)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
use std::{collections::HashMap, sync::Arc}; | ||
|
||
use pyo3::exceptions::PyTypeError; | ||
use pyo3::prelude::*; | ||
use temporal_sdk_core_api::telemetry::metrics; | ||
|
||
use crate::runtime; | ||
|
||
#[pyclass] | ||
pub struct MetricMeterRef { | ||
meter: metrics::TemporalMeter, | ||
#[pyo3(get)] | ||
default_attributes: MetricAttributesRef, | ||
} | ||
|
||
#[pyclass] | ||
#[derive(Clone)] | ||
pub struct MetricAttributesRef { | ||
attrs: metrics::MetricAttributes, | ||
} | ||
|
||
#[pyclass] | ||
pub struct MetricCounterRef { | ||
counter: Arc<dyn metrics::Counter>, | ||
} | ||
|
||
#[pyclass] | ||
pub struct MetricHistogramRef { | ||
histogram: Arc<dyn metrics::Histogram>, | ||
} | ||
|
||
#[pyclass] | ||
pub struct MetricGaugeRef { | ||
gauge: Arc<dyn metrics::Gauge>, | ||
} | ||
|
||
pub fn new_metric_meter(runtime_ref: &runtime::RuntimeRef) -> Option<MetricMeterRef> { | ||
runtime_ref | ||
.runtime | ||
.core | ||
.telemetry() | ||
.get_metric_meter() | ||
.map(|meter| { | ||
let default_attributes = MetricAttributesRef { | ||
attrs: meter.inner.new_attributes(meter.default_attribs.clone()), | ||
}; | ||
MetricMeterRef { | ||
meter, | ||
default_attributes, | ||
} | ||
}) | ||
} | ||
|
||
#[pymethods] | ||
impl MetricMeterRef { | ||
fn new_counter( | ||
&self, | ||
name: String, | ||
description: Option<String>, | ||
unit: Option<String>, | ||
) -> MetricCounterRef { | ||
MetricCounterRef { | ||
counter: self | ||
.meter | ||
.inner | ||
.counter(build_metric_parameters(name, description, unit)), | ||
} | ||
} | ||
|
||
fn new_histogram( | ||
&self, | ||
name: String, | ||
description: Option<String>, | ||
unit: Option<String>, | ||
) -> MetricHistogramRef { | ||
MetricHistogramRef { | ||
histogram: self | ||
.meter | ||
.inner | ||
.histogram(build_metric_parameters(name, description, unit)), | ||
} | ||
} | ||
|
||
fn new_gauge( | ||
&self, | ||
name: String, | ||
description: Option<String>, | ||
unit: Option<String>, | ||
) -> MetricGaugeRef { | ||
MetricGaugeRef { | ||
gauge: self | ||
.meter | ||
.inner | ||
.gauge(build_metric_parameters(name, description, unit)), | ||
} | ||
} | ||
} | ||
|
||
#[pymethods] | ||
impl MetricCounterRef { | ||
fn add(&self, value: u64, attrs_ref: &MetricAttributesRef) { | ||
self.counter.add(value, &attrs_ref.attrs); | ||
} | ||
} | ||
|
||
#[pymethods] | ||
impl MetricHistogramRef { | ||
fn record(&self, value: u64, attrs_ref: &MetricAttributesRef) { | ||
self.histogram.record(value, &attrs_ref.attrs); | ||
} | ||
} | ||
|
||
#[pymethods] | ||
impl MetricGaugeRef { | ||
fn set(&self, value: u64, attrs_ref: &MetricAttributesRef) { | ||
self.gauge.record(value, &attrs_ref.attrs); | ||
} | ||
} | ||
|
||
fn build_metric_parameters( | ||
name: String, | ||
description: Option<String>, | ||
unit: Option<String>, | ||
) -> metrics::MetricParameters { | ||
let mut build = metrics::MetricParametersBuilder::default(); | ||
build.name(name); | ||
if let Some(description) = description { | ||
build.description(description); | ||
} | ||
if let Some(unit) = unit { | ||
build.unit(unit); | ||
} | ||
// Should be nothing that would fail validation here | ||
build.build().unwrap() | ||
} | ||
|
||
#[pymethods] | ||
impl MetricAttributesRef { | ||
fn with_additional_attributes<'p>( | ||
&self, | ||
py: Python<'p>, | ||
new_attrs: HashMap<String, PyObject>, | ||
) -> PyResult<Self> { | ||
let mut attrs = self.attrs.clone(); | ||
attrs.add_new_attrs( | ||
new_attrs | ||
.into_iter() | ||
.map(|(k, obj)| metric_key_value_from_py(py, k, obj)) | ||
.collect::<PyResult<Vec<metrics::MetricKeyValue>>>()?, | ||
); | ||
Ok(MetricAttributesRef { attrs }) | ||
} | ||
} | ||
|
||
fn metric_key_value_from_py<'p>( | ||
py: Python<'p>, | ||
k: String, | ||
obj: PyObject, | ||
) -> PyResult<metrics::MetricKeyValue> { | ||
let val = if let Ok(v) = obj.extract::<String>(py) { | ||
metrics::MetricValue::String(v) | ||
} else if let Ok(v) = obj.extract::<bool>(py) { | ||
metrics::MetricValue::Bool(v) | ||
} else if let Ok(v) = obj.extract::<i64>(py) { | ||
metrics::MetricValue::Int(v) | ||
} else if let Ok(v) = obj.extract::<f64>(py) { | ||
metrics::MetricValue::Float(v) | ||
} else { | ||
return Err(PyTypeError::new_err(format!( | ||
"Invalid value type for key {}, must be str, int, float, or bool", | ||
k | ||
))); | ||
}; | ||
Ok(metrics::MetricKeyValue::new(k, val)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.