diff --git a/Cargo.lock b/Cargo.lock index 7e11d824..ebd82bd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -93,9 +93,9 @@ dependencies = [ [[package]] name = "anstyle-query" -version = "1.0.3" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a64c907d4e79225ac72e2a354c9ce84d50ebb4586dee56c82b3ee73004f537f5" +checksum = "ad186efb764318d35165f1758e7dcef3b10628e26d41a44bc5550652e6804391" dependencies = [ "windows-sys 0.52.0", ] @@ -3559,9 +3559,9 @@ checksum = "d4c87d22b6e3f4a18d4d40ef354e97c90fcb14dd91d7dc0aa9d8a1172ebf7202" [[package]] name = "unicode-width" -version = "0.1.12" +version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68f5e5f3158ecfd4b8ff6fe086db7c8467a2dfdac97fe420f2b7c4aa97af66d6" +checksum = "0336d538f7abc86d282a4189614dfaa90810dfc2c6f6427eaf88e16311dd225d" [[package]] name = "unicode-xid" diff --git a/crates/weaver_codegen_test/build.rs b/crates/weaver_codegen_test/build.rs index 43d823d2..1ea3c589 100644 --- a/crates/weaver_codegen_test/build.rs +++ b/crates/weaver_codegen_test/build.rs @@ -52,7 +52,8 @@ fn main() { let params: Params = serde_yaml::from_str( r#"params: attributes: true - metrics: true"#, + metrics: true + registry_prefix: \"registry.\""#, ) .unwrap_or_else(|e| process_error(&logger, e)); let loader = FileSystemFileLoader::try_new(TEMPLATES_PATH.into(), TARGET) diff --git a/crates/weaver_codegen_test/templates/registry/rust/weaver.yaml b/crates/weaver_codegen_test/templates/registry/rust/weaver.yaml index 7a2fb6d5..bc340bca 100644 --- a/crates/weaver_codegen_test/templates/registry/rust/weaver.yaml +++ b/crates/weaver_codegen_test/templates/registry/rust/weaver.yaml @@ -7,6 +7,11 @@ type_mapping: template[string]: String # Not yet properly handled in codegen template[string[]]: Vec # Not yet properly handled in codegen +# Default parameter values +params: + attributes: true + metrics: true + templates: - pattern: README.md filter: . @@ -23,17 +28,20 @@ templates: # - groups are deduplicated by namespace. # - groups are sorted by namespace. filter: > - .groups - | map(select(.id | startswith("registry."))) - | map(select(.type == "attribute_group") - | { - id, - type, - brief, - prefix}) - | unique_by(.id | split(".") | .[1]) - | sort_by(.id | split(".") | .[1]) - if: params.attributes is undefined or params.attributes + if $attributes then + .groups + | map(select(.id | startswith("registry."))) + | map(select(.type == "attribute_group") + | { + id, + type, + brief, + prefix}) + | unique_by(.id | split(".") | .[1]) + | sort_by(.id | split(".") | .[1]) + else + empty + end application_mode: single - pattern: attributes/attributes.rs.j2 # The following JQ filter extracts the id, type, brief, prefix, and attributes of groups matching the following @@ -42,25 +50,28 @@ templates: # - groups of the type `attribute_group`. # - groups are sorted by namespace. filter: > - .groups - | map(select(.id | startswith("registry."))) - | map(select(.type == "attribute_group") - | { - id, - type, - brief, - prefix, - attributes}) - | group_by(.id | split(".") | .[1]) - | map({ - id: (map(select(.id | endswith(".deprecated") | not)) | first).id, - type: (map(select(.id | endswith(".deprecated") | not)) | first).type, - brief: (map(select(.id | endswith(".deprecated") | not)) | first).brief, - prefix: (map(select(.id | endswith(".deprecated") | not)) | first).prefix, - attributes: map(.attributes) | add - }) - | sort_by(.id | split(".") | .[1]) - if: params.attributes is undefined or params.attributes + if $attributes then + .groups + | map(select(.id | startswith("registry."))) + | map(select(.type == "attribute_group") + | { + id, + type, + brief, + prefix, + attributes}) + | group_by(.id | split(".") | .[1]) + | map({ + id: (map(select(.id | endswith(".deprecated") | not)) | first).id, + type: (map(select(.id | endswith(".deprecated") | not)) | first).type, + brief: (map(select(.id | endswith(".deprecated") | not)) | first).brief, + prefix: (map(select(.id | endswith(".deprecated") | not)) | first).prefix, + attributes: map(.attributes) | add + }) + | sort_by(.id | split(".") | .[1]) + else + empty + end application_mode: each # Templates for the `metric` group type @@ -71,17 +82,20 @@ templates: # - groups are deduplicated by namespace. # - groups are sorted by prefix. filter: > - .groups - | map(select(.id | startswith("metric."))) - | map(select(.type == "metric") - | { - id, - type, - brief, - prefix}) - | unique_by(.id | split(".") | .[1]) - | sort_by(.id | split(".") | .[1]) - if: params.metrics is undefined or params.metrics + if $metrics then + .groups + | map(select(.id | startswith("metric."))) + | map(select(.type == "metric") + | { + id, + type, + brief, + prefix}) + | unique_by(.id | split(".") | .[1]) + | sort_by(.id | split(".") | .[1]) + else + empty + end application_mode: single - pattern: metrics/metrics.rs.j2 # The following JQ filter extracts the id, type, brief, prefix, and attributes of groups matching the following @@ -90,14 +104,17 @@ templates: # - groups of the type `metric`. # - groups are sorted by namespace. filter: > - .groups - | map(select(.id | startswith("metric."))) - | group_by(.id | split(".") | .[1]) - | map({ - prefix: .[0].id | split(".") | .[1], - groups: . - }) - if: params.metrics is undefined or params.metrics + if $metrics then + .groups + | map(select(.id | startswith("metric."))) + | group_by(.id | split(".") | .[1]) + | map({ + prefix: .[0].id | split(".") | .[1], + groups: . + }) + else + empty + end application_mode: each diff --git a/crates/weaver_codegen_test/tests/codegen.rs b/crates/weaver_codegen_test/tests/codegen.rs index 509a1f7d..34f6f883 100644 --- a/crates/weaver_codegen_test/tests/codegen.rs +++ b/crates/weaver_codegen_test/tests/codegen.rs @@ -28,8 +28,8 @@ use opentelemetry::{global, KeyValue}; #[test] fn test_codegen() { // Use a format! macro to avoid compiler optimization - assert_eq!(format!("{}",attributes::ATTRIBUTES_PARAM), "true"); - assert_eq!(format!("{}",metrics::METRICS_PARAM), "true"); + assert_eq!(format!("{}", attributes::ATTRIBUTES_PARAM), "true"); + assert_eq!(format!("{}", metrics::METRICS_PARAM), "true"); // Test the constants generated for the attributes // In the generated API the attributes are typed, so the compiler will catch type errors diff --git a/crates/weaver_forge/src/config.rs b/crates/weaver_forge/src/config.rs index f4cbe5ed..d1894394 100644 --- a/crates/weaver_forge/src/config.rs +++ b/crates/weaver_forge/src/config.rs @@ -15,7 +15,6 @@ use serde_yaml::Value; use crate::error::Error; use crate::error::Error::InvalidConfigFile; use crate::file_loader::FileLoader; -use crate::filter::Filter; use crate::WEAVER_YAML; /// Case convention for naming of functions and structs. @@ -98,104 +97,77 @@ fn default_templates() -> Vec { vec![ TemplateConfig { pattern: Glob::new("**/registry.md").expect("Invalid pattern"), - filter: Filter::try_new(".").expect("Invalid filter"), - r#if: None, + filter: ".".to_owned(), application_mode: ApplicationMode::Single, }, TemplateConfig { pattern: Glob::new("**/attribute_group.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"attribute_group\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"attribute_group\")".to_owned(), application_mode: ApplicationMode::Each, }, TemplateConfig { pattern: Glob::new("**/attribute_groups.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"attribute_group\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"attribute_group\")".to_owned(), application_mode: ApplicationMode::Single, }, TemplateConfig { pattern: Glob::new("**/event.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"event\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"event\")".to_owned(), application_mode: ApplicationMode::Each, }, TemplateConfig { pattern: Glob::new("**/events.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"event\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"event\")".to_owned(), application_mode: ApplicationMode::Single, }, TemplateConfig { pattern: Glob::new("**/group.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups").expect("Invalid filter"), - r#if: None, + filter: ".groups".to_owned(), application_mode: ApplicationMode::Each, }, TemplateConfig { pattern: Glob::new("**/groups.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups").expect("Invalid filter"), - r#if: None, + filter: ".groups".to_owned(), application_mode: ApplicationMode::Single, }, TemplateConfig { pattern: Glob::new("**/metric.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"metric\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"metric\")".to_owned(), application_mode: ApplicationMode::Each, }, TemplateConfig { pattern: Glob::new("**/metrics.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"metric\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"metric\")".to_owned(), application_mode: ApplicationMode::Single, }, TemplateConfig { pattern: Glob::new("**/resource.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"resource\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"resource\")".to_owned(), application_mode: ApplicationMode::Each, }, TemplateConfig { pattern: Glob::new("**/resources.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"resource\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"resource\")".to_owned(), application_mode: ApplicationMode::Single, }, TemplateConfig { pattern: Glob::new("**/scope.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"scope\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"scope\")".to_owned(), application_mode: ApplicationMode::Each, }, TemplateConfig { pattern: Glob::new("**/scopes.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"scope\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"scope\")".to_owned(), application_mode: ApplicationMode::Single, }, TemplateConfig { pattern: Glob::new("**/span.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"span\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"span\")".to_owned(), application_mode: ApplicationMode::Each, }, TemplateConfig { pattern: Glob::new("**/spans.md").expect("Invalid pattern"), - filter: Filter::try_new(".groups[] | select(.type == \"span\")") - .expect("Invalid filter"), - r#if: None, + filter: ".groups[] | select(.type == \"span\")".to_owned(), application_mode: ApplicationMode::Single, }, ] @@ -230,17 +202,20 @@ pub(crate) struct TemplateConfig { /// The filter to apply to the registry before applying the template. /// Applying a filter to a registry will return a list of elements from the /// registry that satisfy the filter. - pub(crate) filter: Filter, - /// An optional `if` condition that must be satisfied to apply the template. - /// The condition is a Jinja2 expression that can use the all the variables - /// defined in the template context (e.g. ctx, params). - pub(crate) r#if: Option, + /// By default, the filter is set to "." which means that the whole registry + /// is returned. + #[serde(default = "default_filter")] + pub(crate) filter: String, /// The mode to apply the template. /// `single`: Apply the template to the output of the filter as a whole. /// `each`: Apply the template to each item of the list returned by the filter. pub(crate) application_mode: ApplicationMode, } +fn default_filter() -> String { + ".".to_owned() +} + /// A template matcher. pub struct TemplateMatcher<'a> { templates: &'a [TemplateConfig], diff --git a/crates/weaver_forge/src/error.rs b/crates/weaver_forge/src/error.rs index a90e0ddd..78706abe 100644 --- a/crates/weaver_forge/src/error.rs +++ b/crates/weaver_forge/src/error.rs @@ -92,17 +92,6 @@ pub enum Error { error: String, }, - /// If expression evaluation failed. - #[error("If expression '{if_expr}' evaluation failed -> {error}")] - IfExprEvaluationFailed { - /// If expression - if_expr: String, - /// Error id used to deduplicate the error. - error_id: String, - /// Error message. - error: String, - }, - /// Invalid template directory. #[error("Invalid template directory: {0}")] InvalidTemplateDirectory(PathBuf), diff --git a/crates/weaver_forge/src/filter.rs b/crates/weaver_forge/src/filter.rs index d89897ad..66219927 100644 --- a/crates/weaver_forge/src/filter.rs +++ b/crates/weaver_forge/src/filter.rs @@ -5,7 +5,6 @@ use crate::error::Error; use core::fmt; use jaq_interpret::{Ctx, FilterT, RcIter, Val}; -use serde::de; use std::fmt::Debug; /// A filter that can be applied to a JSON value. @@ -17,8 +16,9 @@ pub struct Filter { impl Filter { /// Create a new filter from a string expression or return an error if the /// expression is invalid. - pub fn try_new(filter_expr: &str) -> Result { - let vars = Vec::new(); + /// The vars parameter is a list of variable names that can be used in the + /// filter expression. + pub fn try_new(filter_expr: &str, vars: Vec) -> Result { let mut ctx = jaq_interpret::ParseCtx::new(vars); ctx.insert_natives(jaq_core::core()); ctx.insert_defs(jaq_std::std()); @@ -49,9 +49,13 @@ impl Filter { } /// Apply the filter to a JSON value and return the result as a JSON value. - pub fn apply(&self, ctx: serde_json::Value) -> Result { + pub fn apply( + &self, + ctx: serde_json::Value, + jq_ctx: Vec, + ) -> Result { let inputs = RcIter::new(core::iter::empty()); - let filter_result = self.filter.run((Ctx::new([], &inputs), Val::from(ctx))); + let filter_result = self.filter.run((Ctx::new(jq_ctx, &inputs), Val::from(ctx))); let mut errs = Vec::new(); let mut values = Vec::new(); @@ -76,28 +80,97 @@ impl Debug for Filter { } } -struct FilterVisitor; - -impl<'de> de::Visitor<'de> for FilterVisitor { - type Value = Filter; - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("a filter string") - } - - fn visit_str(self, value: &str) -> Result - where - E: de::Error, - { - Filter::try_new(value).map_err(E::custom) - } -} - -impl<'de> de::Deserialize<'de> for Filter { - fn deserialize(deserializer: D) -> Result - where - D: de::Deserializer<'de>, - { - deserializer.deserialize_str(FilterVisitor) +#[cfg(test)] +mod tests { + use jaq_interpret::Val; + + #[test] + fn test_filter() { + let filter = super::Filter::try_new("true", Vec::new()).unwrap(); + let result = filter.apply(serde_json::json!({}), Vec::new()).unwrap(); + assert_eq!(result, serde_json::json!(true)); + + let filter = super::Filter::try_new(".", Vec::new()).unwrap(); + let result = filter.apply(serde_json::json!({}), Vec::new()).unwrap(); + assert_eq!(result, serde_json::Value::Object(serde_json::Map::new())); + + let filter = super::Filter::try_new(".", Vec::new()).unwrap(); + let result = filter + .apply( + serde_json::json!({ + "a": 1, + "b": 2, + }), + Vec::new(), + ) + .unwrap(); + assert_eq!( + result, + serde_json::json!({ + "a": 1, + "b": 2, + }) + ); + + let filter = super::Filter::try_new(".key1", Vec::new()).unwrap(); + let result = filter + .apply( + serde_json::json!({ + "key1": 1, + "key2": 2, + }), + Vec::new(), + ) + .unwrap(); + assert_eq!(result, serde_json::json!(1)); + + let filter = super::Filter::try_new(".[\"key1\"]", Vec::new()).unwrap(); + let result = filter + .apply( + serde_json::json!({ + "key1": 1, + "key2": 2, + }), + Vec::new(), + ) + .unwrap(); + assert_eq!(result, serde_json::json!(1)); + + let vars = vec!["key".to_owned()]; + let ctx = vec![Val::from(serde_json::Value::String("key1".to_owned()))]; + let filter = super::Filter::try_new(".[$key]", vars).unwrap(); + let result = filter + .apply( + serde_json::json!({ + "key1": 1, + "key2": 2, + }), + ctx, + ) + .unwrap(); + assert_eq!(result, serde_json::json!(1)); + + let jq_filter = r#" +if $incubating then + . +else + null +end"#; + let input = serde_json::json!({ + "key1": 1, + "key2": 2, + }); + let vars = vec!["incubating".to_owned()]; + // When incubating is true, the entire input is returned. + let ctx = vec![Val::from(serde_json::Value::Bool(true))]; + let filter = super::Filter::try_new(jq_filter, vars.clone()).unwrap(); + let result = filter.apply(input.clone(), ctx).unwrap(); + assert_eq!(result, input); + + // When incubating = false the filter should return an empty array + let ctx = vec![Val::from(serde_json::Value::Bool(false))]; + let filter = super::Filter::try_new(jq_filter, vars).unwrap(); + let result = filter.apply(input.clone(), ctx).unwrap(); + assert_eq!(result, serde_json::Value::Null); } } diff --git a/crates/weaver_forge/src/lib.rs b/crates/weaver_forge/src/lib.rs index b5d4e415..8cfde6a1 100644 --- a/crates/weaver_forge/src/lib.rs +++ b/crates/weaver_forge/src/lib.rs @@ -10,6 +10,7 @@ use std::fs; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; +use jaq_interpret::Val; use minijinja::syntax::SyntaxConfig; use minijinja::value::{from_args, Object}; use minijinja::{Environment, ErrorKind, State, Value}; @@ -27,9 +28,10 @@ use weaver_common::Logger; use crate::config::{ApplicationMode, Params, TargetConfig}; use crate::debug::error_summary; -use crate::error::Error::{IfExprEvaluationFailed, InvalidConfigFile}; +use crate::error::Error::InvalidConfigFile; use crate::extensions::{ansi, case, code, otel, util}; use crate::file_loader::FileLoader; +use crate::filter::Filter; use crate::registry::{ResolvedGroup, ResolvedRegistry}; pub mod config; @@ -112,6 +114,7 @@ impl ParamsObject { Self { params: new_params } } } + impl Display for ParamsObject { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.write_str(&format!("{:#?}", self.params)) @@ -241,6 +244,27 @@ impl TemplateEngine { error: e.to_string(), })?; + let mut errors = Vec::new(); + + // Build JQ context from the params. + let (jq_vars, jq_ctx): (Vec, Vec) = self + .target_config + .params + .iter() + .filter_map(|(k, v)| { + let json_value = match serde_json::to_value(v) { + Ok(json_value) => json_value, + Err(e) => { + errors.push(ContextSerializationFailed { + error: e.to_string(), + }); + return None; + } + }; + Some((k.clone(), json_value)) + }) + .unzip(); + // Process all files in parallel // - Filter the files that match the template pattern // - Apply the filter to the context @@ -251,11 +275,21 @@ impl TemplateEngine { // - If the application mode is each, the filtered context is // evaluated as an array of objects and each object is evaluated // independently and in parallel with the same template. - let errs = files + let mut errs = files .into_par_iter() .filter_map(|relative_path| { for template in tmpl_matcher.matches(relative_path.clone()) { - let filtered_result = match template.filter.apply(context.clone()) { + let filter = match Filter::try_new(template.filter.as_str(), jq_vars.clone()) { + Ok(filter) => filter, + Err(e) => return Some(e), + }; + // `jaq_interpret::val::Val` is not Sync, so we need to convert json_values to + // jaq_interpret::val::Val here. + let jq_ctx = jq_ctx + .iter() + .map(|v| Val::from(v.clone())) + .collect::>(); + let filtered_result = match filter.apply(context.clone(), jq_ctx) { Ok(result) => result, Err(e) => return Some(e), }; @@ -263,6 +297,13 @@ impl TemplateEngine { match template.application_mode { // The filtered result is evaluated as a single object ApplicationMode::Single => { + if filtered_result.is_null() + || (filtered_result.is_array() + && filtered_result.as_array().expect("is_array").is_empty()) + { + // Skip the template evaluation if the filtered result is null or an empty array + continue; + } if let Err(e) = self.evaluate_template( log.clone(), NewContext { @@ -270,7 +311,6 @@ impl TemplateEngine { } .try_into() .ok()?, - template.r#if.as_ref(), relative_path.as_path(), output_directive, output_dir, @@ -289,7 +329,6 @@ impl TemplateEngine { if let Err(e) = self.evaluate_template( log.clone(), NewContext { ctx: result }.try_into().ok()?, - template.r#if.as_ref(), relative_path.as_path(), output_directive, output_dir, @@ -309,7 +348,6 @@ impl TemplateEngine { } .try_into() .ok()?, - template.r#if.as_ref(), relative_path.as_path(), output_directive, output_dir, @@ -323,6 +361,7 @@ impl TemplateEngine { }) .collect::>(); + errs.extend(errors); handle_errors(errs) } @@ -332,7 +371,6 @@ impl TemplateEngine { &self, log: impl Logger + Clone + Sync, ctx: serde_json::Value, - if_expr: Option<&String>, template_path: &Path, output_directive: &OutputDirective, output_dir: &Path, @@ -359,29 +397,6 @@ impl TemplateEngine { Value::from_object(ParamsObject::new(self.target_config.params.clone())), ); - // Evaluate the if expression if it exists - if let Some(if_expr) = if_expr { - let compiled_if_expr = - engine - .compile_expression(if_expr) - .map_err(|e| IfExprEvaluationFailed { - if_expr: if_expr.clone(), - error_id: e.to_string(), - error: error_summary(e), - })?; - let result = - compiled_if_expr - .eval(ctx.clone()) - .map_err(|e| IfExprEvaluationFailed { - if_expr: if_expr.clone(), - error_id: e.to_string(), - error: error_summary(e), - })?; - if !result.is_true() { - return Ok(()); - } - } - let template = engine.get_template(template_file).map_err(|e| { let templates = engine .templates() @@ -503,7 +518,6 @@ mod tests { use crate::debug::print_dedup_errors; use crate::extensions::case::case_converter; use crate::file_loader::FileSystemFileLoader; - use crate::filter::Filter; use crate::registry::ResolvedRegistry; use crate::OutputDirective; @@ -642,8 +656,7 @@ mod tests { // for test coverage purposes. engine.target_config.templates.push(TemplateConfig { pattern: Glob::new("converter.md").unwrap(), - filter: Filter::try_new(".").unwrap(), - r#if: None, + filter: ".".to_owned(), application_mode: ApplicationMode::Single, });