Skip to content

Commit 99513c8

Browse files
authored
Apply histogram bucket overrides consistently (#884)
1 parent 583fe15 commit 99513c8

2 files changed

Lines changed: 32 additions & 15 deletions

File tree

core/src/telemetry/otel.rs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,22 @@ fn histo_view(
5252
}
5353

5454
pub(super) fn augment_meter_provider_with_defaults(
55-
mpb: MeterProviderBuilder,
55+
mut mpb: MeterProviderBuilder,
5656
global_tags: &HashMap<String, String>,
5757
use_seconds: bool,
5858
bucket_overrides: HistogramBucketOverrides,
5959
) -> opentelemetry::metrics::Result<MeterProviderBuilder> {
60-
// Some histograms are actually gauges, but we have to use histograms otherwise they forget
61-
// their value between collections since we don't use callbacks.
60+
for (name, buckets) in bucket_overrides.overrides {
61+
mpb = mpb.with_view(new_view(
62+
Instrument::new().name(format!("*{name}")),
63+
opentelemetry_sdk::metrics::Stream::new().aggregation(
64+
Aggregation::ExplicitBucketHistogram {
65+
boundaries: buckets,
66+
record_min_max: true,
67+
},
68+
),
69+
)?)
70+
}
6271
let mut mpb = mpb
6372
.with_view(histo_view(
6473
WORKFLOW_E2E_LATENCY_HISTOGRAM_NAME,
@@ -84,17 +93,6 @@ pub(super) fn augment_meter_provider_with_defaults(
8493
ACTIVITY_EXEC_LATENCY_HISTOGRAM_NAME,
8594
use_seconds,
8695
)?);
87-
for (name, buckets) in bucket_overrides.overrides {
88-
mpb = mpb.with_view(new_view(
89-
Instrument::new().name(format!("*{name}")),
90-
opentelemetry_sdk::metrics::Stream::new().aggregation(
91-
Aggregation::ExplicitBucketHistogram {
92-
boundaries: buckets,
93-
record_min_max: true,
94-
},
95-
),
96-
)?)
97-
}
9896
// Fallback default
9997
mpb = mpb.with_view(new_view(
10098
{

tests/integ_tests/metrics_tests.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ use temporal_sdk::{
1818
};
1919
use temporal_sdk_core::{
2020
CoreRuntime, TokioRuntimeBuilder, init_worker,
21-
telemetry::{build_otlp_metric_exporter, start_prometheus_metric_exporter},
21+
telemetry::{
22+
WORKFLOW_TASK_EXECUTION_LATENCY_HISTOGRAM_NAME, build_otlp_metric_exporter,
23+
start_prometheus_metric_exporter,
24+
},
2225
};
2326
use temporal_sdk_core_api::{
2427
Worker,
@@ -585,6 +588,16 @@ async fn latency_metrics(
585588
.socket_addr(ANY_PORT.parse().unwrap())
586589
.use_seconds_for_durations(use_seconds_latency)
587590
.unit_suffix(show_units)
591+
.histogram_bucket_overrides(HistogramBucketOverrides {
592+
overrides: {
593+
let mut hm = HashMap::new();
594+
hm.insert(
595+
WORKFLOW_TASK_EXECUTION_LATENCY_HISTOGRAM_NAME.to_string(),
596+
vec![1337.0],
597+
);
598+
hm
599+
},
600+
})
588601
.build()
589602
.unwrap(),
590603
));
@@ -620,6 +633,12 @@ async fn latency_metrics(
620633
assert!(matching_line.contains("le=\"100\""));
621634
}
622635

636+
let matching_line = body
637+
.lines()
638+
.find(|l| l.starts_with("temporal_workflow_task_execution_latency"))
639+
.unwrap();
640+
assert!(matching_line.contains("le=\"1337\""));
641+
623642
// Ensure poll metrics show up as long polls properly
624643
let matching_lines = body
625644
.lines()

0 commit comments

Comments
 (0)