From c31fdb44d7942914ec0093623f2f8cb86e7c0810 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Sun, 13 Jul 2025 14:34:35 +1000 Subject: [PATCH 1/2] The aggregation params of metrics can be defined #844 --- CHANGELOG.md | 2 ++ crates/weaver_forge/src/registry.rs | 9 +++++++ crates/weaver_resolved_schema/src/registry.rs | 7 ++++++ crates/weaver_resolver/src/registry.rs | 1 + crates/weaver_semconv/data/jvm-metrics.yaml | 7 ++++++ crates/weaver_semconv/src/aggregation.rs | 25 +++++++++++++++++++ crates/weaver_semconv/src/group.rs | 18 +++++++++++++ crates/weaver_semconv/src/lib.rs | 1 + crates/weaver_semconv/src/registry.rs | 2 ++ 9 files changed, 72 insertions(+) create mode 100644 crates/weaver_semconv/src/aggregation.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 500c32c8d..29b0484d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file. ([#823](https://github.com/open-telemetry/weaver/pull/823) by @lmolkova) - Don't serialize default values and empty arrays when resolving semantic conventions. ([#822](https://github.com/open-telemetry/weaver/pull/822) by @lmolkova) +- Support for describing aggregation being done on metrics + ([#845](https://github.com/open-telemetry/weaver/pull/822) by @thompson-tomo) # [0.16.1] - 2025-07-04 diff --git a/crates/weaver_forge/src/registry.rs b/crates/weaver_forge/src/registry.rs index f44ec30cf..b526f256b 100644 --- a/crates/weaver_forge/src/registry.rs +++ b/crates/weaver_forge/src/registry.rs @@ -12,6 +12,7 @@ use weaver_resolved_schema::attribute::Attribute; use weaver_resolved_schema::catalog::Catalog; use weaver_resolved_schema::lineage::GroupLineage; use weaver_resolved_schema::registry::{Group, Registry}; +use weaver_semconv::aggregation::AggregationSpec; use weaver_semconv::any_value::AnyValueSpec; use weaver_semconv::deprecated::Deprecated; use weaver_semconv::group::{GroupType, InstrumentSpec, SpanKindSpec}; @@ -99,6 +100,12 @@ pub struct ResolvedGroup { /// Note: This field is required if type is metric. #[serde(skip_serializing_if = "Option::is_none")] pub unit: Option, + /// The aggregation which should occur on the data points being capture by a meter. + /// Semconv metrics all use the default aggregation type, hence this option is for + /// providing the parameters of the aggregation. + /// For more details: [Metrics SDK - Aggregation](https://opentelemetry.io/docs/specs/otel/metrics/sdk/#aggregation). + #[serde(skip_serializing_if = "Option::is_none")] + pub aggregation: Option, /// The name of the event. If not specified, the prefix is used. /// If prefix is empty (or unspecified), name is required. #[serde(skip_serializing_if = "Option::is_none")] @@ -168,6 +175,7 @@ impl ResolvedGroup { metric_name: group.metric_name.clone(), instrument: group.instrument.clone(), unit: group.unit.clone(), + aggregation: group.aggregation.clone(), name: group.name.clone(), lineage, display_name: group.display_name.clone(), @@ -229,6 +237,7 @@ impl ResolvedRegistry { metric_name: group.metric_name.clone(), instrument: group.instrument.clone(), unit: group.unit.clone(), + aggregation: group.aggregation.clone(), name: group.name.clone(), lineage, display_name: group.display_name.clone(), diff --git a/crates/weaver_resolved_schema/src/registry.rs b/crates/weaver_resolved_schema/src/registry.rs index 52eff43d5..4ae82691e 100644 --- a/crates/weaver_resolved_schema/src/registry.rs +++ b/crates/weaver_resolved_schema/src/registry.rs @@ -16,6 +16,7 @@ use crate::registry::GroupStats::{ AttributeGroup, Entity, Event, Metric, MetricGroup, Scope, Span, Undefined, }; use serde::{Deserialize, Serialize}; +use weaver_semconv::aggregation::AggregationSpec; use weaver_semconv::deprecated::Deprecated; use weaver_semconv::group::{GroupType, InstrumentSpec, SpanKindSpec}; use weaver_semconv::provenance::Provenance; @@ -113,6 +114,12 @@ pub struct Group { /// Note: This field is required if type is metric. #[serde(skip_serializing_if = "Option::is_none")] pub unit: Option, + /// The aggregation which should occur on the data points being capture by a meter. + /// Semconv metrics all use the default aggregation type, hence this option is for + /// providing the parameters of the aggregation. + /// For more details: [Metrics SDK - Aggregation](https://opentelemetry.io/docs/specs/otel/metrics/sdk/#aggregation). + #[serde(skip_serializing_if = "Option::is_none")] + pub aggregation: Option, /// The name of the event. If not specified, the prefix is used. /// If prefix is empty (or unspecified), name is required. #[serde(skip_serializing_if = "Option::is_none")] diff --git a/crates/weaver_resolver/src/registry.rs b/crates/weaver_resolver/src/registry.rs index 8123bf6f9..5875914d2 100644 --- a/crates/weaver_resolver/src/registry.rs +++ b/crates/weaver_resolver/src/registry.rs @@ -393,6 +393,7 @@ fn group_from_spec(group: GroupSpecWithProvenance) -> UnresolvedGroup { metric_name: group.spec.metric_name, instrument: group.spec.instrument, unit: group.spec.unit, + aggregation: group.spec.aggregation, name: group.spec.name, lineage: Some(GroupLineage::new(group.provenance.clone())), display_name: group.spec.display_name, diff --git a/crates/weaver_semconv/data/jvm-metrics.yaml b/crates/weaver_semconv/data/jvm-metrics.yaml index b1703fef1..c9cd70e0c 100644 --- a/crates/weaver_semconv/data/jvm-metrics.yaml +++ b/crates/weaver_semconv/data/jvm-metrics.yaml @@ -71,6 +71,13 @@ groups: brief: "Duration of JVM garbage collection actions." instrument: histogram unit: "s" + aggregation: + parameters: + boundaries: + - 0 + - 5 + - 10 + recordMinMax: true attributes: - id: jvm.gc.name stability: stable diff --git a/crates/weaver_semconv/src/aggregation.rs b/crates/weaver_semconv/src/aggregation.rs new file mode 100644 index 000000000..d18393ca4 --- /dev/null +++ b/crates/weaver_semconv/src/aggregation.rs @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Metric specification. +//! +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use crate::YamlValue; + +/// An aggregation specification. +#[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct AggregationSpec { + /// The parameters used in the aggregation + #[serde(skip_serializing_if = "Option::is_none")] + pub parameters: Option>, +} + +impl AggregationSpec { + /// Returns the parameters of the aggregation. + #[must_use] + pub fn parameters(&self) -> &Option> { + &self.parameters + } +} \ No newline at end of file diff --git a/crates/weaver_semconv/src/group.rs b/crates/weaver_semconv/src/group.rs index 831e1b232..6a54e2d7d 100644 --- a/crates/weaver_semconv/src/group.rs +++ b/crates/weaver_semconv/src/group.rs @@ -10,6 +10,7 @@ use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; use std::fmt::{Display, Formatter}; +use crate::aggregation::AggregationSpec; use crate::any_value::AnyValueSpec; use crate::attribute::{AttributeSpec, AttributeType, PrimitiveOrArrayTypeSpec}; use crate::deprecated::Deprecated; @@ -97,6 +98,12 @@ pub struct GroupSpec { /// Note: This field is required if type is metric. #[serde(skip_serializing_if = "Option::is_none")] pub unit: Option, + /// The aggregation which should occur on the data points being capture by a meter. + /// Semconv metrics all use the default aggregation type, hence this option is for + /// providing the parameters of the aggregation. + /// For more details: [Metrics SDK - Aggregation](https://opentelemetry.io/docs/specs/otel/metrics/sdk/#aggregation). + #[serde(skip_serializing_if = "Option::is_none")] + pub aggregation: Option, /// The name of the event (valid only when the group `type` is `event`). /// /// Note: If not specified, the prefix is used. If the prefix is empty (or unspecified), the name is required. @@ -694,6 +701,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, name: None, display_name: None, body: None, @@ -859,6 +867,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, name: None, display_name: None, body: None, @@ -1145,6 +1154,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, display_name: None, attributes: vec![], body: Some(AnyValueSpec::String { @@ -1360,6 +1370,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, display_name: None, attributes: vec![], body: Some(AnyValueSpec::String { @@ -1515,6 +1526,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, name: None, display_name: None, body: None, @@ -1586,6 +1598,7 @@ mod tests { group.metric_name = Some("test".to_owned()); group.instrument = Some(Counter); group.unit = Some("test".to_owned()); + group.aggregation = None; let result = group.validate("").into_result_failing_non_fatal(); assert_eq!( Err(InvalidGroupStability { @@ -1685,6 +1698,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, name: None, display_name: None, body: None, @@ -1776,6 +1790,7 @@ mod tests { group.metric_name = Some("test".to_owned()); group.instrument = Some(Counter); group.unit = Some("test".to_owned()); + group.aggregation = None; assert!(group .validate("") .into_result_failing_non_fatal() @@ -1837,6 +1852,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, name: None, display_name: None, body: None, @@ -1897,6 +1913,7 @@ mod tests { metric_name: Some("metric".to_owned()), instrument: Some(Gauge), unit: Some("{thing}".to_owned()), + aggregation: None, name: None, display_name: None, body: None, @@ -1913,6 +1930,7 @@ mod tests { group.metric_name = None; group.instrument = None; group.unit = None; + group.aggregation = None; group.span_kind = Some(SpanKindSpec::Client); assert!(group .validate("") diff --git a/crates/weaver_semconv/src/lib.rs b/crates/weaver_semconv/src/lib.rs index 37b7c9cac..074c7bc21 100644 --- a/crates/weaver_semconv/src/lib.rs +++ b/crates/weaver_semconv/src/lib.rs @@ -13,6 +13,7 @@ use std::path::PathBuf; use weaver_common::diagnostic::{DiagnosticMessage, DiagnosticMessages}; use weaver_common::error::{format_errors, WeaverError}; +pub mod aggregation; pub mod any_value; pub mod attribute; pub mod deprecated; diff --git a/crates/weaver_semconv/src/registry.rs b/crates/weaver_semconv/src/registry.rs index 9160b9498..f2cd1043a 100644 --- a/crates/weaver_semconv/src/registry.rs +++ b/crates/weaver_semconv/src/registry.rs @@ -348,6 +348,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, brief: "brief".to_owned(), note: "note".to_owned(), extends: None, @@ -375,6 +376,7 @@ mod tests { metric_name: None, instrument: None, unit: None, + aggregation: None, brief: "brief".to_owned(), note: "note".to_owned(), extends: None, From 71e1ca56ddf8c121550f905f03d42153509e0058 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 24 Jul 2025 12:40:49 +1000 Subject: [PATCH 2/2] Add in aggregation method --- crates/weaver_semconv/src/aggregation.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/weaver_semconv/src/aggregation.rs b/crates/weaver_semconv/src/aggregation.rs index d18393ca4..e70d25139 100644 --- a/crates/weaver_semconv/src/aggregation.rs +++ b/crates/weaver_semconv/src/aggregation.rs @@ -11,9 +11,12 @@ use crate::YamlValue; #[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq, JsonSchema)] #[serde(deny_unknown_fields)] pub struct AggregationSpec { + /// The method of aggregation to be used + #[serde(skip_serializing_if = "Option::is_none")] + pub method: Option, /// The parameters used in the aggregation #[serde(skip_serializing_if = "Option::is_none")] - pub parameters: Option>, + pub parameters: Option>, } impl AggregationSpec { @@ -22,4 +25,22 @@ impl AggregationSpec { pub fn parameters(&self) -> &Option> { &self.parameters } +} + +/// The Aggregation Method +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum AggregationMethodSpec { + /// Use the Default Aggregation Method. + Default, + /// Use the Sum Aggregation Method. + Sum, + /// Use the Last Value Aggregation Method. + LastValue, + /// Use the Histogram Aggregation Method. + Histogram, + /// Use the Explicit Histogram Aggregation Method. + ExplicitHistogram, + /// Use the Exponential Histogram Aggregation Method. + ExponentialHistogram, } \ No newline at end of file