From 766561d972b06c49125810a7c79531c4c0b71fff Mon Sep 17 00:00:00 2001 From: neuronull Date: Thu, 5 Oct 2023 15:55:08 -0600 Subject: [PATCH 1/3] chore(metrics): improve creation of Origin metadata structures --- lib/vector-core/src/event/metadata.rs | 35 +++---- lib/vector-core/src/event/proto.rs | 27 ++--- src/sinks/datadog/metrics/encoder.rs | 48 +++++---- src/sources/datadog_agent/metrics.rs | 11 +- src/transforms/log_to_metric.rs | 140 +++++++++++++++++++------- 5 files changed, 158 insertions(+), 103 deletions(-) diff --git a/lib/vector-core/src/event/metadata.rs b/lib/vector-core/src/event/metadata.rs index a592a328a9c8d..2803c3d5fb1de 100644 --- a/lib/vector-core/src/event/metadata.rs +++ b/lib/vector-core/src/event/metadata.rs @@ -76,38 +76,31 @@ pub struct DatadogMetricOriginMetadata { } impl DatadogMetricOriginMetadata { - /// Replaces the `OriginProduct`. + /// Creates a new `DatadogMetricOriginMetadata`. + /// When Vector sends out metrics containing the Origin metadata, it should do so with + /// all of the fields defined. + /// The edge case where the Orign metadata is created within a component and does not + /// initially contain all of the metadata fields, is in the `log_to_metric` transform. #[must_use] - pub fn with_product(mut self, product: u32) -> Self { - self.product = Some(product); - self - } - - /// Replaces the `OriginCategory`. - #[must_use] - pub fn with_category(mut self, category: u32) -> Self { - self.category = Some(category); - self - } - - /// Replaces the `OriginService`. - #[must_use] - pub fn with_service(mut self, service: u32) -> Self { - self.service = Some(service); - self + pub fn new(product: Option, category: Option, service: Option) -> Self { + Self { + product, + category, + service, + } } - /// Returns a reference to the `OriginProduct` + /// Returns a reference to the `OriginProduct`. pub fn product(&self) -> Option { self.product } - /// Returns a reference to the `OriginCategory` + /// Returns a reference to the `OriginCategory`. pub fn category(&self) -> Option { self.category } - /// Returns a reference to the `OriginService` + /// Returns a reference to the `OriginService`. pub fn service(&self) -> Option { self.service } diff --git a/lib/vector-core/src/event/proto.rs b/lib/vector-core/src/event/proto.rs index 7dfb59c32d5b5..ebe25c5a44ea8 100644 --- a/lib/vector-core/src/event/proto.rs +++ b/lib/vector-core/src/event/proto.rs @@ -564,30 +564,17 @@ fn decode_metadata( ) { let value = input.value.and_then(decode_value); - let datadog_origin_metadata = input - .datadog_origin_metadata - .as_ref() - .map(decode_origin_metadata); + let datadog_origin_metadata = input.datadog_origin_metadata.as_ref().map(|input| { + super::DatadogMetricOriginMetadata::new( + input.origin_product, + input.origin_category, + input.origin_service, + ) + }); (value, datadog_origin_metadata) } -fn decode_origin_metadata(input: &DatadogOriginMetadata) -> super::DatadogMetricOriginMetadata { - let mut origin = super::DatadogMetricOriginMetadata::default(); - - if let Some(product) = input.origin_product { - origin = origin.with_product(product); - } - if let Some(category) = input.origin_category { - origin = origin.with_category(category); - } - if let Some(service) = input.origin_service { - origin = origin.with_service(service); - } - - origin -} - fn decode_value(input: Value) -> Option { match input.kind { Some(value::Kind::RawBytes(data)) => Some(event::Value::Bytes(data)), diff --git a/src/sinks/datadog/metrics/encoder.rs b/src/sinks/datadog/metrics/encoder.rs index 3ffcf8c5a3dda..e2325e00121b5 100644 --- a/src/sinks/datadog/metrics/encoder.rs +++ b/src/sinks/datadog/metrics/encoder.rs @@ -415,12 +415,23 @@ fn generate_sketch_metadata( origin_product_value: u32, ) -> Option { generate_origin_metadata(maybe_pass_through, maybe_source_type, origin_product_value).map( - |origin| ddmetric_proto::Metadata { - origin: Some(ddmetric_proto::Origin { - origin_product: origin.product().expect("OriginProduct should be set"), - origin_category: origin.category().expect("OriginCategory should be set"), - origin_service: origin.service().expect("OriginService should be set"), - }), + |origin| { + if origin.product().is_none() | origin.category().is_none() | origin.service().is_none() + { + warn!( + message = "Generated sketch origin metadata should have each field set.", + product = origin.product(), + category = origin.category(), + service = origin.service() + ); + } + ddmetric_proto::Metadata { + origin: Some(ddmetric_proto::Origin { + origin_product: origin.product().expect("OriginProduct should be set"), + origin_category: origin.category().expect("OriginCategory should be set"), + origin_service: origin.service().expect("OriginService should be set"), + }), + } }, ) } @@ -626,12 +637,11 @@ fn generate_origin_metadata( // - `log_to_metric` transform set the OriginService in the EventMetadata when it creates // the new metric. if let Some(pass_through) = maybe_pass_through { - Some( - DatadogMetricOriginMetadata::default() - .with_product(pass_through.product().unwrap_or(origin_product_value)) - .with_category(pass_through.category().unwrap_or(ORIGIN_CATEGORY_VALUE)) - .with_service(pass_through.service().unwrap_or(no_value)), - ) + Some(DatadogMetricOriginMetadata::new( + pass_through.product().or(Some(origin_product_value)), + pass_through.category().or(Some(ORIGIN_CATEGORY_VALUE)), + pass_through.service().or(Some(no_value)), + )) // No metadata has been set upstream } else { @@ -640,10 +650,11 @@ fn generate_origin_metadata( // In order to preserve consistent behavior, we intentionally don't set origin metadata // for the case where the Datadog Agent did not set it. source_type_to_service(source_type).map(|origin_service_value| { - DatadogMetricOriginMetadata::default() - .with_product(origin_product_value) - .with_category(ORIGIN_CATEGORY_VALUE) - .with_service(origin_service_value) + DatadogMetricOriginMetadata::new( + Some(origin_product_value), + Some(ORIGIN_CATEGORY_VALUE), + Some(origin_service_value), + ) }) }) } @@ -1073,10 +1084,7 @@ mod tests { let service = 9; let event_metadata = EventMetadata::default().with_origin_metadata( - DatadogMetricOriginMetadata::default() - .with_product(product) - .with_category(category) - .with_service(service), + DatadogMetricOriginMetadata::new(Some(product), Some(category), Some(service)), ); let counter = get_simple_counter_with_metadata(event_metadata); diff --git a/src/sources/datadog_agent/metrics.rs b/src/sources/datadog_agent/metrics.rs index e4b146b933852..ee1c998c13ced 100644 --- a/src/sources/datadog_agent/metrics.rs +++ b/src/sources/datadog_agent/metrics.rs @@ -244,12 +244,11 @@ fn get_event_metadata(metadata: Option<&Metadata>) -> EventMetadata { origin.origin_category, origin.origin_service, ); - EventMetadata::default().with_origin_metadata( - DatadogMetricOriginMetadata::default() - .with_product(origin.origin_product) - .with_category(origin.origin_category) - .with_service(origin.origin_service), - ) + EventMetadata::default().with_origin_metadata(DatadogMetricOriginMetadata::new( + Some(origin.origin_product), + Some(origin.origin_category), + Some(origin.origin_service), + )) }) } diff --git a/src/transforms/log_to_metric.rs b/src/transforms/log_to_metric.rs index 0571d715c6cc6..f6ba686e09de2 100644 --- a/src/transforms/log_to_metric.rs +++ b/src/transforms/log_to_metric.rs @@ -268,9 +268,11 @@ fn to_metric(config: &MetricConfig, event: &Event) -> Result Date: Thu, 5 Oct 2023 16:01:26 -0600 Subject: [PATCH 2/3] spell checker --- lib/vector-core/src/event/metadata.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vector-core/src/event/metadata.rs b/lib/vector-core/src/event/metadata.rs index 2803c3d5fb1de..a3b82c0b3f427 100644 --- a/lib/vector-core/src/event/metadata.rs +++ b/lib/vector-core/src/event/metadata.rs @@ -79,7 +79,7 @@ impl DatadogMetricOriginMetadata { /// Creates a new `DatadogMetricOriginMetadata`. /// When Vector sends out metrics containing the Origin metadata, it should do so with /// all of the fields defined. - /// The edge case where the Orign metadata is created within a component and does not + /// The edge case where the Origin metadata is created within a component and does not /// initially contain all of the metadata fields, is in the `log_to_metric` transform. #[must_use] pub fn new(product: Option, category: Option, service: Option) -> Self { From 0298eeda9ac3109d1e22aa4e4873331b1a6e5e2a Mon Sep 17 00:00:00 2001 From: neuronull Date: Fri, 6 Oct 2023 10:41:59 -0600 Subject: [PATCH 3/3] feedback ds --- src/sinks/datadog/metrics/encoder.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/sinks/datadog/metrics/encoder.rs b/src/sinks/datadog/metrics/encoder.rs index e2325e00121b5..60b1772986836 100644 --- a/src/sinks/datadog/metrics/encoder.rs +++ b/src/sinks/datadog/metrics/encoder.rs @@ -416,7 +416,9 @@ fn generate_sketch_metadata( ) -> Option { generate_origin_metadata(maybe_pass_through, maybe_source_type, origin_product_value).map( |origin| { - if origin.product().is_none() | origin.category().is_none() | origin.service().is_none() + if origin.product().is_none() + || origin.category().is_none() + || origin.service().is_none() { warn!( message = "Generated sketch origin metadata should have each field set.", @@ -427,9 +429,9 @@ fn generate_sketch_metadata( } ddmetric_proto::Metadata { origin: Some(ddmetric_proto::Origin { - origin_product: origin.product().expect("OriginProduct should be set"), - origin_category: origin.category().expect("OriginCategory should be set"), - origin_service: origin.service().expect("OriginService should be set"), + origin_product: origin.product().unwrap_or_default(), + origin_category: origin.category().unwrap_or_default(), + origin_service: origin.service().unwrap_or_default(), }), } },