Merge pull request open-telemetry#76 from jsuereth/pr-attribute-regis…

…try-parameter

Attribute registry location is now an argument to markdown update

crates/weaver_semconv_gen/legacy_tests/parameter_tag/test.md

@@ -1,16 +1,16 @@
 # DB
 
-<!-- Note: Compared to build-tools, we've removed any-of constraint text *AND* added attribute registry links. -->
+<!-- Note: Compared to build-tools, we've removed any-of constraint texts. -->
 
 <!-- semconv db(tag=connection-level) -->
 | Attribute  | Type | Description  | Examples  | [Requirement Level](https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/) | Stability |
 |---|---|---|---|---|
-| [`db.type`](../attributes-registry/db.md) | string | Database type. For any SQL database, "sql". For others, the lower-case database category. | `sql`; `cassandra`; `hbase`; `mongodb`; `redis`; `couchbase`; `couchdb` | Required |
-| [`db.connection_string`](../attributes-registry/db.md) | string | The connection string used to connect to the database. [1] | `Server=(localdb)\v11.0;Integrated Security=true;` | Recommended |
-| [`db.user`](../attributes-registry/db.md) | string | Username for accessing the database. | `readonly_user`; `reporting_user` | Recommended |
-| [`net.peer.ip`](../attributes-registry/net.md) | string | Remote address of the peer (dotted decimal for IPv4 or [RFC5952](https://tools.ietf.org/html/rfc5952) for IPv6) | `127.0.0.1` | Recommended |
-| [`net.peer.name`](../attributes-registry/net.md) | string | Remote hostname or similar, see note below. | `example.com` | Recommended |
-| [`net.peer.port`](../attributes-registry/net.md) | int | Remote port number. | `80`; `8080`; `443` | Recommended |
+| db.type | string | Database type. For any SQL database, "sql". For others, the lower-case database category. | `sql`; `cassandra`; `hbase`; `mongodb`; `redis`; `couchbase`; `couchdb` | Required |
+| db.connection_string | string | The connection string used to connect to the database. [1] | `Server=(localdb)\v11.0;Integrated Security=true;` | Recommended |
+| db.user | string | Username for accessing the database. | `readonly_user`; `reporting_user` | Recommended |
+| net.peer.ip | string | Remote address of the peer (dotted decimal for IPv4 or [RFC5952](https://tools.ietf.org/html/rfc5952) for IPv6) | `127.0.0.1` | Recommended |
+| net.peer.name | string | Remote hostname or similar, see note below. | `example.com` | Recommended |
+| net.peer.port | int | Remote port number. | `80`; `8080`; `443` | Recommended |
 
 **[1]:** It is recommended to remove embedded credentials.
 

crates/weaver_semconv_gen/src/gen.rs

@@ -120,23 +120,28 @@ impl<'a> AttributeView<'a> {
         }
     }
 
-    fn write_registry_link<T: Write>(&self, out: &mut T) -> Result<(), Error> {
+    fn write_registry_link<T: Write>(&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<Out: Write>(&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<Out: Write>(
+        &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)?;
             }
         }

crates/weaver_semconv_gen/src/lib.rs

@@ -128,6 +128,7 @@ impl GenerateMarkdownArgs {
 fn generate_markdown_snippet(
     lookup: &ResolvedSemconvRegistry,
     args: GenerateMarkdownArgs,
+    attribute_registry_base_url: Option<&str>,
 ) -> Result<String, Error> {
     let mut ctx = GenerateMarkdownContext::default();
     let mut result = String::new();
@@ -136,7 +137,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)
 }
@@ -145,6 +146,7 @@ fn generate_markdown_snippet(
 fn update_markdown_contents(
     contents: &str,
     lookup: &ResolvedSemconvRegistry,
+    attribute_registry_base_url: Option<&str>,
 ) -> Result<String, Error> {
     let mut result = String::new();
     let mut handling_snippet = false;
@@ -166,7 +168,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);
             }
         }
@@ -179,9 +181,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(())
@@ -266,17 +270,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(())
     }
@@ -302,6 +308,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)
     }
 }

src/registry/update_markdown.rs

@@ -31,6 +31,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_registry_base_url: Option<String>,
 }
 
 /// 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(), &registry, args.dry_run)
-        {
+        if let Err(error) = update_markdown(
+            &entry.path().display().to_string(),
+            &registry,
+            args.dry_run,
+            args.attribute_registry_base_url.as_deref(),
+        ) {
             has_error = true;
             log.error(&format!("{error}"));
         }