Sort attributes by name, not by source (#205)

semantic-conventions/CHANGELOG.md

@@ -6,6 +6,8 @@ Please update the changelog as part of any significant pull request.
 
 - When an attribute is referenced using `ref:` from a group that already inherits the attribute with `extends:`, resolve the reference to the closest inherited attribute instead of the primary definition. This makes a difference in case the inherited reference overwrites any properties.
   ([#204](https://github.com/open-telemetry/build-tools/pull/204))
+- Sort attributes by name
+  ([#205](https://github.com/open-telemetry/build-tools/pull/205))
 
 ## v0.21.0
 

semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py

@@ -126,12 +126,15 @@ def _get_attributes(self, templates: Optional[bool]):
         if not hasattr(self, "attrs_by_name"):
             return []
 
-        return [
-            attr
-            for attr in self.attrs_by_name.values()
-            if templates is None
-            or templates == AttributeType.is_template_type(attr.attr_type)
-        ]
+        return sorted(
+            [
+                attr
+                for attr in self.attrs_by_name.values()
+                if templates is None
+                or templates == AttributeType.is_template_type(attr.attr_type)
+            ],
+            key=lambda attr: attr.fqn,
+        )
 
     def __init__(self, group):
         super().__init__(group)
@@ -431,32 +434,13 @@ def _populate_extends_single(self, semconv, unprocessed):
             parent_attributes = {}
             for ext_attr in extended.attributes:
                 parent_attributes[ext_attr.fqn] = ext_attr.inherit_attribute()
-            # By induction, parent semconv is already correctly sorted
-            parent_attributes.update(
-                SemanticConventionSet._sort_attributes_dict(semconv.attrs_by_name)
-            )
-            if parent_attributes or semconv.attributes:
-                semconv.attrs_by_name = parent_attributes
-        elif semconv.attributes:  # No parent, sort of current attributes
-            semconv.attrs_by_name = SemanticConventionSet._sort_attributes_dict(
-                semconv.attrs_by_name
-            )
+
+            parent_attributes.update(semconv.attrs_by_name)
+            semconv.attrs_by_name = parent_attributes
+
         # delete from remaining semantic conventions to process
         del unprocessed[semconv.semconv_id]
 
-    @staticmethod
-    def _sort_attributes_dict(
-        attributes: typing.Dict[str, SemanticAttribute]
-    ) -> typing.Dict[str, SemanticAttribute]:
-        """
-        First  imported, and then defined attributes.
-        :param attributes: Dictionary of attributes to sort
-        :return: A sorted dictionary of attributes
-        """
-        return dict(
-            sorted(attributes.items(), key=lambda kv: 0 if kv[1].imported else 1)
-        )
-
     def _populate_anyof_attributes(self):
         any_of: AnyOf
         for semconv in self.models.values():

semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py

@@ -207,10 +207,7 @@ def to_markdown_attribute_table(
         self, semconv: BaseSemanticConvention, output: io.StringIO
     ):
         attr_to_print = []
-        for attr in sorted(
-            semconv.attributes_and_templates,
-            key=lambda a: "" if a.ref is None else a.ref,
-        ):
+        for attr in semconv.attributes_and_templates:
             if self.render_ctx.group_key is not None:
                 if attr.tag == self.render_ctx.group_key:
                     attr_to_print.append(attr)

semantic-conventions/src/tests/data/markdown/deprecated/expected.md

@@ -4,14 +4,14 @@
 <!-- semconv http -->
 | Attribute  | Type | Description  | Examples  | Requirement Level |
 |---|---|---|---|---|
-| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required |
-| `http.url` | string | Full HTTP request URL in the form `scheme://host[:port]/path?query[#fragment]`. Usually the fragment is not transmitted over HTTP, but if it is known, it should be included nevertheless. | `https://www.foo.bar/search?q=OpenTelemetry#SemConv` | Recommended |
-| `http.target` | string | The full request target as passed in a HTTP request line or equivalent. | `/path/12314/?q=ddds#123` | Recommended |
+| `http.flavor` | string | **Deprecated. Use attribute `flavor_new` instead.**<br>Kind of HTTP protocol used [1] | `1.0` | Recommended |
 | `http.host` | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). When the header is empty or not present, this attribute should be the same. | `www.example.org` | Recommended |
+| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required |
 | `http.scheme` | string | The URI scheme identifying the used protocol. | `http`; `https` | Recommended |
 | `http.status_code` | int | [HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6). | `200` | Conditionally Required: if and only if one was received/sent |
 | `http.status_text` | string | **Deprecated: Use attribute `status_description` instead.**<br>[HTTP reason phrase](https://tools.ietf.org/html/rfc7230#section-3.1.2). | `OK` | Recommended |
-| `http.flavor` | string | **Deprecated. Use attribute `flavor_new` instead.**<br>Kind of HTTP protocol used [1] | `1.0` | Recommended |
+| `http.target` | string | The full request target as passed in a HTTP request line or equivalent. | `/path/12314/?q=ddds#123` | Recommended |
+| `http.url` | string | Full HTTP request URL in the form `scheme://host[:port]/path?query[#fragment]`. Usually the fragment is not transmitted over HTTP, but if it is known, it should be included nevertheless. | `https://www.foo.bar/search?q=OpenTelemetry#SemConv` | Recommended |
 | `http.user_agent` | string | Value of the [HTTP User-Agent](https://tools.ietf.org/html/rfc7231#section-5.5.3) header sent by the client. | `CERN-LineMode/2.15 libwww/2.17b3` | Recommended |
 
 **[1]:** If `net.transport` is not specified, it can be assumed to be `IP.TCP` except if `http.flavor` is `QUIC`, in which case `IP.UDP` is assumed.

semantic-conventions/src/tests/data/markdown/event/expected.md

@@ -5,10 +5,10 @@ The event name MUST be `exception`.
 
 | Attribute  | Type | Description  | Examples  | Requirement Level |
 |---|---|---|---|---|
-| `exception.type` | string | The type of the exception. | `java.net.ConnectException`; `OSError` | See below |
+| `exception.escaped` | boolean | SHOULD be set to true if the exception event is recorded at a point where it is known that the exception is escaping the scope of the span. [1] |  | Recommended |
 | `exception.message` | string | The exception message. | `Division by zero`; `Can't convert 'int' object to str implicitly` | See below |
 | `exception.stacktrace` | string | A stacktrace. | `Exception in thread "main" java.lang.RuntimeException: Test exception\n at com.example.GenerateTrace.methodB(GenerateTrace.java:13)\n at com.example.GenerateTrace.methodA(GenerateTrace.java:9)\n at com.example.GenerateTrace.main(GenerateTrace.java:5)` | Recommended |
-| `exception.escaped` | boolean | SHOULD be set to true if the exception event is recorded at a point where it is known that the exception is escaping the scope of the span. [1] |  | Recommended |
+| `exception.type` | string | The type of the exception. | `java.net.ConnectException`; `OSError` | See below |
 
 **[1]:** An exception is considered to have escaped.
 

semantic-conventions/src/tests/data/markdown/extend_constraint/expected.md

@@ -36,8 +36,8 @@ Some database systems may allow a connection to switch to a different `db.user`,
 <!-- semconv db(tag=connection-level) -->
 | Attribute  | Type | Description  | Examples  | Requirement Level |
 |---|---|---|---|---|
-| `db.system` | string | An identifier for the database management system (DBMS) product being used. See below for a list of well-known identifiers. | `other_sql` | Required |
 | `db.connection_string` | string | The connection string used to connect to the database. [1] | `Server=(localdb)\v11.0;Integrated Security=true;` | Recommended |
+| `db.system` | string | An identifier for the database management system (DBMS) product being used. See below for a list of well-known identifiers. | `other_sql` | Required |
 | `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` | See below |
 | `net.peer.name` | string | Remote hostname or similar, see note below. | `example.com` | See below |
@@ -127,8 +127,8 @@ When additional attributes are added that only apply to a specific DBMS, its ide
 <!-- semconv db(tag=connection-level-tech-specific,remove_constraints) -->
 | Attribute  | Type | Description  | Examples  | Requirement Level |
 |---|---|---|---|---|
-| `db.mssql.instance_name` | string | The Microsoft SQL Server [instance name](https://docs.microsoft.com/en-us/sql/connect/jdbc/building-the-connection-url?view=sql-server-ver15) connecting to. This name is used to determine the port of a named instance. [1] | `MSSQLSERVER` | Recommended |
 | `db.jdbc.driver_classname` | string | The fully-qualified class name of the JDBC driver used to connect. | `org.postgresql.Driver`; `com.microsoft.sqlserver.jdbc.SQLServerDriver` | Recommended |
+| `db.mssql.instance_name` | string | The Microsoft SQL Server [instance name](https://docs.microsoft.com/en-us/sql/connect/jdbc/building-the-connection-url?view=sql-server-ver15) connecting to. This name is used to determine the port of a named instance. [1] | `MSSQLSERVER` | Recommended |
 
 **[1]:** If setting a `db.mssql.instance_name`, `net.peer.port` is no longer required (but still recommended if non-standard).
 <!-- endsemconv -->
@@ -142,16 +142,16 @@ Usually only one `db.name` will be used per connection though.
 | Attribute  | Type | Description  | Examples  | Requirement Level |
 |---|---|---|---|---|
 | `db.name` | string | If no tech-specific attribute is defined, this attribute is used to report the name of the database being accessed. For commands that switch the database, this should be set to the target database (even if the command fails). [1] | `customers`; `main` | Conditionally Required: [2] |
-| `db.statement` | string | The database statement being executed. [3] | `SELECT * FROM wuser_table`; `SET mykey "WuValue"` | Conditionally Required: if applicable. |
-| `db.operation` | string | The name of the operation being executed, e.g. the [MongoDB command name](https://docs.mongodb.com/manual/reference/command/#database-operations) such as `findAndModify`. [4] | `findAndModify`; `HMSET` | Conditionally Required: if `db.statement` is not applicable. |
+| `db.operation` | string | The name of the operation being executed, e.g. the [MongoDB command name](https://docs.mongodb.com/manual/reference/command/#database-operations) such as `findAndModify`. [3] | `findAndModify`; `HMSET` | Conditionally Required: if `db.statement` is not applicable. |
+| `db.statement` | string | The database statement being executed. [4] | `SELECT * FROM wuser_table`; `SET mykey "WuValue"` | Conditionally Required: if applicable. |
 
 **[1]:** In some SQL databases, the database name to be used is called "schema name".
 
 **[2]:** if applicable and no more-specific attribute is defined.
 
-**[3]:** The value may be sanitized to exclude sensitive information.
+**[3]:** While it would semantically make sense to set this, e.g., to a SQL keyword like `SELECT` or `INSERT`, it is not recommended to attempt any client-side parsing of `db.statement` just to get this property (the back end can do that if required).
 
-**[4]:** While it would semantically make sense to set this, e.g., to a SQL keyword like `SELECT` or `INSERT`, it is not recommended to attempt any client-side parsing of `db.statement` just to get this property (the back end can do that if required).
+**[4]:** The value may be sanitized to exclude sensitive information.
 <!-- endsemconv -->
 
 For **Redis**, the value provided for `db.statement` SHOULD correspond to the syntax of the Redis CLI.

semantic-conventions/src/tests/data/markdown/extend_grandparent/database.yaml

@@ -0,0 +1,35 @@
+groups:
+  - id: database.common_attributes
+    prefix: db
+    type: attribute_group
+    brief: These attributes describe database calls.
+    attributes:
+      - id: name
+        brief: Database name.
+        type: string
+        examples: [ 'the_shop' ]
+
+  - id: database.foo.attributes
+    prefix: db.foo
+    type: attribute_group
+    brief: These attributes describe database Foo calls.
+    extends: database.common_attributes
+    attributes:
+      - id: bar
+        brief: Some property.
+        type: string
+        examples: [ 'baz' ]
+
+  - id: database.foo.span
+    prefix: db.foo
+    type: span
+    brief: These attributes describe database Foo spans.
+    extends: database.foo.attributes
+
+  - id: database.foo.duration.metric
+    type: metric
+    metric_name: db.foo.duration
+    instrument: histogram
+    unit: s
+    brief: Measures the duration of database Foo calls.
+    extends: database.foo.attributes

semantic-conventions/src/tests/data/markdown/extend_grandparent/expected.md

@@ -0,0 +1,23 @@
+## DB spans
+
+<!-- semconv database.foo.span(full) -->
+| Attribute  | Type | Description  | Examples  | Requirement Level |
+|---|---|---|---|---|
+| `db.foo.bar` | string | Some property. | `baz` | Recommended |
+| `db.name` | string | Database name. | `the_shop` | Recommended |
+<!-- endsemconv -->
+
+## DB metrics
+
+<!-- semconv database.foo.duration.metric(metric_table) -->
+| Name     | Instrument Type | Unit (UCUM) | Description    |
+| -------- | --------------- | ----------- | -------------- |
+| `db.foo.duration` | Histogram | `s` | Measures the duration of database Foo calls. |
+<!-- endsemconv -->
+
+<!-- semconv database.foo.duration.metric(full) -->
+| Attribute  | Type | Description  | Examples  | Requirement Level |
+|---|---|---|---|---|
+| `db.foo.bar` | string | Some property. | `baz` | Recommended |
+| `db.name` | string | Database name. | `the_shop` | Recommended |
+<!-- endsemconv -->

semantic-conventions/src/tests/data/markdown/extend_grandparent/input.md

@@ -0,0 +1,12 @@
+## DB spans
+
+<!-- semconv database.foo.span(full) -->
+<!-- endsemconv -->
+
+## DB metrics
+
+<!-- semconv database.foo.duration.metric(metric_table) -->
+<!-- endsemconv -->
+
+<!-- semconv database.foo.duration.metric(full) -->
+<!-- endsemconv -->

semantic-conventions/src/tests/data/markdown/include/expected.md

@@ -3,19 +3,19 @@
 <!-- semconv faas.http(full) -->
 | Attribute  | Type | Description  | Examples  | Requirement Level |
 |---|---|---|---|---|
-| `faas.trigger` | string | Type of the trigger on which the function is executed. | `datasource` | Required |
 | `faas.execution` | string | The execution id of the current function execution. | `af9d5aa4-a685-4c5f-a22b-444f80b3cc28` | Recommended |
-| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required |
-| `http.url` | string | Full HTTP request URL in the form `scheme://host[:port]/path?query[#fragment]`. Usually the fragment is not transmitted over HTTP, but if it is known, it should be included nevertheless. | `https://www.foo.bar/search?q=OpenTelemetry#SemConv` | See below |
-| `http.target` | string | The full request target as passed in a HTTP request line or equivalent. | `/path/12314/?q=ddds#123` | See below |
+| `faas.trigger` | string | Type of the trigger on which the function is executed. | `datasource` | Required |
 | `http.host` | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). When the header is empty or not present, this attribute should be the same. | `www.example.org` | See below |
+| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required |
+| `http.recommended_attribute` | string | brief | `foo` | Recommended: short note |
+| `http.recommended_attribute_long_note` | string | brief | `bar` | Recommended: [1] |
 | `http.scheme` | string | The URI scheme identifying the used protocol. | `http`; `https` | See below |
+| [`http.server_name`](input_http.md) | string | The primary server name of the matched virtual host. [2] | `example.com` | Conditionally Required: [3] |
 | `http.status_code` | int | [HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6). | `200` | Conditionally Required: if and only if one was received/sent |
 | `http.status_text` | string | [HTTP reason phrase](https://tools.ietf.org/html/rfc7230#section-3.1.2). | `OK` | Recommended |
+| `http.target` | string | The full request target as passed in a HTTP request line or equivalent. | `/path/12314/?q=ddds#123` | See below |
+| `http.url` | string | Full HTTP request URL in the form `scheme://host[:port]/path?query[#fragment]`. Usually the fragment is not transmitted over HTTP, but if it is known, it should be included nevertheless. | `https://www.foo.bar/search?q=OpenTelemetry#SemConv` | See below |
 | `http.user_agent` | string | Value of the [HTTP User-Agent](https://tools.ietf.org/html/rfc7231#section-5.5.3) header sent by the client. | `CERN-LineMode/2.15 libwww/2.17b3` | Recommended |
-| `http.recommended_attribute` | string | brief | `foo` | Recommended: short note |
-| `http.recommended_attribute_long_note` | string | brief | `bar` | Recommended: [1] |
-| [`http.server_name`](input_http.md) | string | The primary server name of the matched virtual host. [2] | `example.com` | Conditionally Required: [3] |
 
 **[1]:** some very long note that should be written under the semconv table