From ff76f9c0cf83c4f61e15508971217ac068812e85 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Mon, 1 Apr 2024 12:49:49 -0400 Subject: [PATCH] Add attribute registry as a parameter When updating markdown, allow attribute registry links to be specified via command line argument. --- crates/weaver_semconv_gen/src/gen.rs | 39 ++++++++++++++++------------ crates/weaver_semconv_gen/src/lib.rs | 16 ++++++++---- src/registry/update_markdown.rs | 14 +++++++--- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/crates/weaver_semconv_gen/src/gen.rs b/crates/weaver_semconv_gen/src/gen.rs index 63d4bc89..9157e4a8 100644 --- a/crates/weaver_semconv_gen/src/gen.rs +++ b/crates/weaver_semconv_gen/src/gen.rs @@ -120,23 +120,28 @@ impl<'a> AttributeView<'a> { } } - fn write_registry_link(&self, out: &mut T) -> Result<(), Error> { + fn write_registry_link(&self, out: &mut T, prefix: &str) -> Result<(), Error> { let reg_name = self.attribute.name.split('.').next().unwrap_or(""); - // TODO - the existing build-tools semconv will look at currently - // generating markdown location to see if it's the same structure - // as where the attribute originated from. - // - // Going forward, link vs. not link should be an option in generation. - // OR we should move this to a template-render scenario. - Ok(write!(out, "../attributes-registry/{reg_name}.md")?) + // TODO - We should try to link to the name itself, instead + // of just the correct group. + Ok(write!(out, "{prefix}/{reg_name}.md")?) } - fn write_name_with_optional_link(&self, out: &mut Out) -> Result<(), Error> { - write!(out, "[`")?; - self.write_name(out)?; - write!(out, "`](")?; - self.write_registry_link(out)?; - write!(out, ")")?; + fn write_name_with_optional_link( + &self, + out: &mut Out, + attribute_registry_base_url: Option<&str>, + ) -> Result<(), Error> { + match attribute_registry_base_url { + Some(prefix) => { + write!(out, "[`")?; + self.write_name(out)?; + write!(out, "`](")?; + self.write_registry_link(out, prefix)?; + write!(out, ")")?; + } + None => self.write_name(out)?, + } Ok(()) } @@ -337,6 +342,7 @@ impl<'a> AttributeTableView<'a> { out: &mut Out, args: &GenerateMarkdownArgs, ctx: &mut GenerateMarkdownContext, + attribute_registry_base_url: Option<&str>, ) -> Result<(), Error> { if self.group.r#type == GroupType::Event { write!(out, "The event name MUST be `{}`\n\n", self.event_name())?; @@ -366,7 +372,7 @@ impl<'a> AttributeTableView<'a> { for attr in &attributes { write!(out, "| ")?; - attr.write_name_with_optional_link(out)?; + attr.write_name_with_optional_link(out, attribute_registry_base_url)?; write!(out, " | ")?; attr.write_type_string(out)?; write!(out, " | ")?; @@ -401,9 +407,8 @@ impl<'a> AttributeTableView<'a> { "and SHOULD be provided **at span creation time** (if provided at all):\n\n" )?; for a in sampling_relevant { - // TODO - existing output uses registry-link-name. write!(out, "* ")?; - a.write_name_with_optional_link(out)?; + a.write_name_with_optional_link(out, attribute_registry_base_url)?; writeln!(out)?; } } diff --git a/crates/weaver_semconv_gen/src/lib.rs b/crates/weaver_semconv_gen/src/lib.rs index 84301f1c..ee342666 100644 --- a/crates/weaver_semconv_gen/src/lib.rs +++ b/crates/weaver_semconv_gen/src/lib.rs @@ -127,6 +127,7 @@ impl GenerateMarkdownArgs { fn generate_markdown_snippet( lookup: &ResolvedSemconvRegistry, args: GenerateMarkdownArgs, + attribute_registry_base_url: Option<&str>, ) -> Result { let mut ctx = GenerateMarkdownContext::default(); let mut result = String::new(); @@ -135,7 +136,7 @@ fn generate_markdown_snippet( view.generate_markdown(&mut result, &mut ctx)?; } else { let other = AttributeTableView::try_new(args.id.as_str(), lookup)?; - other.generate_markdown(&mut result, &args, &mut ctx)?; + other.generate_markdown(&mut result, &args, &mut ctx, attribute_registry_base_url)?; } Ok(result) } @@ -144,6 +145,7 @@ fn generate_markdown_snippet( fn update_markdown_contents( contents: &str, lookup: &ResolvedSemconvRegistry, + attribute_registry_base_url: Option<&str>, ) -> Result { let mut result = String::new(); let mut handling_snippet = false; @@ -165,7 +167,7 @@ fn update_markdown_contents( if parser::is_markdown_snippet_directive(line) { handling_snippet = true; let arg = parser::parse_markdown_snippet_directive(line)?; - let snippet = generate_markdown_snippet(lookup, arg)?; + let snippet = generate_markdown_snippet(lookup, arg, attribute_registry_base_url)?; result.push_str(&snippet); } } @@ -178,9 +180,11 @@ pub fn update_markdown( file: &str, lookup: &ResolvedSemconvRegistry, dry_run: bool, + attribute_registry_base_url: Option<&str>, ) -> Result<(), Error> { let original_markdown = fs::read_to_string(file)?; - let updated_markdown = update_markdown_contents(&original_markdown, lookup)?; + let updated_markdown = + update_markdown_contents(&original_markdown, lookup, attribute_registry_base_url)?; if !dry_run { fs::write(file, updated_markdown)?; Ok(()) @@ -273,17 +277,19 @@ mod tests { fn test_http_semconv() -> Result<(), Error> { let logger = TestLogger::default(); let lookup = ResolvedSemconvRegistry::try_from_path("data/**/*.yaml", logger.clone())?; - + let attribute_registry_url = "../attributes-registry"; // Check our test files. force_print_error(update_markdown( "data/http-span-full-attribute-table.md", &lookup, true, + Some(attribute_registry_url), )); force_print_error(update_markdown( "data/http-metric-semconv.md", &lookup, true, + Some(attribute_registry_url), )); Ok(()) } @@ -309,6 +315,6 @@ mod tests { let lookup = ResolvedSemconvRegistry::try_from_path(&semconv_path, logger.clone())?; let test_path = path.join("test.md").display().to_string(); // Attempts to update the test - will fail if there is any difference in the generated markdown. - update_markdown(&test_path, &lookup, true) + update_markdown(&test_path, &lookup, true, None) } } diff --git a/src/registry/update_markdown.rs b/src/registry/update_markdown.rs index cf6f59db..c2fb5623 100644 --- a/src/registry/update_markdown.rs +++ b/src/registry/update_markdown.rs @@ -30,6 +30,11 @@ pub struct RegistryUpdateMarkdownArgs { /// Whether or not to run updates in dry-run mode. #[arg(long, default_value = "false")] pub dry_run: bool, + + /// Optional path to the attribute registry. + /// If provided, all attributes will be linked here. + #[arg(long)] + pub attribute_regsitry_base_url: Option, } /// Update markdown files. @@ -67,9 +72,12 @@ pub(crate) fn command( }) { log.info(&format!("{}: ${}", operation, entry.path().display())); - if let Err(error) = - update_markdown(&entry.path().display().to_string(), ®istry, args.dry_run) - { + if let Err(error) = update_markdown( + &entry.path().display().to_string(), + ®istry, + args.dry_run, + args.attribute_regsitry_base_url.as_deref(), + ) { has_error = true; log.error(&format!("{error}")); }