From c1ccabdd8d07acb7f322268d2338309e7a4e22ca Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 11 Jan 2022 17:12:46 +1100 Subject: [PATCH 01/54] functionality for metrics table generation from yaml --- semantic-conventions/README.md | 3 ++ .../semconv/model/semantic_convention.py | 43 ++++++++++++++++++- .../semconv/templating/markdown/__init__.py | 22 +++++++++- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/README.md b/semantic-conventions/README.md index 7ce2b398..cc3ddd8d 100644 --- a/semantic-conventions/README.md +++ b/semantic-conventions/README.md @@ -83,6 +83,9 @@ convention that have the tag `network`. `` will print the constraints and attributes of both `http` and `http.server` semantic conventions that have the tag `network`. +`` will print a table of metrics with all metrics prefixed with +`metric.http.server`. + ## Code Generator The image supports [Jinja](https://jinja.palletsprojects.com/en/2.11.x/) templates to generate code from the models. diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 60c78378..87e46d4a 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -53,6 +53,18 @@ def parse(span_kind_value): return kind_map.get(span_kind_value) +class InstrumentKind(Enum): + Empty = 1 + Counter = 2 + AsynchronousCounter = 3 + Histogram = 4 + AsynchronousGauge = 5 + UpDownCounter = 6 + AsynchronousUpDownCounter = 7 + + def __str__(self): + return self.name + def parse_semantic_convention_type(type_value): # Gracefully transition to the new types if type_value is None: @@ -238,7 +250,36 @@ def __init__(self, group): class MetricSemanticConvention(BaseSemanticConvention): GROUP_TYPE_NAME = "metric" - allowed_keys = () + allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ("metrics",) + + class Metric: + def __init__(self, metric): + self.id: str = metric.get("id") + self.instrument: InstrumentKind = InstrumentKind[metric.get("instrument")] + self.units: str = metric.get("units") + self.brief: str = metric.get("brief") + + if None in [metric.get("instrument"), self.id, self.units, self.brief]: + raise ValueError + + def __init__(self, group): + super().__init__(group) + self.metrics = () + if group.get("metrics"): + try: + self.metrics: Tuple[MetricSemanticConvention.Metric] = tuple( + map(lambda m: MetricSemanticConvention.Metric(m), group.get("metrics")) + ) + except ValueError: + raise ValidationError.from_yaml_pos( + self._position, "id, instrument, units, and brief must all be defined for concrete metrics" + ) + for metric in self.metrics: + if not metric.id.startswith(self.semconv_id): + raise ValidationError.from_yaml_pos( + self._position, + "id of metric `{}` must be prefixed with its parent's id `{}`".format(metric.id, self.semconv_id) + ) @dataclass diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 9f093183..e32411bb 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -30,6 +30,7 @@ ) from opentelemetry.semconv.model.semantic_convention import ( EventSemanticConvention, + MetricSemanticConvention, SemanticConventionSet, UnitSemanticConvention, ) @@ -66,7 +67,7 @@ class MarkdownRenderer: ) p_end = re.compile("") default_break_conditional_labels = 50 - valid_parameters = ["tag", "full", "remove_constraints"] + valid_parameters = ["tag", "full", "remove_constraints", "metric_table"] prelude = "\n" table_headers = "| Attribute | Type | Description | Examples | Required |\n|---|---|---|---|---|\n" @@ -174,6 +175,19 @@ def to_markdown_attr( ) ) + def to_markdown_metric_table(self, semconv: MetricSemanticConvention, output: io.StringIO): + """ + This method renders metrics as markdown table entry + """ + output.write( + "| Name | Instrument | Unit ([UCUM](README.md#instrument-units)) | Description |\n" + "| -------- | ---------------- | --------- | -------------- |\n" + ) + for metric in semconv.metrics: + output.write( + "| `{}` | {} | `{}` | {} |\n".format(metric.id, metric.instrument, metric.units, metric.brief) + ) + def to_markdown_anyof(self, anyof: AnyOf, output: io.StringIO): """ This method renders anyof constraints into markdown lists @@ -414,15 +428,21 @@ def _render_group(self, semconv, parameters, output): self.render_ctx.is_remove_constraint = "remove_constraints" in parameters self.render_ctx.group_key = parameters.get("tag") self.render_ctx.is_full = "full" in parameters + self.render_ctx.is_metric_table = "metric_table" in parameters if isinstance(semconv, EventSemanticConvention): output.write("The event name MUST be `{}`.\n\n".format(semconv.name)) + if isinstance(semconv, MetricSemanticConvention) and self.render_ctx.is_metric_table: + self.to_markdown_metric_table(semconv, output) + attr_to_print = [] attr: SemanticAttribute for attr in sorted( semconv.attributes, key=lambda a: "" if a.ref is None else a.ref ): + if self.render_ctx.is_metric_table: + break if self.render_ctx.group_key is not None: if attr.tag == self.render_ctx.group_key: attr_to_print.append(attr) From 40d73ef6556c151d665d75b21d4847d8092d957b Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 12 Jan 2022 17:50:02 +1100 Subject: [PATCH 02/54] tests for metric convention yaml->markdown --- .../data/markdown/metrics_tables/expected.md | 76 ++++++++ .../data/markdown/metrics_tables/general.yaml | 86 +++++++++ .../data/markdown/metrics_tables/http.yaml | 172 ++++++++++++++++++ .../markdown/metrics_tables/http_metrics.yaml | 65 +++++++ .../data/markdown/metrics_tables/input.md | 44 +++++ .../src/tests/data/yaml/http_metrics.yaml | 65 +++++++ .../tests/semconv/model/test_correct_parse.py | 64 +++++++ .../tests/semconv/templating/test_markdown.py | 3 + 8 files changed, 575 insertions(+) create mode 100644 semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md create mode 100644 semantic-conventions/src/tests/data/markdown/metrics_tables/general.yaml create mode 100644 semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml create mode 100644 semantic-conventions/src/tests/data/markdown/metrics_tables/http_metrics.yaml create mode 100644 semantic-conventions/src/tests/data/markdown/metrics_tables/input.md create mode 100644 semantic-conventions/src/tests/data/yaml/http_metrics.yaml diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md new file mode 100644 index 00000000..15788ded --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -0,0 +1,76 @@ + +## Common Attributes + +The following attributes SHOULD be included on all HTTP metrics for both server and client. + + +| Attribute | Type | Description | Examples | Required | +|---|---|---|---|---| +| `http.host` | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). An empty Host header should also be reported, see note. [1] | `www.example.org` | See attribute alternatives | +| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Yes | +| `http.scheme` | string | The URI scheme identifying the used protocol. | `http`; `https` | See attribute alternative | +| `http.status_code` | int | [HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6). | `200` | No | + +**[1]:** When the header is present but empty the attribute SHOULD be set to the empty string. Note that this is a valid situation that is expected in certain cases, according the aforementioned [section of RFC 7230](https://tools.ietf.org/html/rfc7230#section-5.4). When the header is not set the attribute MUST NOT be set. + + +## HTTP Client + +### HTTP Client Metrics + + +| Name | Instrument | Unit ([UCUM](README.md#instrument-units)) | Description | +| -------- | ---------------- | --------- | -------------- | +| `metric.http.client.duration` | Histogram | `ms` | Measures the duration of the outbound HTTP request. | + + +### HTTP Client Attributes + +The following attributes SHOULD be included on HTTP Client metrics, where applicable and available. + + +| Attribute | Type | Description | Examples | Required | +|---|---|---|---|---| +| `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 | +| `net.peer.port` | int | Remote port number. | `80`; `8080`; `443` | See below | + +**Additional attribute requirements:** At least one of the following sets of attributes is required: + +* `http.url` +* `http.scheme`, `http.host`, `http.target` +* `http.scheme`, `net.peer.name`, `net.peer.port`, `http.target` +* `http.scheme`, `net.peer.ip`, `net.peer.port`, `http.target` + + +## HTTP Server + +### HTTP Server Metrics + + +| Name | Instrument | Unit ([UCUM](README.md#instrument-units)) | Description | +| -------- | ---------------- | --------- | -------------- | +| `metric.http.server.duration` | Histogram | `ms` | Measures the duration of the inbound HTTP request. | +| `metric.http.server.active_requests` | AsynchronousUpDownCounter | `{requests}` | Measures the number of concurrent HTTP requests that are currently in-flight. | + + +### HTTP Server Attributes + +The following attributes SHOULD be included on HTTP Server metrics, where applicable and available. + + +| Attribute | Type | Description | Examples | Required | +|---|---|---|---|---| +| `http.server_name` | string | The primary server name of the matched virtual host. This should be obtained via configuration. If no such configuration can be obtained, this attribute MUST NOT be set ( `net.host.name` should be used instead). [1] | `example.com` | See below | +| `net.host.name` | string | Local hostname or similar, see note below. | `localhost` | See below | +| `net.host.port` | int | Like `net.peer.port` but for the host port. | `35555` | See below | + +**[1]:** `http.url` is usually not readily available on the server side but would have to be assembled in a cumbersome and sometimes lossy process from other information (see e.g. open-telemetry/opentelemetry-python/pull/148). It is thus preferred to supply the raw data that is available. + +**Additional attribute requirements:** At least one of the following sets of attributes is required: + +* `http.scheme`, `http.host`, `http.target` +* `http.scheme`, `http.server_name`, `net.host.port`, `http.target` +* `http.scheme`, `net.host.name`, `net.host.port`, `http.target` +* `http.url` + diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/general.yaml b/semantic-conventions/src/tests/data/markdown/metrics_tables/general.yaml new file mode 100644 index 00000000..5e91e7e6 --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/general.yaml @@ -0,0 +1,86 @@ +groups: + - id: network + prefix: net + type: span + brief: > + These attributes may be used for any network related operation. + attributes: + - id: transport + type: + allow_custom_values: false + members: + - id: ip.tcp + value: "IP.TCP" + - id: ip.udp + value: "IP.UDP" + - id: ip + value: "IP" + brief: 'Another IP-based protocol' + - id: unix + value: "Unix" + brief: 'Unix Domain socket. See below.' + - id: pipe + value: "pipe" + brief: 'Named or anonymous pipe. See note below.' + - id: inproc + value: "inproc" + brief: 'In-process communication.' + note: > + Signals that there is only in-process communication not using a "real" network protocol + in cases where network attributes would normally be expected. Usually all other network + attributes can be left out in that case. + - id: other + value: "other" + brief: 'Something else (non IP-based).' + brief: > + Transport protocol used. See note below. + examples: 'ip.tcp' + - id: peer.ip + type: string + brief: > + Remote address of the peer (dotted decimal for IPv4 or + [RFC5952](https://tools.ietf.org/html/rfc5952) for IPv6) + examples: '127.0.0.1' + - id: peer.port + type: int + brief: 'Remote port number.' + examples: [80, 8080, 443] + - id: peer.name + type: string + brief: 'Remote hostname or similar, see note below.' + examples: 'example.com' + - id: host.ip + type: string + brief: 'Like `net.peer.ip` but for the host IP. Useful in case of a multi-IP host.' + examples: '192.168.0.1' + - id: host.port + type: int + brief: 'Like `net.peer.port` but for the host port.' + examples: 35555 + - id: host.name + type: string + brief: 'Local hostname or similar, see note below.' + examples: 'localhost' + - id: identity + type: span + prefix: enduser + brief: > + These attributes may be used for any operation with an authenticated and/or authorized enduser. + attributes: + - id: id + type: string + brief: > + Username or client_id extracted from the access token or Authorization header + in the inbound request from outside the system. + examples: 'username' + - id: role + type: string + brief: 'Actual/assumed role the client is making the request under extracted from token or application security context.' + examples: 'admin' + - id: scope + type: string + brief: > + Scopes or granted authorities the client currently possesses extracted from token + or application security context. The value would come from the scope associated + with an OAuth 2.0 Access Token or an attribute value in a SAML 2.0 Assertion. + examples: 'read:message, write:files' diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml b/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml new file mode 100644 index 00000000..20adebca --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml @@ -0,0 +1,172 @@ +groups: + - id: http + prefix: http + brief: 'This document defines semantic conventions for HTTP client and server Spans.' + note: > + These conventions can be used for http and https schemes + and various HTTP versions like 1.1, 2 and SPDY. + attributes: + - id: method + type: string + required: always + brief: 'HTTP request method.' + sampling_relevant: true + examples: ["GET", "POST", "HEAD"] + - id: url + type: string + brief: > + 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. + note: > + `http.url` MUST NOT contain credentials passed via URL in form of `https://username:password@www.example.com/`. + In such case the attribute's value should be `https://www.example.com/`. + sampling_relevant: true + examples: ['https://www.foo.bar/search?q=OpenTelemetry#SemConv'] + - id: target + type: string + brief: 'The full request target as passed in a HTTP request line or equivalent.' + sampling_relevant: true + examples: ['/path/12314/?q=ddds#123'] + - id: host + type: string + brief: > + The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). + An empty Host header should also be reported, see note. + note: > + When the header is present but empty the attribute SHOULD be set to + the empty string. Note that this is a valid situation that is expected + in certain cases, according the aforementioned + [section of RFC 7230](https://tools.ietf.org/html/rfc7230#section-5.4). + When the header is not set the attribute MUST NOT be set. + sampling_relevant: true + examples: ['www.example.org'] + - id: scheme + type: string + brief: 'The URI scheme identifying the used protocol.' + sampling_relevant: true + examples: ["http", "https"] + - id: status_code + type: int + required: + conditional: If and only if one was received/sent. + brief: '[HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6).' + examples: [200] + - id: flavor + type: + # Default value: `true`. If false, it helps the code gen tool to + # encode checks that only accept the listed values. + allow_custom_values: true + members: + - id: http_1_0 + value: '1.0' + brief: 'HTTP 1.0' + - id: http_1_1 + value: '1.1' + brief: 'HTTP 1.1' + - id: http_2_0 + value: '2.0' + brief: 'HTTP 2' + - id: spdy + value: 'SPDY' + brief: 'SPDY protocol.' + - id: quic + value: 'QUIC' + brief: 'QUIC protocol.' + brief: 'Kind of HTTP protocol used.' + note: > + 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. + - id: user_agent + type: string + brief: 'Value of the [HTTP User-Agent](https://tools.ietf.org/html/rfc7231#section-5.5.3) header sent by the client.' + examples: ['CERN-LineMode/2.15 libwww/2.17b3'] + - id: request_content_length + type: int + brief: > + The size of the request payload body in bytes. This is the number of bytes transferred excluding headers and + is often, but not always, present as the [Content-Length](https://tools.ietf.org/html/rfc7230#section-3.3.2) + header. For requests using transport encoding, this should be the compressed size. + examples: 3495 + - id: request_content_length_uncompressed + type: int + brief: > + The size of the uncompressed request payload body after transport decoding. Not set if transport encoding not used. + examples: 5493 + - id: response_content_length + type: int + brief: > + The size of the response payload body in bytes. This is the number of bytes transferred excluding headers and + is often, but not always, present as the [Content-Length](https://tools.ietf.org/html/rfc7230#section-3.3.2) + header. For requests using transport encoding, this should be the compressed size. + examples: 3495 + - id: response_content_length_uncompressed + type: int + brief: > + The size of the uncompressed response payload body after transport decoding. Not set if transport encoding not used. + examples: 5493 + - ref: net.peer.name + sampling_relevant: true + - ref: net.peer.ip + sampling_relevant: true + - ref: net.peer.port + sampling_relevant: true + constraints: + - include: network + + - id: http.client + prefix: http + extends: http + span_kind: client + brief: 'Semantic Convention for HTTP Client' + constraints: + - any_of: + - [http.url] + - [http.scheme, http.host, http.target] + - [http.scheme, net.peer.name, net.peer.port, http.target] + - [http.scheme, net.peer.ip, net.peer.port, http.target] + + - id: http.server + prefix: http + extends: http + span_kind: server + brief: 'Semantic Convention for HTTP Server' + attributes: + - id: server_name + type: string + brief: > + The primary server name of the matched virtual host. This should be obtained via configuration. + If no such configuration can be obtained, this attribute MUST NOT be set ( `net.host.name` should be used instead). + note: > + `http.url` is usually not readily available on the server side but would have to be assembled in a cumbersome and + sometimes lossy process from other information (see e.g. open-telemetry/opentelemetry-python/pull/148). + It is thus preferred to supply the raw data that is available. + examples: ['example.com'] + - id: route + type: string + brief: > + The matched route (path template). + examples: '/users/:userID?' + - id: client_ip + type: string + brief: > + The IP address of the original client behind all proxies, if + known (e.g. from [X-Forwarded-For](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For)). + note: | + This is not necessarily the same as `net.peer.ip`, which would + identify the network-level peer, which may be a proxy. + + This attribute should be set when a source of information different + from the one used for `net.peer.ip`, is available even if that other + source just confirms the same value as `net.peer.ip`. + Rationale: For `net.peer.ip`, one typically does not know if it + comes from a proxy, reverse proxy, or the actual client. Setting + `http.client_ip` when it's the same as `net.peer.ip` means that + one is at least somewhat confident that the address is not that of + the closest proxy. + examples: '83.164.160.102' + constraints: + - any_of: + - [http.scheme, http.host, http.target] + - [http.scheme, http.server_name, net.host.port, http.target] + - [http.scheme, net.host.name, net.host.port, http.target] + - [http.url] diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/http_metrics.yaml b/semantic-conventions/src/tests/data/markdown/metrics_tables/http_metrics.yaml new file mode 100644 index 00000000..65045c87 --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/http_metrics.yaml @@ -0,0 +1,65 @@ +groups: + - id: metric.http + prefix: http + type: metric + brief: "This document defines semantic convention for HTTP client and server metrics." + note: > + These conventions can be used for http and https schemes + and various HTTP versions like 1.1, 2 and SPDY. + attributes: + - ref: http.method + required: always + - ref: http.host + required: + conditional: "See attribute alternatives" + - ref: http.scheme + required: + conditional: "See attribute alternative" + - ref: http.status_code + + - id: metric.http.client + prefix: http + type: metric + extends: metric.http + brief: 'Semantic Convention for HTTP Client' + constraints: + - any_of: + - [http.url] + - [http.scheme, http.host, http.target] + - [http.scheme, net.peer.name, net.peer.port, http.target] + - [http.scheme, net.peer.ip, net.peer.port, http.target] + attributes: + - ref: net.peer.name + - ref: net.peer.port + - ref: net.peer.ip + metrics: + - id: metric.http.client.duration + brief: "Measures the duration of the outbound HTTP request." + extends: metric.http.client + instrument: Histogram + units: ms + + - id: metric.http.server + prefix: http + type: metric + extends: metric.http + brief: 'Semantic Convention for HTTP Server' + constraints: + - any_of: + - [http.scheme, http.host, http.target] + - [http.scheme, http.server_name, net.host.port, http.target] + - [http.scheme, net.host.name, net.host.port, http.target] + - [http.url] + attributes: + - ref: http.server_name + - ref: net.host.name + - ref: net.host.port + metrics: + - id: metric.http.server.duration + brief: "Measures the duration of the inbound HTTP request." + instrument: Histogram + units: ms + - id: metric.http.server.active_requests + brief: "Measures the number of concurrent HTTP requests that are currently in-flight." + instrument: AsynchronousUpDownCounter + units: "{requests}" diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md new file mode 100644 index 00000000..b9966675 --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md @@ -0,0 +1,44 @@ + +## Common Attributes + +The following attributes SHOULD be included on all HTTP metrics for both server and client. + + +| Attribute | Type | Description | Examples | Required | +|---|---|---|---|---| +| [`http.host`](../../trace/semantic_conventions/http.md) | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). An empty Host header should also be reported, see note. [1] | `www.example.org` | See attribute alternatives | +| [`http.method`](../../trace/semantic_conventions/http.md) | string | HTTP request method. | `GET`; `POST`; `HEAD` | Yes | +| [`http.scheme`](../../trace/semantic_conventions/http.md) | string | The URI scheme identifying the used protocol. | `http`; `https` | See attribute alternative | +| [`http.status_code`](../../trace/semantic_conventions/http.md) | int | [HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6). | `200` | No | + +**[1]:** When the header is present but empty the attribute SHOULD be set to the empty string. Note that this is a valid situation that is expected in certain cases, according the aforementioned [section of RFC 7230](https://tools.ietf.org/html/rfc7230#section-5.4). When the header is not set the attribute MUST NOT be set. + + +## HTTP Client + +### HTTP Client Metrics + + + + +### HTTP Client Attributes + +The following attributes SHOULD be included on HTTP Client metrics, where applicable and available. + + + + +## HTTP Server + +### HTTP Server Metrics + + + + +### HTTP Server Attributes + +The following attributes SHOULD be included on HTTP Server metrics, where applicable and available. + + + + diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml new file mode 100644 index 00000000..65045c87 --- /dev/null +++ b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml @@ -0,0 +1,65 @@ +groups: + - id: metric.http + prefix: http + type: metric + brief: "This document defines semantic convention for HTTP client and server metrics." + note: > + These conventions can be used for http and https schemes + and various HTTP versions like 1.1, 2 and SPDY. + attributes: + - ref: http.method + required: always + - ref: http.host + required: + conditional: "See attribute alternatives" + - ref: http.scheme + required: + conditional: "See attribute alternative" + - ref: http.status_code + + - id: metric.http.client + prefix: http + type: metric + extends: metric.http + brief: 'Semantic Convention for HTTP Client' + constraints: + - any_of: + - [http.url] + - [http.scheme, http.host, http.target] + - [http.scheme, net.peer.name, net.peer.port, http.target] + - [http.scheme, net.peer.ip, net.peer.port, http.target] + attributes: + - ref: net.peer.name + - ref: net.peer.port + - ref: net.peer.ip + metrics: + - id: metric.http.client.duration + brief: "Measures the duration of the outbound HTTP request." + extends: metric.http.client + instrument: Histogram + units: ms + + - id: metric.http.server + prefix: http + type: metric + extends: metric.http + brief: 'Semantic Convention for HTTP Server' + constraints: + - any_of: + - [http.scheme, http.host, http.target] + - [http.scheme, http.server_name, net.host.port, http.target] + - [http.scheme, net.host.name, net.host.port, http.target] + - [http.url] + attributes: + - ref: http.server_name + - ref: net.host.name + - ref: net.host.port + metrics: + - id: metric.http.server.duration + brief: "Measures the duration of the inbound HTTP request." + instrument: Histogram + units: ms + - id: metric.http.server.active_requests + brief: "Measures the number of concurrent HTTP requests that are currently in-flight." + instrument: AsynchronousUpDownCounter + units: "{requests}" diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index bad3ed68..6301c879 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -14,6 +14,7 @@ import os import unittest +from typing import List, Tuple, cast from opentelemetry.semconv.model.constraints import AnyOf, Include from opentelemetry.semconv.model.semantic_attribute import StabilityLevel @@ -21,6 +22,7 @@ EventSemanticConvention, SemanticConventionSet, SpanSemanticConvention, + MetricSemanticConvention, ) @@ -193,6 +195,62 @@ def test_http(self): } self.semantic_convention_check(list(semconv.models.values())[2], expected) + def test_metrics_http(self): + semconv = SemanticConventionSet(debug=False) + semconv.parse(self.load_file("yaml/http_metrics.yaml")) + self.assertEqual(len(semconv.models), 3) + semconv.parse(self.load_file("yaml/general.yaml")) + semconv.parse(self.load_file("yaml/http.yaml")) + + metric_semconvs = cast(List[MetricSemanticConvention], list(semconv.models.values())[:3]) + + expected = { + "id": "metric.http", + "prefix": "http", + "extends": "", + "n_constraints": 0, + "attributes": ["http.method", "http.host", "http.scheme", "http.status_code"] + } + self.semantic_convention_check(metric_semconvs[0], expected) + + expected = { + "id": "metric.http.client", + "prefix": "http", + "extends": "metric.http", + "n_constraints": 1, + "attributes": ["net.peer.name", "net.peer.port", "net.peer.ip"], + "metrics": [{ + "id": "metric.http.client.duration", + "instrument": "Histogram", + "units": "ms" + }] + } + self.semantic_convention_check(metric_semconvs[1], expected) + self.metric_check(metric_semconvs[1].metrics, expected.get("metrics")) + + expected = { + "id": "metric.http.server", + "prefix": "http", + "extends": "metric.http", + "n_constraints": 1, + "attributes": ["http.server_name", "net.host.name", "net.host.port"], + "metrics": [ + { + "id": "metric.http.server.duration", + "instrument": "Histogram", + "units": "ms" + }, + { + "id": "metric.http.server.active_requests", + "instrument": "AsynchronousUpDownCounter", + "units": "{requests}" + } + ] + } + self.semantic_convention_check(metric_semconvs[2], expected) + self.metric_check(metric_semconvs[2].metrics, expected.get("metrics")) + + def test_resource(self): semconv = SemanticConventionSet(debug=False) semconv.parse(self.load_file("yaml/cloud.yaml")) @@ -669,6 +727,12 @@ def semantic_convention_check(self, s, expected): self.assertEqual(expected["n_constraints"], len(s.constraints)) self.assertEqual(expected["attributes"], [a.fqn for a in s.attributes]) + def metric_check(self, metrics: Tuple[MetricSemanticConvention.Metric], expected): + for i, metric in enumerate(metrics): + self.assertEqual(metric.id, expected[i]["id"]) + self.assertEqual(str(metric.instrument), expected[i]["instrument"]) + self.assertEqual(metric.units, expected[i]["units"]) + _TEST_DIR = os.path.dirname(__file__) def load_file(self, filename): diff --git a/semantic-conventions/src/tests/semconv/templating/test_markdown.py b/semantic-conventions/src/tests/semconv/templating/test_markdown.py index 996fd043..53701861 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_markdown.py +++ b/semantic-conventions/src/tests/semconv/templating/test_markdown.py @@ -123,6 +123,9 @@ def test_event_noprefix(self): def test_event_renamed(self): self.check("markdown/event_renamed/") + def test_metric_tables(self): + self.check("markdown/metrics_tables") + def testSamplingRelevant(self): self.check("markdown/sampling_relevant/") From beead271a5aed3bdcccfde5e4b9c31bdb1d6c13a Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 12 Jan 2022 18:11:41 +1100 Subject: [PATCH 03/54] appease linter --- .../semconv/model/semantic_convention.py | 17 +++++++---- .../semconv/templating/markdown/__init__.py | 15 ++++++++-- .../tests/semconv/model/test_correct_parse.py | 30 ++++++++++++------- 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 87e46d4a..aa01a12a 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -65,6 +65,7 @@ class InstrumentKind(Enum): def __str__(self): return self.name + def parse_semantic_convention_type(type_value): # Gracefully transition to the new types if type_value is None: @@ -268,17 +269,23 @@ def __init__(self, group): if group.get("metrics"): try: self.metrics: Tuple[MetricSemanticConvention.Metric] = tuple( - map(lambda m: MetricSemanticConvention.Metric(m), group.get("metrics")) + map( + MetricSemanticConvention.Metric, + group.get("metrics"), + ) ) - except ValueError: + except ValueError as e: raise ValidationError.from_yaml_pos( - self._position, "id, instrument, units, and brief must all be defined for concrete metrics" - ) + self._position, + "id, instrument, units, and brief must all be defined for concrete metrics", + ) from e for metric in self.metrics: if not metric.id.startswith(self.semconv_id): raise ValidationError.from_yaml_pos( self._position, - "id of metric `{}` must be prefixed with its parent's id `{}`".format(metric.id, self.semconv_id) + "id of metric `{}` must be prefixed with its parent's id `{}`".format( + metric.id, self.semconv_id + ), ) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index e32411bb..5a5db8c0 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -42,6 +42,7 @@ class RenderContext: def __init__(self): self.is_full = False self.is_remove_constraint = False + self.is_metric_table = False self.group_key = "" self.enums = [] self.notes = [] @@ -175,7 +176,10 @@ def to_markdown_attr( ) ) - def to_markdown_metric_table(self, semconv: MetricSemanticConvention, output: io.StringIO): + @staticmethod + def to_markdown_metric_table( + semconv: MetricSemanticConvention, output: io.StringIO + ): """ This method renders metrics as markdown table entry """ @@ -185,7 +189,9 @@ def to_markdown_metric_table(self, semconv: MetricSemanticConvention, output: io ) for metric in semconv.metrics: output.write( - "| `{}` | {} | `{}` | {} |\n".format(metric.id, metric.instrument, metric.units, metric.brief) + "| `{}` | {} | `{}` | {} |\n".format( + metric.id, metric.instrument, metric.units, metric.brief + ) ) def to_markdown_anyof(self, anyof: AnyOf, output: io.StringIO): @@ -433,7 +439,10 @@ def _render_group(self, semconv, parameters, output): if isinstance(semconv, EventSemanticConvention): output.write("The event name MUST be `{}`.\n\n".format(semconv.name)) - if isinstance(semconv, MetricSemanticConvention) and self.render_ctx.is_metric_table: + if ( + isinstance(semconv, MetricSemanticConvention) + and self.render_ctx.is_metric_table + ): self.to_markdown_metric_table(semconv, output) attr_to_print = [] diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index 6301c879..08c81c9f 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -20,9 +20,9 @@ from opentelemetry.semconv.model.semantic_attribute import StabilityLevel from opentelemetry.semconv.model.semantic_convention import ( EventSemanticConvention, + MetricSemanticConvention, SemanticConventionSet, SpanSemanticConvention, - MetricSemanticConvention, ) @@ -202,14 +202,21 @@ def test_metrics_http(self): semconv.parse(self.load_file("yaml/general.yaml")) semconv.parse(self.load_file("yaml/http.yaml")) - metric_semconvs = cast(List[MetricSemanticConvention], list(semconv.models.values())[:3]) + metric_semconvs = cast( + List[MetricSemanticConvention], list(semconv.models.values())[:3] + ) expected = { "id": "metric.http", "prefix": "http", "extends": "", "n_constraints": 0, - "attributes": ["http.method", "http.host", "http.scheme", "http.status_code"] + "attributes": [ + "http.method", + "http.host", + "http.scheme", + "http.status_code", + ], } self.semantic_convention_check(metric_semconvs[0], expected) @@ -219,11 +226,13 @@ def test_metrics_http(self): "extends": "metric.http", "n_constraints": 1, "attributes": ["net.peer.name", "net.peer.port", "net.peer.ip"], - "metrics": [{ + "metrics": [ + { "id": "metric.http.client.duration", "instrument": "Histogram", - "units": "ms" - }] + "units": "ms", + } + ], } self.semantic_convention_check(metric_semconvs[1], expected) self.metric_check(metric_semconvs[1].metrics, expected.get("metrics")) @@ -238,19 +247,18 @@ def test_metrics_http(self): { "id": "metric.http.server.duration", "instrument": "Histogram", - "units": "ms" + "units": "ms", }, { "id": "metric.http.server.active_requests", "instrument": "AsynchronousUpDownCounter", - "units": "{requests}" - } - ] + "units": "{requests}", + }, + ], } self.semantic_convention_check(metric_semconvs[2], expected) self.metric_check(metric_semconvs[2].metrics, expected.get("metrics")) - def test_resource(self): semconv = SemanticConventionSet(debug=False) semconv.parse(self.load_file("yaml/cloud.yaml")) From 6e4ff1176642a000fcd7bc17ad93f01f514c8417 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 25 Jan 2022 13:34:29 +1100 Subject: [PATCH 04/54] use zip instead of enumerate --- .../src/tests/semconv/model/test_correct_parse.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index 08c81c9f..b8f6fc9e 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -736,10 +736,10 @@ def semantic_convention_check(self, s, expected): self.assertEqual(expected["attributes"], [a.fqn for a in s.attributes]) def metric_check(self, metrics: Tuple[MetricSemanticConvention.Metric], expected): - for i, metric in enumerate(metrics): - self.assertEqual(metric.id, expected[i]["id"]) - self.assertEqual(str(metric.instrument), expected[i]["instrument"]) - self.assertEqual(metric.units, expected[i]["units"]) + for m, ex in zip(metrics, expected): + self.assertEqual(m.id, ex["id"]) + self.assertEqual(str(m.instrument), ex["instrument"]) + self.assertEqual(m.units, ex["units"]) _TEST_DIR = os.path.dirname(__file__) From cd9d815edd1a9d454992c607d401e50b2bcbdfa6 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 25 Jan 2022 14:36:04 +1100 Subject: [PATCH 05/54] raise error if metric_table is specified on non-metric semconv --- .../semconv/templating/markdown/__init__.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 5a5db8c0..84faf9b8 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -439,6 +439,16 @@ def _render_group(self, semconv, parameters, output): if isinstance(semconv, EventSemanticConvention): output.write("The event name MUST be `{}`.\n\n".format(semconv.name)) + if self.render_ctx.is_metric_table: + if isinstance(semconv, MetricSemanticConvention): + self.to_markdown_metric_table(semconv, output) + else: + raise ValueError( + "semconv `{}` was specified with `metric_table`, but it is not a metric convention".format( + semconv.semconv_id + ) + ) + if ( isinstance(semconv, MetricSemanticConvention) and self.render_ctx.is_metric_table From f4aef51b6cccfbab22037f340887b1f2e96f5b85 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 25 Jan 2022 15:00:36 +1100 Subject: [PATCH 06/54] update syntax.md with initial metrics syntax --- semantic-conventions/syntax.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index e447a40c..ae1d08f6 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -40,7 +40,7 @@ All attributes are lower case. groups ::= semconv | semconv groups -semconv ::= id [convtype] brief [note] [prefix] [extends] [stability] [deprecated] attributes [constraints] [specificfields] +semconv ::= id [convtype] brief [note] [prefix] [extends] [stability] [deprecated] attributes [constraints] [specificfields] [metrics] id ::= string @@ -119,6 +119,18 @@ events ::= id {id} # MUST point to an existing event group name ::= string +instrument ::= "Counter" + | "AsynchronousCounter" + | "Histogram" + | "AsynchronousGauge" + | "UpDownCounter" + | "AsynchronousUpDownCounter" + +units ::= string + +metric ::= id instrument units brief + +metrics ::= {metrics} ``` ## Semantics From 2421f0b8a86f3bf8310f4e6220439c84fd031520 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 25 Jan 2022 16:32:46 +1100 Subject: [PATCH 07/54] remove duplicated code and files --- .../semconv/templating/markdown/__init__.py | 6 -- .../data/markdown/metrics_tables/general.yaml | 86 ------------------- .../markdown/metrics_tables/http_metrics.yaml | 65 -------------- .../data/markdown/metrics_tables/input.md | 8 -- 4 files changed, 165 deletions(-) delete mode 100644 semantic-conventions/src/tests/data/markdown/metrics_tables/general.yaml delete mode 100644 semantic-conventions/src/tests/data/markdown/metrics_tables/http_metrics.yaml diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 84faf9b8..3219bdab 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -449,12 +449,6 @@ def _render_group(self, semconv, parameters, output): ) ) - if ( - isinstance(semconv, MetricSemanticConvention) - and self.render_ctx.is_metric_table - ): - self.to_markdown_metric_table(semconv, output) - attr_to_print = [] attr: SemanticAttribute for attr in sorted( diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/general.yaml b/semantic-conventions/src/tests/data/markdown/metrics_tables/general.yaml deleted file mode 100644 index 5e91e7e6..00000000 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/general.yaml +++ /dev/null @@ -1,86 +0,0 @@ -groups: - - id: network - prefix: net - type: span - brief: > - These attributes may be used for any network related operation. - attributes: - - id: transport - type: - allow_custom_values: false - members: - - id: ip.tcp - value: "IP.TCP" - - id: ip.udp - value: "IP.UDP" - - id: ip - value: "IP" - brief: 'Another IP-based protocol' - - id: unix - value: "Unix" - brief: 'Unix Domain socket. See below.' - - id: pipe - value: "pipe" - brief: 'Named or anonymous pipe. See note below.' - - id: inproc - value: "inproc" - brief: 'In-process communication.' - note: > - Signals that there is only in-process communication not using a "real" network protocol - in cases where network attributes would normally be expected. Usually all other network - attributes can be left out in that case. - - id: other - value: "other" - brief: 'Something else (non IP-based).' - brief: > - Transport protocol used. See note below. - examples: 'ip.tcp' - - id: peer.ip - type: string - brief: > - Remote address of the peer (dotted decimal for IPv4 or - [RFC5952](https://tools.ietf.org/html/rfc5952) for IPv6) - examples: '127.0.0.1' - - id: peer.port - type: int - brief: 'Remote port number.' - examples: [80, 8080, 443] - - id: peer.name - type: string - brief: 'Remote hostname or similar, see note below.' - examples: 'example.com' - - id: host.ip - type: string - brief: 'Like `net.peer.ip` but for the host IP. Useful in case of a multi-IP host.' - examples: '192.168.0.1' - - id: host.port - type: int - brief: 'Like `net.peer.port` but for the host port.' - examples: 35555 - - id: host.name - type: string - brief: 'Local hostname or similar, see note below.' - examples: 'localhost' - - id: identity - type: span - prefix: enduser - brief: > - These attributes may be used for any operation with an authenticated and/or authorized enduser. - attributes: - - id: id - type: string - brief: > - Username or client_id extracted from the access token or Authorization header - in the inbound request from outside the system. - examples: 'username' - - id: role - type: string - brief: 'Actual/assumed role the client is making the request under extracted from token or application security context.' - examples: 'admin' - - id: scope - type: string - brief: > - Scopes or granted authorities the client currently possesses extracted from token - or application security context. The value would come from the scope associated - with an OAuth 2.0 Access Token or an attribute value in a SAML 2.0 Assertion. - examples: 'read:message, write:files' diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/http_metrics.yaml b/semantic-conventions/src/tests/data/markdown/metrics_tables/http_metrics.yaml deleted file mode 100644 index 65045c87..00000000 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/http_metrics.yaml +++ /dev/null @@ -1,65 +0,0 @@ -groups: - - id: metric.http - prefix: http - type: metric - brief: "This document defines semantic convention for HTTP client and server metrics." - note: > - These conventions can be used for http and https schemes - and various HTTP versions like 1.1, 2 and SPDY. - attributes: - - ref: http.method - required: always - - ref: http.host - required: - conditional: "See attribute alternatives" - - ref: http.scheme - required: - conditional: "See attribute alternative" - - ref: http.status_code - - - id: metric.http.client - prefix: http - type: metric - extends: metric.http - brief: 'Semantic Convention for HTTP Client' - constraints: - - any_of: - - [http.url] - - [http.scheme, http.host, http.target] - - [http.scheme, net.peer.name, net.peer.port, http.target] - - [http.scheme, net.peer.ip, net.peer.port, http.target] - attributes: - - ref: net.peer.name - - ref: net.peer.port - - ref: net.peer.ip - metrics: - - id: metric.http.client.duration - brief: "Measures the duration of the outbound HTTP request." - extends: metric.http.client - instrument: Histogram - units: ms - - - id: metric.http.server - prefix: http - type: metric - extends: metric.http - brief: 'Semantic Convention for HTTP Server' - constraints: - - any_of: - - [http.scheme, http.host, http.target] - - [http.scheme, http.server_name, net.host.port, http.target] - - [http.scheme, net.host.name, net.host.port, http.target] - - [http.url] - attributes: - - ref: http.server_name - - ref: net.host.name - - ref: net.host.port - metrics: - - id: metric.http.server.duration - brief: "Measures the duration of the inbound HTTP request." - instrument: Histogram - units: ms - - id: metric.http.server.active_requests - brief: "Measures the number of concurrent HTTP requests that are currently in-flight." - instrument: AsynchronousUpDownCounter - units: "{requests}" diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md index b9966675..e03f49e0 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md @@ -4,14 +4,6 @@ The following attributes SHOULD be included on all HTTP metrics for both server and client. -| Attribute | Type | Description | Examples | Required | -|---|---|---|---|---| -| [`http.host`](../../trace/semantic_conventions/http.md) | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). An empty Host header should also be reported, see note. [1] | `www.example.org` | See attribute alternatives | -| [`http.method`](../../trace/semantic_conventions/http.md) | string | HTTP request method. | `GET`; `POST`; `HEAD` | Yes | -| [`http.scheme`](../../trace/semantic_conventions/http.md) | string | The URI scheme identifying the used protocol. | `http`; `https` | See attribute alternative | -| [`http.status_code`](../../trace/semantic_conventions/http.md) | int | [HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6). | `200` | No | - -**[1]:** When the header is present but empty the attribute SHOULD be set to the empty string. Note that this is a valid situation that is expected in certain cases, according the aforementioned [section of RFC 7230](https://tools.ietf.org/html/rfc7230#section-5.4). When the header is not set the attribute MUST NOT be set. ## HTTP Client From 5ac58953d330985ea0296021b06f836c1bd31486 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 25 Jan 2022 17:06:02 +1100 Subject: [PATCH 08/54] add extra_yaml_files param and use it in metric_table test --- .../src/tests/semconv/templating/test_markdown.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/semantic-conventions/src/tests/semconv/templating/test_markdown.py b/semantic-conventions/src/tests/semconv/templating/test_markdown.py index 53701861..c8badac1 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_markdown.py +++ b/semantic-conventions/src/tests/semconv/templating/test_markdown.py @@ -124,7 +124,10 @@ def test_event_renamed(self): self.check("markdown/event_renamed/") def test_metric_tables(self): - self.check("markdown/metrics_tables") + self.check( + "markdown/metrics_tables", + extra_yaml_files=["yaml/general.yaml", "yaml/http_metrics.yaml"], + ) def testSamplingRelevant(self): self.check("markdown/sampling_relevant/") @@ -136,6 +139,7 @@ def check( *, expected_name="expected.md", extra_yaml_dirs: Sequence[str] = (), + extra_yaml_files: Sequence[str] = (), assert_raises=None ) -> Optional[BaseException]: dirpath = Path(self.get_file_path(input_dir)) @@ -151,6 +155,9 @@ def check( for fname in Path(self.get_file_path(extra_dir)).glob("*.yaml"): print("Parsing", fname) semconv.parse(fname) + for fname in map(self.get_file_path, extra_yaml_files): + print("Parsing ", fname) + semconv.parse(fname) semconv.finish() From bec1a1bd7534e4d9cf59923268a37794dc3ae75f Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 27 Jan 2022 13:27:01 +1100 Subject: [PATCH 09/54] use 'FQN = prefix + id' for metric FQNs --- .../semconv/model/semantic_convention.py | 13 +++---------- .../semconv/templating/markdown/__init__.py | 2 +- .../src/tests/data/yaml/http_metrics.yaml | 15 +++++++-------- .../src/tests/semconv/model/test_correct_parse.py | 12 ++++++------ 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index aa01a12a..21cf9949 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -254,8 +254,9 @@ class MetricSemanticConvention(BaseSemanticConvention): allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ("metrics",) class Metric: - def __init__(self, metric): + def __init__(self, metric, parent_prefix): self.id: str = metric.get("id") + self.fqn = "{}.{}".format(parent_prefix, self.id) self.instrument: InstrumentKind = InstrumentKind[metric.get("instrument")] self.units: str = metric.get("units") self.brief: str = metric.get("brief") @@ -270,7 +271,7 @@ def __init__(self, group): try: self.metrics: Tuple[MetricSemanticConvention.Metric] = tuple( map( - MetricSemanticConvention.Metric, + lambda m: MetricSemanticConvention.Metric(m, self.prefix), group.get("metrics"), ) ) @@ -279,14 +280,6 @@ def __init__(self, group): self._position, "id, instrument, units, and brief must all be defined for concrete metrics", ) from e - for metric in self.metrics: - if not metric.id.startswith(self.semconv_id): - raise ValidationError.from_yaml_pos( - self._position, - "id of metric `{}` must be prefixed with its parent's id `{}`".format( - metric.id, self.semconv_id - ), - ) @dataclass diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 3219bdab..1974f584 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -190,7 +190,7 @@ def to_markdown_metric_table( for metric in semconv.metrics: output.write( "| `{}` | {} | `{}` | {} |\n".format( - metric.id, metric.instrument, metric.units, metric.brief + metric.fqn, metric.instrument, metric.units, metric.brief ) ) diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml index 65045c87..deb76dcc 100644 --- a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml @@ -1,6 +1,6 @@ groups: - id: metric.http - prefix: http + prefix: metric.http type: metric brief: "This document defines semantic convention for HTTP client and server metrics." note: > @@ -18,7 +18,7 @@ groups: - ref: http.status_code - id: metric.http.client - prefix: http + prefix: metric.http type: metric extends: metric.http brief: 'Semantic Convention for HTTP Client' @@ -33,17 +33,16 @@ groups: - ref: net.peer.port - ref: net.peer.ip metrics: - - id: metric.http.client.duration + - id: client.duration brief: "Measures the duration of the outbound HTTP request." - extends: metric.http.client instrument: Histogram units: ms - id: metric.http.server - prefix: http + prefix: metric.http type: metric extends: metric.http - brief: 'Semantic Convention for HTTP Server' + brief: "Semantic Convention for HTTP Server" constraints: - any_of: - [http.scheme, http.host, http.target] @@ -55,11 +54,11 @@ groups: - ref: net.host.name - ref: net.host.port metrics: - - id: metric.http.server.duration + - id: server.duration brief: "Measures the duration of the inbound HTTP request." instrument: Histogram units: ms - - id: metric.http.server.active_requests + - id: server.active_requests brief: "Measures the number of concurrent HTTP requests that are currently in-flight." instrument: AsynchronousUpDownCounter units: "{requests}" diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index b8f6fc9e..0500be03 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -208,7 +208,7 @@ def test_metrics_http(self): expected = { "id": "metric.http", - "prefix": "http", + "prefix": "metric.http", "extends": "", "n_constraints": 0, "attributes": [ @@ -222,13 +222,13 @@ def test_metrics_http(self): expected = { "id": "metric.http.client", - "prefix": "http", + "prefix": "metric.http", "extends": "metric.http", "n_constraints": 1, "attributes": ["net.peer.name", "net.peer.port", "net.peer.ip"], "metrics": [ { - "id": "metric.http.client.duration", + "id": "client.duration", "instrument": "Histogram", "units": "ms", } @@ -239,18 +239,18 @@ def test_metrics_http(self): expected = { "id": "metric.http.server", - "prefix": "http", + "prefix": "metric.http", "extends": "metric.http", "n_constraints": 1, "attributes": ["http.server_name", "net.host.name", "net.host.port"], "metrics": [ { - "id": "metric.http.server.duration", + "id": "server.duration", "instrument": "Histogram", "units": "ms", }, { - "id": "metric.http.server.active_requests", + "id": "server.active_requests", "instrument": "AsynchronousUpDownCounter", "units": "{requests}", }, From 22373cde5cdce37f287837a1d9f3719a018e3815 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 13 Apr 2022 14:51:55 +1000 Subject: [PATCH 10/54] change InstrumentKind enum to ALL_CAPS and remove sync/async distinction --- .../semconv/model/semantic_convention.py | 12 +++++------- semantic-conventions/syntax.md | 4 +--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 21cf9949..4559dcda 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -54,13 +54,11 @@ def parse(span_kind_value): class InstrumentKind(Enum): - Empty = 1 - Counter = 2 - AsynchronousCounter = 3 - Histogram = 4 - AsynchronousGauge = 5 - UpDownCounter = 6 - AsynchronousUpDownCounter = 7 + EMPTY = 1 + COUNTER = 2 + UP_DOWN_COUNTER = 3 + HISTOGRAM = 4 + GAUGE = 5 def __str__(self): return self.name diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index ae1d08f6..aab9e05f 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -120,11 +120,9 @@ events ::= id {id} # MUST point to an existing event group name ::= string instrument ::= "Counter" - | "AsynchronousCounter" | "Histogram" - | "AsynchronousGauge" + | "Gauge" | "UpDownCounter" - | "AsynchronousUpDownCounter" units ::= string From ec986438223e8c192dd5174318a530fb58855a4d Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 13 Apr 2022 15:55:51 +1000 Subject: [PATCH 11/54] implement syntax review and semantics docs --- semantic-conventions/syntax.md | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index aab9e05f..ce998572 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -119,16 +119,16 @@ events ::= id {id} # MUST point to an existing event group name ::= string -instrument ::= "Counter" - | "Histogram" - | "Gauge" - | "UpDownCounter" +instrument ::= "counter" + | "histogram" + | "gauge" + | "updowncounter" units ::= string metric ::= id instrument units brief -metrics ::= {metrics} +metrics ::= {metric} ``` ## Semantics @@ -176,6 +176,18 @@ The following is only valid if `type` is `event`: If not specified, the `prefix` is used. If `prefix` is empty (or unspecified), `name` is required. +#### Metric semantic convention + +The following is only valid if `type` is `metric`: + +- `metrics`, an optional list of metrics that belong to the semantic convention. + Each individual metric has the following semantics: + - `id`, the ID of the metric. The fully qualified name of the metric includes its parent + semantic convention ID prefixed like so: `{parent.id}.{metric.id}`. + - `brief`, a brief description of the metric. + - `instrument`, the instrument that *should* be used to record the metric. + - `units`, the units in which the metric is measured. + ### Attributes An attribute is defined by: From 9bcc6e49ba822282e29cf69a092ffac6e7c9ebba Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 13 Apr 2022 15:56:32 +1000 Subject: [PATCH 12/54] raise error from different position to avoid re-raising --- .../semconv/model/semantic_convention.py | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 4559dcda..3da9f073 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -252,33 +252,30 @@ class MetricSemanticConvention(BaseSemanticConvention): allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ("metrics",) class Metric: - def __init__(self, metric, parent_prefix): + def __init__(self, metric, parent_prefix, position): self.id: str = metric.get("id") self.fqn = "{}.{}".format(parent_prefix, self.id) self.instrument: InstrumentKind = InstrumentKind[metric.get("instrument")] self.units: str = metric.get("units") self.brief: str = metric.get("brief") + self._position = position if None in [metric.get("instrument"), self.id, self.units, self.brief]: - raise ValueError + raise ValidationError.from_yaml_pos( + self._position, + "id, instrument, units, and brief must all be defined for concrete metrics", + ) def __init__(self, group): super().__init__(group) self.metrics = () if group.get("metrics"): - try: - self.metrics: Tuple[MetricSemanticConvention.Metric] = tuple( - map( - lambda m: MetricSemanticConvention.Metric(m, self.prefix), - group.get("metrics"), - ) + self.metrics: Tuple[MetricSemanticConvention.Metric, ...] = tuple( + map( + lambda m: MetricSemanticConvention.Metric(m, self.prefix), + group.get("metrics"), ) - except ValueError as e: - raise ValidationError.from_yaml_pos( - self._position, - "id, instrument, units, and brief must all be defined for concrete metrics", - ) from e - + ) @dataclass class SemanticConventionSet: From 87dfe5d4d7d667bb1b25147368092982c9304a02 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 13 Apr 2022 15:56:58 +1000 Subject: [PATCH 13/54] remove markdown link --- .../src/opentelemetry/semconv/templating/markdown/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 1974f584..5f5edea7 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -184,8 +184,8 @@ def to_markdown_metric_table( This method renders metrics as markdown table entry """ output.write( - "| Name | Instrument | Unit ([UCUM](README.md#instrument-units)) | Description |\n" - "| -------- | ---------------- | --------- | -------------- |\n" + "| Name | Instrument | Unit (UCUM) | Description |\n" + "| -------- | ---------------- | --------- | -------------- |\n" ) for metric in semconv.metrics: output.write( From 6df71dab0b6cea6a6adb58148afc862b6334a542 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 14 Apr 2022 10:59:45 +1000 Subject: [PATCH 14/54] implement various review items --- .../semconv/model/semantic_convention.py | 46 +++++++++++---- .../semconv/templating/markdown/__init__.py | 59 +++++++++---------- .../data/markdown/metrics_tables/expected.md | 10 ++-- .../src/tests/data/yaml/http_metrics.yaml | 2 +- .../tests/semconv/model/test_correct_parse.py | 3 +- 5 files changed, 73 insertions(+), 47 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 3da9f073..3aa588d0 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -54,14 +54,29 @@ def parse(span_kind_value): class InstrumentKind(Enum): - EMPTY = 1 - COUNTER = 2 - UP_DOWN_COUNTER = 3 - HISTOGRAM = 4 - GAUGE = 5 + COUNTER = 1 + UP_DOWN_COUNTER = 2 + HISTOGRAM = 3 + GAUGE = 4 + + @staticmethod + def parse(instrument_kind_value): + return InstrumentKind.kind_map().get(instrument_kind_value) + + @staticmethod + def kind_map(): + return { + "Counter": InstrumentKind.COUNTER, + "UpDownCounter": InstrumentKind.UP_DOWN_COUNTER, + "Histogram": InstrumentKind.HISTOGRAM, + "Gauge": InstrumentKind.GAUGE, + } def __str__(self): - return self.name + # reverse lookup kind_map() + return next(filter(lambda i: i[1] is self, InstrumentKind.kind_map().items()))[ + 0 + ] def parse_semantic_convention_type(type_value): @@ -255,16 +270,24 @@ class Metric: def __init__(self, metric, parent_prefix, position): self.id: str = metric.get("id") self.fqn = "{}.{}".format(parent_prefix, self.id) - self.instrument: InstrumentKind = InstrumentKind[metric.get("instrument")] + self._position = position self.units: str = metric.get("units") self.brief: str = metric.get("brief") - self._position = position + instrument_str = metric.get("instrument") + self.instrument: InstrumentKind = InstrumentKind.parse(instrument_str) - if None in [metric.get("instrument"), self.id, self.units, self.brief]: + if None in [instrument_str, self.id, self.units, self.brief]: raise ValidationError.from_yaml_pos( self._position, "id, instrument, units, and brief must all be defined for concrete metrics", ) + if self.instrument is None: + raise ValidationError.from_yaml_pos( + self._position, + "Instrument '{}' is not a valid instrument name".format( + metric.get("instrument") + ), + ) def __init__(self, group): super().__init__(group) @@ -272,11 +295,14 @@ def __init__(self, group): if group.get("metrics"): self.metrics: Tuple[MetricSemanticConvention.Metric, ...] = tuple( map( - lambda m: MetricSemanticConvention.Metric(m, self.prefix), + lambda m: MetricSemanticConvention.Metric( + m, self.prefix, self._position + ), group.get("metrics"), ) ) + @dataclass class SemanticConventionSet: """Contains the list of models. diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 5f5edea7..2caf99ca 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -183,9 +183,16 @@ def to_markdown_metric_table( """ This method renders metrics as markdown table entry """ + if not isinstance(semconv, MetricSemanticConvention): + raise ValueError( + "semconv `{}` was specified with `metric_table`, but it is not a metric convention".format( + semconv.semconv_id + ) + ) + output.write( - "| Name | Instrument | Unit (UCUM) | Description |\n" - "| -------- | ---------------- | --------- | -------------- |\n" + "| Name | Instrument | Unit (UCUM) | Description |\n" + "| -------- | ------------- | ----------- | -------------- |\n" ) for metric in semconv.metrics: output.write( @@ -439,39 +446,31 @@ def _render_group(self, semconv, parameters, output): if isinstance(semconv, EventSemanticConvention): output.write("The event name MUST be `{}`.\n\n".format(semconv.name)) + attr_to_print = [] if self.render_ctx.is_metric_table: - if isinstance(semconv, MetricSemanticConvention): - self.to_markdown_metric_table(semconv, output) - else: + self.to_markdown_metric_table(semconv, output) + else: + attr: SemanticAttribute + for attr in sorted( + semconv.attributes, key=lambda a: "" if a.ref is None else a.ref + ): + if self.render_ctx.group_key is not None: + if attr.tag == self.render_ctx.group_key: + attr_to_print.append(attr) + continue + if self.render_ctx.is_full or attr.is_local: + attr_to_print.append(attr) + if self.render_ctx.group_key is not None and not attr_to_print: raise ValueError( - "semconv `{}` was specified with `metric_table`, but it is not a metric convention".format( - semconv.semconv_id + "No attributes retained for '{}' filtering by '{}'".format( + semconv.semconv_id, self.render_ctx.group_key ) ) + if attr_to_print: + output.write(MarkdownRenderer.table_headers) + for attr in attr_to_print: + self.to_markdown_attr(attr, output) - attr_to_print = [] - attr: SemanticAttribute - for attr in sorted( - semconv.attributes, key=lambda a: "" if a.ref is None else a.ref - ): - if self.render_ctx.is_metric_table: - break - if self.render_ctx.group_key is not None: - if attr.tag == self.render_ctx.group_key: - attr_to_print.append(attr) - continue - if self.render_ctx.is_full or attr.is_local: - attr_to_print.append(attr) - if self.render_ctx.group_key is not None and not attr_to_print: - raise ValueError( - "No attributes retained for '{}' filtering by '{}'".format( - semconv.semconv_id, self.render_ctx.group_key - ) - ) - if attr_to_print: - output.write(MarkdownRenderer.table_headers) - for attr in attr_to_print: - self.to_markdown_attr(attr, output) self.to_markdown_notes(output) if not self.render_ctx.is_remove_constraint: for cnst in semconv.constraints: diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md index 15788ded..ef7207e2 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -19,8 +19,8 @@ The following attributes SHOULD be included on all HTTP metrics for both server ### HTTP Client Metrics -| Name | Instrument | Unit ([UCUM](README.md#instrument-units)) | Description | -| -------- | ---------------- | --------- | -------------- | +| Name | Instrument | Unit (UCUM) | Description | +| -------- | ------------- | ----------- | -------------- | | `metric.http.client.duration` | Histogram | `ms` | Measures the duration of the outbound HTTP request. | @@ -48,10 +48,10 @@ The following attributes SHOULD be included on HTTP Client metrics, where applic ### HTTP Server Metrics -| Name | Instrument | Unit ([UCUM](README.md#instrument-units)) | Description | -| -------- | ---------------- | --------- | -------------- | +| Name | Instrument | Unit (UCUM) | Description | +| -------- | ------------- | ----------- | -------------- | | `metric.http.server.duration` | Histogram | `ms` | Measures the duration of the inbound HTTP request. | -| `metric.http.server.active_requests` | AsynchronousUpDownCounter | `{requests}` | Measures the number of concurrent HTTP requests that are currently in-flight. | +| `metric.http.server.active_requests` | UpDownCounter | `{requests}` | Measures the number of concurrent HTTP requests that are currently in-flight. | ### HTTP Server Attributes diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml index deb76dcc..2839967a 100644 --- a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml @@ -60,5 +60,5 @@ groups: units: ms - id: server.active_requests brief: "Measures the number of concurrent HTTP requests that are currently in-flight." - instrument: AsynchronousUpDownCounter + instrument: UpDownCounter units: "{requests}" diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index 0500be03..05abdb9a 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -20,6 +20,7 @@ from opentelemetry.semconv.model.semantic_attribute import StabilityLevel from opentelemetry.semconv.model.semantic_convention import ( EventSemanticConvention, + InstrumentKind, MetricSemanticConvention, SemanticConventionSet, SpanSemanticConvention, @@ -251,7 +252,7 @@ def test_metrics_http(self): }, { "id": "server.active_requests", - "instrument": "AsynchronousUpDownCounter", + "instrument": "UpDownCounter", "units": "{requests}", }, ], From 93eb44337006af2a44434cf7f2dc53aa30ec435c Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 14 Apr 2022 13:23:53 +1000 Subject: [PATCH 15/54] upgrade black version to try solve build failure --- semantic-conventions/dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/semantic-conventions/dev-requirements.txt b/semantic-conventions/dev-requirements.txt index fca6d6cf..06728756 100644 --- a/semantic-conventions/dev-requirements.txt +++ b/semantic-conventions/dev-requirements.txt @@ -1,4 +1,4 @@ -black==21.8b0 +black==22.3.0 mypy==0.910 pytest==6.2.5 flake8==3.9.2 From a77142ce22d87ade750b49ddbad24fcbf25a2032 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 14 Apr 2022 13:26:35 +1000 Subject: [PATCH 16/54] appease linter --- .../src/tests/semconv/model/test_correct_parse.py | 1 - 1 file changed, 1 deletion(-) diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index 05abdb9a..5773c1fa 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -20,7 +20,6 @@ from opentelemetry.semconv.model.semantic_attribute import StabilityLevel from opentelemetry.semconv.model.semantic_convention import ( EventSemanticConvention, - InstrumentKind, MetricSemanticConvention, SemanticConventionSet, SpanSemanticConvention, From c50b9e0885898fe61415fbdfe7610f1b66eef6c4 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 14 Apr 2022 13:32:32 +1000 Subject: [PATCH 17/54] fix typo --- semantic-conventions/syntax.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index ce998572..aebb85e6 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -183,7 +183,7 @@ The following is only valid if `type` is `metric`: - `metrics`, an optional list of metrics that belong to the semantic convention. Each individual metric has the following semantics: - `id`, the ID of the metric. The fully qualified name of the metric includes its parent - semantic convention ID prefixed like so: `{parent.id}.{metric.id}`. + semantic convention prefix like so: `{parent.prefix}.{metric.id}`. - `brief`, a brief description of the metric. - `instrument`, the instrument that *should* be used to record the metric. - `units`, the units in which the metric is measured. From 13316cd4bdb13073f08589716d6e105fc2ac78f1 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 26 Apr 2022 16:21:49 +1000 Subject: [PATCH 18/54] stop using enum for instrument kind --- .../semconv/model/semantic_convention.py | 43 ++++--------------- 1 file changed, 9 insertions(+), 34 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 3aa588d0..f8028302 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -53,32 +53,6 @@ def parse(span_kind_value): return kind_map.get(span_kind_value) -class InstrumentKind(Enum): - COUNTER = 1 - UP_DOWN_COUNTER = 2 - HISTOGRAM = 3 - GAUGE = 4 - - @staticmethod - def parse(instrument_kind_value): - return InstrumentKind.kind_map().get(instrument_kind_value) - - @staticmethod - def kind_map(): - return { - "Counter": InstrumentKind.COUNTER, - "UpDownCounter": InstrumentKind.UP_DOWN_COUNTER, - "Histogram": InstrumentKind.HISTOGRAM, - "Gauge": InstrumentKind.GAUGE, - } - - def __str__(self): - # reverse lookup kind_map() - return next(filter(lambda i: i[1] is self, InstrumentKind.kind_map().items()))[ - 0 - ] - - def parse_semantic_convention_type(type_value): # Gracefully transition to the new types if type_value is None: @@ -267,26 +241,27 @@ class MetricSemanticConvention(BaseSemanticConvention): allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ("metrics",) class Metric: + allowed_instruments: Tuple[str, ...] = ("Counter", "UpDownCounter", "Histogram", "Gauge") + def __init__(self, metric, parent_prefix, position): self.id: str = metric.get("id") self.fqn = "{}.{}".format(parent_prefix, self.id) self._position = position self.units: str = metric.get("units") self.brief: str = metric.get("brief") - instrument_str = metric.get("instrument") - self.instrument: InstrumentKind = InstrumentKind.parse(instrument_str) + self.instrument: str = metric.get("instrument") - if None in [instrument_str, self.id, self.units, self.brief]: + if self.instrument not in self.allowed_instruments: raise ValidationError.from_yaml_pos( self._position, - "id, instrument, units, and brief must all be defined for concrete metrics", + "Instrument '{}' is not a valid instrument name".format( + self.instrument + ), ) - if self.instrument is None: + if None in [self.instrument, self.id, self.units, self.brief]: raise ValidationError.from_yaml_pos( self._position, - "Instrument '{}' is not a valid instrument name".format( - metric.get("instrument") - ), + "id, instrument, units, and brief must all be defined for concrete metrics", ) def __init__(self, group): From e01f29670534463411f4d6793501c2edcbeb1076 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 26 Apr 2022 16:47:59 +1000 Subject: [PATCH 19/54] extract attribute table writing to own function --- .../semconv/templating/markdown/__init__.py | 59 ++++++++++--------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 642bcab1..5c3a6bb4 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -29,6 +29,7 @@ StabilityLevel, ) from opentelemetry.semconv.model.semantic_convention import ( + BaseSemanticConvention, EventSemanticConvention, MetricSemanticConvention, SemanticConventionSet, @@ -176,6 +177,32 @@ def to_markdown_attr( ) ) + def to_markdown_attribute_table(self, semconv: BaseSemanticConvention, output: io.StringIO): + attr_to_print = [] + for attr in sorted( + semconv.attributes, key=lambda a: "" if a.ref is None else a.ref + ): + if self.render_ctx.group_key is not None: + if attr.tag == self.render_ctx.group_key: + attr_to_print.append(attr) + continue + if self.render_ctx.is_full or attr.is_local: + attr_to_print.append(attr) + if self.render_ctx.group_key is not None and not attr_to_print: + raise ValueError( + "No attributes retained for '{}' filtering by '{}'".format( + semconv.semconv_id, self.render_ctx.group_key + ) + ) + if attr_to_print: + output.write(MarkdownRenderer.table_headers) + for attr in attr_to_print: + self.to_markdown_attr(attr, output) + attr_sampling_relevant = [ + attr for attr in attr_to_print if attr.sampling_relevant + ] + self.to_creation_time_attributes(attr_sampling_relevant, output) + @staticmethod def to_markdown_metric_table( semconv: MetricSemanticConvention, output: io.StringIO @@ -445,33 +472,12 @@ def _render_group(self, semconv, parameters, output): self.render_ctx.is_full = "full" in parameters self.render_ctx.is_metric_table = "metric_table" in parameters - if isinstance(semconv, EventSemanticConvention): - output.write("The event name MUST be `{}`.\n\n".format(semconv.name)) - - attr_to_print = [] if self.render_ctx.is_metric_table: self.to_markdown_metric_table(semconv, output) else: - attr: SemanticAttribute - for attr in sorted( - semconv.attributes, key=lambda a: "" if a.ref is None else a.ref - ): - if self.render_ctx.group_key is not None: - if attr.tag == self.render_ctx.group_key: - attr_to_print.append(attr) - continue - if self.render_ctx.is_full or attr.is_local: - attr_to_print.append(attr) - if self.render_ctx.group_key is not None and not attr_to_print: - raise ValueError( - "No attributes retained for '{}' filtering by '{}'".format( - semconv.semconv_id, self.render_ctx.group_key - ) - ) - if attr_to_print: - output.write(MarkdownRenderer.table_headers) - for attr in attr_to_print: - self.to_markdown_attr(attr, output) + if isinstance(semconv, EventSemanticConvention): + output.write("The event name MUST be `{}`.\n\n".format(semconv.name)) + self.to_markdown_attribute_table(semconv, output) self.to_markdown_notes(output) if not self.render_ctx.is_remove_constraint: @@ -482,9 +488,4 @@ def _render_group(self, semconv, parameters, output): if isinstance(semconv, UnitSemanticConvention): self.to_markdown_unit_table(semconv.members, output) - attr_sampling_relevant = [ - attr for attr in attr_to_print if attr.sampling_relevant - ] - self.to_creation_time_attributes(attr_sampling_relevant, output) - output.write("") From 5fbdcf93f5b68ecebc5f1d3adf56870c21eb4dd7 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 26 Apr 2022 16:48:30 +1000 Subject: [PATCH 20/54] appease linter --- .../src/opentelemetry/semconv/model/semantic_convention.py | 7 ++++++- .../opentelemetry/semconv/templating/markdown/__init__.py | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index f8028302..4b974657 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -241,7 +241,12 @@ class MetricSemanticConvention(BaseSemanticConvention): allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ("metrics",) class Metric: - allowed_instruments: Tuple[str, ...] = ("Counter", "UpDownCounter", "Histogram", "Gauge") + allowed_instruments: Tuple[str, ...] = ( + "Counter", + "UpDownCounter", + "Histogram", + "Gauge", + ) def __init__(self, metric, parent_prefix, position): self.id: str = metric.get("id") diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 5c3a6bb4..5a7e92d0 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -177,10 +177,12 @@ def to_markdown_attr( ) ) - def to_markdown_attribute_table(self, semconv: BaseSemanticConvention, output: io.StringIO): + def to_markdown_attribute_table( + self, semconv: BaseSemanticConvention, output: io.StringIO + ): attr_to_print = [] for attr in sorted( - semconv.attributes, key=lambda a: "" if a.ref is None else a.ref + semconv.attributes, key=lambda a: "" if a.ref is None else a.ref ): if self.render_ctx.group_key is not None: if attr.tag == self.render_ctx.group_key: From a3b7e7ff68b53a72ae848e5898e3eba1a6323169 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 26 Apr 2022 16:55:57 +1000 Subject: [PATCH 21/54] add link to UCUM --- semantic-conventions/syntax.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index aebb85e6..c3f3ec4f 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -186,7 +186,8 @@ The following is only valid if `type` is `metric`: semantic convention prefix like so: `{parent.prefix}.{metric.id}`. - `brief`, a brief description of the metric. - `instrument`, the instrument that *should* be used to record the metric. - - `units`, the units in which the metric is measured. + - `units`, the units in which the metric is measured, which should adhere to + [UCUM](https://ucum.org/ucum.html). ### Attributes From 8ec68eddb14993548702d980aadf46d82f94e0a7 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 26 Apr 2022 17:04:36 +1000 Subject: [PATCH 22/54] consistent camelcase syntax --- semantic-conventions/syntax.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index c3f3ec4f..f7e3d83f 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -119,10 +119,10 @@ events ::= id {id} # MUST point to an existing event group name ::= string -instrument ::= "counter" - | "histogram" - | "gauge" - | "updowncounter" +instrument ::= "Counter" + | "Histogram" + | "Gauge" + | "UpDownCounter" units ::= string From dfe7b0e626caa29ebaf7cea2b46559896d561803 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Mon, 6 Jun 2022 17:16:02 +1000 Subject: [PATCH 23/54] Address reyang comments --- .../src/opentelemetry/semconv/model/semantic_convention.py | 2 +- semantic-conventions/syntax.md | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index aa0fcbad..25c5831f 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -254,7 +254,7 @@ class Metric: def __init__(self, metric, parent_prefix, position): self.id: str = metric.get("id") - self.fqn = "{}.{}".format(parent_prefix, self.id) + self.fqn = "{}.{}".format(parent_prefix, self.id).removeprefix("metric.") self._position = position self.units: str = metric.get("units") self.brief: str = metric.get("brief") diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index 61d096c6..2e381aa7 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -186,7 +186,8 @@ The following is only valid if `type` is `metric`: - `id`, the ID of the metric. The fully qualified name of the metric includes its parent semantic convention prefix like so: `{parent.prefix}.{metric.id}`. - `brief`, a brief description of the metric. - - `instrument`, the instrument that *should* be used to record the metric. + - `instrument`, the [instrument type]( https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics/semantic_conventions#instrument-types) + that *should* be used to record the metric. - `units`, the units in which the metric is measured, which should adhere to [UCUM](https://ucum.org/ucum.html). From cde420013c7ea9a4fb8fa57dcda3fb639a0ba591 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Mon, 6 Jun 2022 17:16:14 +1000 Subject: [PATCH 24/54] update tests so they pass with latest main --- .../data/markdown/metrics_tables/expected.md | 20 +++++++++---------- .../data/markdown/metrics_tables/http.yaml | 6 +++--- .../src/tests/data/yaml/http_metrics.yaml | 10 +++++----- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md index ef7207e2..6e461f80 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -4,12 +4,12 @@ The following attributes SHOULD be included on all HTTP metrics for both server and client. -| Attribute | Type | Description | Examples | Required | +| Attribute | Type | Description | Examples | Requirement Level | |---|---|---|---|---| -| `http.host` | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). An empty Host header should also be reported, see note. [1] | `www.example.org` | See attribute alternatives | -| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Yes | -| `http.scheme` | string | The URI scheme identifying the used protocol. | `http`; `https` | See attribute alternative | -| `http.status_code` | int | [HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6). | `200` | No | +| `http.host` | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). An empty Host header should also be reported, see note. [1] | `www.example.org` | Conditionally Required: See attribute alternatives | +| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required | +| `http.scheme` | string | The URI scheme identifying the used protocol. | `http`; `https` | Conditionally Required: See attribute alternative | +| `http.status_code` | int | [HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6). | `200` | Recommended | **[1]:** When the header is present but empty the attribute SHOULD be set to the empty string. Note that this is a valid situation that is expected in certain cases, according the aforementioned [section of RFC 7230](https://tools.ietf.org/html/rfc7230#section-5.4). When the header is not set the attribute MUST NOT be set. @@ -21,7 +21,7 @@ The following attributes SHOULD be included on all HTTP metrics for both server | Name | Instrument | Unit (UCUM) | Description | | -------- | ------------- | ----------- | -------------- | -| `metric.http.client.duration` | Histogram | `ms` | Measures the duration of the outbound HTTP request. | +| `http.client.duration` | Histogram | `ms` | Measures the duration of the outbound HTTP request. | ### HTTP Client Attributes @@ -29,7 +29,7 @@ The following attributes SHOULD be included on all HTTP metrics for both server The following attributes SHOULD be included on HTTP Client metrics, where applicable and available. -| Attribute | Type | Description | Examples | Required | +| Attribute | Type | Description | Examples | Requirement Level | |---|---|---|---|---| | `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 | @@ -50,8 +50,8 @@ The following attributes SHOULD be included on HTTP Client metrics, where applic | Name | Instrument | Unit (UCUM) | Description | | -------- | ------------- | ----------- | -------------- | -| `metric.http.server.duration` | Histogram | `ms` | Measures the duration of the inbound HTTP request. | -| `metric.http.server.active_requests` | UpDownCounter | `{requests}` | Measures the number of concurrent HTTP requests that are currently in-flight. | +| `http.server.duration` | Histogram | `ms` | Measures the duration of the inbound HTTP request. | +| `http.server.active_requests` | UpDownCounter | `{requests}` | Measures the number of concurrent HTTP requests that are currently in-flight. | ### HTTP Server Attributes @@ -59,7 +59,7 @@ The following attributes SHOULD be included on HTTP Client metrics, where applic The following attributes SHOULD be included on HTTP Server metrics, where applicable and available. -| Attribute | Type | Description | Examples | Required | +| Attribute | Type | Description | Examples | Requirement Level | |---|---|---|---|---| | `http.server_name` | string | The primary server name of the matched virtual host. This should be obtained via configuration. If no such configuration can be obtained, this attribute MUST NOT be set ( `net.host.name` should be used instead). [1] | `example.com` | See below | | `net.host.name` | string | Local hostname or similar, see note below. | `localhost` | See below | diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml b/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml index 20adebca..ffa3f133 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml @@ -8,7 +8,7 @@ groups: attributes: - id: method type: string - required: always + requirement_level: required brief: 'HTTP request method.' sampling_relevant: true examples: ["GET", "POST", "HEAD"] @@ -47,8 +47,8 @@ groups: examples: ["http", "https"] - id: status_code type: int - required: - conditional: If and only if one was received/sent. + requirement_level: + conditionally_required: If and only if one was received/sent. brief: '[HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6).' examples: [200] - id: flavor diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml index 2839967a..e90691af 100644 --- a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml @@ -8,13 +8,13 @@ groups: and various HTTP versions like 1.1, 2 and SPDY. attributes: - ref: http.method - required: always + requirement_level: required - ref: http.host - required: - conditional: "See attribute alternatives" + requirement_level: + conditionally_required: "See attribute alternatives" - ref: http.scheme - required: - conditional: "See attribute alternative" + requirement_level: + conditionally_required: "See attribute alternative" - ref: http.status_code - id: metric.http.client From 9217e78e3a71e4698ee58a86c8176001a020ea4d Mon Sep 17 00:00:00 2001 From: James Moessis Date: Mon, 6 Jun 2022 17:36:10 +1000 Subject: [PATCH 25/54] move logic of stripping metric. prefix --- .../src/opentelemetry/semconv/model/semantic_convention.py | 2 +- .../src/opentelemetry/semconv/templating/markdown/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 25c5831f..aa0fcbad 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -254,7 +254,7 @@ class Metric: def __init__(self, metric, parent_prefix, position): self.id: str = metric.get("id") - self.fqn = "{}.{}".format(parent_prefix, self.id).removeprefix("metric.") + self.fqn = "{}.{}".format(parent_prefix, self.id) self._position = position self.units: str = metric.get("units") self.brief: str = metric.get("brief") diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 120f63c2..2e7f8ec3 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -238,7 +238,7 @@ def to_markdown_metric_table( for metric in semconv.metrics: output.write( "| `{}` | {} | `{}` | {} |\n".format( - metric.fqn, metric.instrument, metric.units, metric.brief + metric.fqn.removeprefix("metric."), metric.instrument, metric.units, metric.brief ) ) From 9178ab34c70a6d48972e5beb14e2de0a233063eb Mon Sep 17 00:00:00 2001 From: James Moessis Date: Mon, 6 Jun 2022 17:39:56 +1000 Subject: [PATCH 26/54] appease linter --- .../opentelemetry/semconv/templating/markdown/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 2e7f8ec3..1b40d3c0 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -238,7 +238,10 @@ def to_markdown_metric_table( for metric in semconv.metrics: output.write( "| `{}` | {} | `{}` | {} |\n".format( - metric.fqn.removeprefix("metric."), metric.instrument, metric.units, metric.brief + metric.fqn.removeprefix("metric."), + metric.instrument, + metric.units, + metric.brief, ) ) From a012741a9e3691643d02024ea7b3ad3fd6658701 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 7 Jun 2022 09:56:18 +1000 Subject: [PATCH 27/54] remove 'metric.' prefix so we don't have to removeprefix() when serialising --- .../opentelemetry/semconv/templating/markdown/__init__.py | 2 +- semantic-conventions/src/tests/data/yaml/http_metrics.yaml | 6 +++--- .../src/tests/semconv/model/test_correct_parse.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 1b40d3c0..da0e6912 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -238,7 +238,7 @@ def to_markdown_metric_table( for metric in semconv.metrics: output.write( "| `{}` | {} | `{}` | {} |\n".format( - metric.fqn.removeprefix("metric."), + metric.fqn, metric.instrument, metric.units, metric.brief, diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml index e90691af..f39e17fc 100644 --- a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml @@ -1,6 +1,6 @@ groups: - id: metric.http - prefix: metric.http + prefix: http type: metric brief: "This document defines semantic convention for HTTP client and server metrics." note: > @@ -18,7 +18,7 @@ groups: - ref: http.status_code - id: metric.http.client - prefix: metric.http + prefix: http type: metric extends: metric.http brief: 'Semantic Convention for HTTP Client' @@ -39,7 +39,7 @@ groups: units: ms - id: metric.http.server - prefix: metric.http + prefix: http type: metric extends: metric.http brief: "Semantic Convention for HTTP Server" diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index 5773c1fa..444e5929 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -208,7 +208,7 @@ def test_metrics_http(self): expected = { "id": "metric.http", - "prefix": "metric.http", + "prefix": "http", "extends": "", "n_constraints": 0, "attributes": [ @@ -222,7 +222,7 @@ def test_metrics_http(self): expected = { "id": "metric.http.client", - "prefix": "metric.http", + "prefix": "http", "extends": "metric.http", "n_constraints": 1, "attributes": ["net.peer.name", "net.peer.port", "net.peer.ip"], @@ -239,7 +239,7 @@ def test_metrics_http(self): expected = { "id": "metric.http.server", - "prefix": "metric.http", + "prefix": "http", "extends": "metric.http", "n_constraints": 1, "attributes": ["http.server_name", "net.host.name", "net.host.port"], From f028520c032dfd5a4053a86f22beff0486ae7d39 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 9 Sep 2022 10:31:03 +1000 Subject: [PATCH 28/54] make test match current changed expectation for requirement level --- .../data/markdown/metrics_tables/expected.md | 2 +- .../src/tests/data/yaml/http_metrics_new.yaml | 64 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md index 6e461f80..fd9bc5de 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -9,7 +9,7 @@ The following attributes SHOULD be included on all HTTP metrics for both server | `http.host` | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). An empty Host header should also be reported, see note. [1] | `www.example.org` | Conditionally Required: See attribute alternatives | | `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required | | `http.scheme` | string | The URI scheme identifying the used protocol. | `http`; `https` | Conditionally Required: See attribute alternative | -| `http.status_code` | int | [HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6). | `200` | 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. | **[1]:** When the header is present but empty the attribute SHOULD be set to the empty string. Note that this is a valid situation that is expected in certain cases, according the aforementioned [section of RFC 7230](https://tools.ietf.org/html/rfc7230#section-5.4). When the header is not set the attribute MUST NOT be set. diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml new file mode 100644 index 00000000..67e08066 --- /dev/null +++ b/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml @@ -0,0 +1,64 @@ +groups: + - id: metric.http + prefix: http + type: metric + brief: "This document defines semantic convention for HTTP client and server metrics." + note: > + These conventions can be used for http and https schemes + and various HTTP versions like 1.1, 2 and SPDY. + attributes: + - ref: http.method + requirement_level: required + - ref: http.host + requirement_level: + conditionally_required: "See attribute alternatives" + - ref: http.scheme + requirement_level: + conditionally_required: "See attribute alternative" + - ref: http.status_code + + - id: metric.http.client.duration + prefix: http + type: metric + extends: metric.http + brief: 'Semantic Convention for HTTP Client' + constraints: + - any_of: + - [http.url] + - [http.scheme, http.host, http.target] + - [http.scheme, net.peer.name, net.peer.port, http.target] + - [http.scheme, net.peer.ip, net.peer.port, http.target] + attributes: + - ref: net.peer.name + - ref: net.peer.port + - ref: net.peer.ip + metrics: + - id: client.duration + brief: "Measures the duration of the outbound HTTP request." + instrument: Histogram + units: ms + + - id: metric.http.server + prefix: http + type: metric + extends: metric.http + brief: "Semantic Convention for HTTP Server" + constraints: + - any_of: + - [http.scheme, http.host, http.target] + - [http.scheme, http.server_name, net.host.port, http.target] + - [http.scheme, net.host.name, net.host.port, http.target] + - [http.url] + attributes: + - ref: http.server_name + - ref: net.host.name + - ref: net.host.port + metrics: + - id: server.duration + brief: "Measures the duration of the inbound HTTP request." + instrument: Histogram + units: ms + - id: server.active_requests + brief: "Measures the number of concurrent HTTP requests that are currently in-flight." + instrument: UpDownCounter + units: "{requests}" From 68b1154bb24c15d269057dee4ecd4d4668753150 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 9 Sep 2022 11:28:03 +1000 Subject: [PATCH 29/54] new yaml structure for metrics --- .../src/tests/data/yaml/http_metrics_new.yaml | 212 +++++++++++++----- 1 file changed, 159 insertions(+), 53 deletions(-) diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml index 67e08066..ce509024 100644 --- a/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml +++ b/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml @@ -6,59 +6,165 @@ groups: note: > These conventions can be used for http and https schemes and various HTTP versions like 1.1, 2 and SPDY. - attributes: - - ref: http.method - requirement_level: required - - ref: http.host - requirement_level: - conditionally_required: "See attribute alternatives" - - ref: http.scheme - requirement_level: - conditionally_required: "See attribute alternative" - - ref: http.status_code - id: metric.http.client.duration - prefix: http - type: metric - extends: metric.http - brief: 'Semantic Convention for HTTP Client' - constraints: - - any_of: - - [http.url] - - [http.scheme, http.host, http.target] - - [http.scheme, net.peer.name, net.peer.port, http.target] - - [http.scheme, net.peer.ip, net.peer.port, http.target] - attributes: - - ref: net.peer.name - - ref: net.peer.port - - ref: net.peer.ip - metrics: - - id: client.duration - brief: "Measures the duration of the outbound HTTP request." - instrument: Histogram - units: ms + prefix: http + type: metric + extends: metric.http + name: http.client.duration + brief: "Measures the duration of the outbound HTTP request." + instrument: Histogram + units: ms + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.flavor + requirement_level: recommended + - ref: net.peer.name + requirement_level: required + - ref: net.peer.port + requirement_level: + conditionally_required: If not default (`80` for `http`, `443` for `https`). + - ref: net.sock.peer.addr + requirement_level: recommended - - id: metric.http.server - prefix: http - type: metric - extends: metric.http - brief: "Semantic Convention for HTTP Server" - constraints: - - any_of: - - [http.scheme, http.host, http.target] - - [http.scheme, http.server_name, net.host.port, http.target] - - [http.scheme, net.host.name, net.host.port, http.target] - - [http.url] - attributes: - - ref: http.server_name - - ref: net.host.name - - ref: net.host.port - metrics: - - id: server.duration - brief: "Measures the duration of the inbound HTTP request." - instrument: Histogram - units: ms - - id: server.active_requests - brief: "Measures the number of concurrent HTTP requests that are currently in-flight." - instrument: UpDownCounter - units: "{requests}" + - id: metric.http.client.request.size + prefix: http + type: metric + extends: metric.http + name: http.client.request.size + brief: "Measures the size of HTTP request messages (compressed)." + instrument: Histogram + units: By + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.flavor + requirement_level: recommended + - ref: net.peer.name + requirement_level: required + - ref: net.peer.port + requirement_level: + conditionally_required: If not default (`80` for `http`, `443` for `https`). + - ref: net.sock.peer.addr + requirement_level: recommended + + - id: metric.http.client.response.size + prefix: http + type: metric + extends: metric.http + name: http.client.response.size + brief: "Measures the size of HTTP response messages (compressed)." + instrument: Histogram + units: By + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.flavor + requirement_level: recommended + - ref: net.peer.name + requirement_level: required + - ref: net.peer.port + requirement_level: + conditionally_required: "if not default (`80` for `http`, `443` for `https`)." + - ref: net.sock.peer.addr + requirement_level: recommended + + - id: metric.http.server.duration + prefix: http + type: metric + extends: metric.http + name: http.server.duration + brief: "Measures the duration inbound HTTP requests." + instrument: Histogram + units: ms + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.scheme + requirement_level: required + - ref: http.flavor + requirement_level: recommended + - ref: net.host.name + requirement_level: required + - ref: net.host.port + requirement_level: + conditionally_required: "if not default (`80` for `http`, `443` for `https`)." + + - id: metric.http.server.request.size + prefix: http + type: metric + extends: metric.http + name: http.server.request.size + brief: "Measures the size of HTTP request messages (compressed)." + instrument: Histogram + units: By + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.scheme + requirement_level: required + - ref: http.flavor + requirement_level: recommended + - ref: net.host.name + requirement_level: required + - ref: net.host.port + requirement_level: + conditionally_required: "if not default (`80` for `http`, `443` for `https`)." + + - id: metric.http.server.request.size + prefix: http + type: metric + extends: metric.http + name: http.server.response.size + brief: "Measures the size of HTTP response messages (compressed)." + instrument: Histogram + units: By + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.scheme + requirement_level: required + - ref: http.flavor + requirement_level: recommended + - ref: net.host.name + requirement_level: required + - ref: net.host.port + requirement_level: + conditionally_required: "if not default (`80` for `http`, `443` for `https`)." + + - id: metric.http.server.active_requests + prefix: http + type: metric + extends: metric.http + name: http.server.request.size + brief: "Measures the number of concurrent HTTP requests that are currently in-flight." + instrument: UpDownCounter + units: "{requests}" + attributes: + - ref: http.method + requirement_level: required + - ref: http.scheme + requirement_level: required + - ref: http.flavor + requirement_level: recommended + - ref: net.host.name + requirement_level: required From 849c3763bd5124c2d225f65709a75103db812759 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 9 Sep 2022 14:37:54 +1000 Subject: [PATCH 30/54] change the structure of yaml for metrics --- .../semconv/model/semantic_convention.py | 68 +++---- .../semconv/templating/markdown/__init__.py | 19 +- .../src/tests/data/yaml/http_metrics.yaml | 178 ++++++++++++++---- .../src/tests/data/yaml/http_metrics_new.yaml | 170 ----------------- .../tests/semconv/model/test_correct_parse.py | 61 ++---- 5 files changed, 196 insertions(+), 300 deletions(-) delete mode 100644 semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 4616153d..1f69208f 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -16,7 +16,7 @@ import typing from dataclasses import dataclass, field from enum import Enum -from typing import Tuple, Union +from typing import Tuple, Union, Dict from ruamel.yaml import YAML @@ -247,50 +247,42 @@ def __init__(self, group): class MetricSemanticConvention(BaseSemanticConvention): GROUP_TYPE_NAME = "metric" - allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ("metrics",) + allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ( + "name", + "units", + "instrument" + ) - class Metric: - allowed_instruments: Tuple[str, ...] = ( - "Counter", - "UpDownCounter", - "Histogram", - "Gauge", - ) + yaml_to_markdown_instrument_repr: Dict[str, str] = { + "counter": "Counter", + "updowncounter": "UpDownCounter", + "histogram": "Histogram", + "gauge": "Gauge", + } - def __init__(self, metric, parent_prefix, position): - self.id: str = metric.get("id") - self.fqn = "{}.{}".format(parent_prefix, self.id) - self._position = position - self.units: str = metric.get("units") - self.brief: str = metric.get("brief") - self.instrument: str = metric.get("instrument") - - if self.instrument not in self.allowed_instruments: - raise ValidationError.from_yaml_pos( - self._position, - "Instrument '{}' is not a valid instrument name".format( - self.instrument - ), - ) - if None in [self.instrument, self.id, self.units, self.brief]: - raise ValidationError.from_yaml_pos( - self._position, - "id, instrument, units, and brief must all be defined for concrete metrics", - ) + allowed_instruments: Tuple[str, ...] = tuple(yaml_to_markdown_instrument_repr.keys()) + (None,) def __init__(self, group): super().__init__(group) - self.metrics = () - if group.get("metrics"): - self.metrics: Tuple[MetricSemanticConvention.Metric, ...] = tuple( - map( - lambda m: MetricSemanticConvention.Metric( - m, self.prefix, self._position - ), - group.get("metrics"), - ) + self.name = group.get("name") + self.units = group.get("units") + self.instrument = group.get("instrument") + self.instrument_markdown_fmt = self.yaml_to_markdown_instrument_repr.get(self.instrument) + self.validate() + + def validate(self): + val_tuple = (self.name, self.units, self.instrument) + if not all(val_tuple) and any(val_tuple): + raise ValidationError.from_yaml_pos( + self._position, + "Either all or none of name, units, and instrument must be defined" ) + if self.instrument not in self.allowed_instruments: + raise ValidationError.from_yaml_pos( + self._position, + "Instrument '{}' is not a valid instrument name".format(self.instrument) + ) @dataclass class SemanticConventionSet: diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 39705a85..3ac0cda3 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -232,18 +232,17 @@ def to_markdown_metric_table( ) output.write( - "| Name | Instrument | Unit (UCUM) | Description |\n" - "| -------- | ------------- | ----------- | -------------- |\n" + "| Name | Instrument Type | Unit (UCUM) | Description |\n" + "| -------- | --------------- | ----------- | -------------- |\n" ) - for metric in semconv.metrics: - output.write( - "| `{}` | {} | `{}` | {} |\n".format( - metric.fqn, - metric.instrument, - metric.units, - metric.brief, - ) + output.write( + "| `{}` | {} | `{}` | {} |\n".format( + semconv.name, + semconv.instrument_markdown_fmt, + semconv.units, + semconv.brief, ) + ) def to_markdown_anyof(self, anyof: AnyOf, output: io.StringIO): """ diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml index f39e17fc..14f19601 100644 --- a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml @@ -6,59 +6,165 @@ groups: note: > These conventions can be used for http and https schemes and various HTTP versions like 1.1, 2 and SPDY. + + - id: metric.http.client.duration + prefix: http + type: metric + extends: metric.http + name: http.client.duration + brief: "Measures the duration of the outbound HTTP request." + instrument: histogram + units: ms attributes: - ref: http.method requirement_level: required - - ref: http.host + - ref: http.status_code requirement_level: - conditionally_required: "See attribute alternatives" - - ref: http.scheme + conditionally_required: "if and only if one was received/sent." + - ref: http.flavor + requirement_level: recommended + - ref: net.peer.name + requirement_level: required + - ref: net.peer.port requirement_level: - conditionally_required: "See attribute alternative" + conditionally_required: If not default (`80` for `http`, `443` for `https`). + - ref: net.sock.peer.addr + requirement_level: recommended + + - id: metric.http.client.request.size + prefix: http + type: metric + extends: metric.http + name: http.client.request.size + brief: "Measures the size of HTTP request messages (compressed)." + instrument: histogram + units: By + attributes: + - ref: http.method + requirement_level: required - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.flavor + requirement_level: recommended + - ref: net.peer.name + requirement_level: required + - ref: net.peer.port + requirement_level: + conditionally_required: If not default (`80` for `http`, `443` for `https`). + - ref: net.sock.peer.addr + requirement_level: recommended - - id: metric.http.client + - id: metric.http.client.response.size prefix: http type: metric extends: metric.http - brief: 'Semantic Convention for HTTP Client' - constraints: - - any_of: - - [http.url] - - [http.scheme, http.host, http.target] - - [http.scheme, net.peer.name, net.peer.port, http.target] - - [http.scheme, net.peer.ip, net.peer.port, http.target] + name: http.client.response.size + brief: "Measures the size of HTTP response messages (compressed)." + instrument: histogram + units: By attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.flavor + requirement_level: recommended - ref: net.peer.name + requirement_level: required - ref: net.peer.port - - ref: net.peer.ip - metrics: - - id: client.duration - brief: "Measures the duration of the outbound HTTP request." - instrument: Histogram - units: ms + requirement_level: + conditionally_required: "if not default (`80` for `http`, `443` for `https`)." + - ref: net.sock.peer.addr + requirement_level: recommended + + - id: metric.http.server.duration + prefix: http + type: metric + extends: metric.http + name: http.server.duration + brief: "Measures the duration inbound HTTP requests." + instrument: histogram + units: ms + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.scheme + requirement_level: required + - ref: http.flavor + requirement_level: recommended + - ref: net.host.name + requirement_level: required + - ref: net.host.port + requirement_level: + conditionally_required: "if not default (`80` for `http`, `443` for `https`)." + + - id: metric.http.server.request.size + prefix: http + type: metric + extends: metric.http + name: http.server.request.size + brief: "Measures the size of HTTP request messages (compressed)." + instrument: histogram + units: By + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.scheme + requirement_level: required + - ref: http.flavor + requirement_level: recommended + - ref: net.host.name + requirement_level: required + - ref: net.host.port + requirement_level: + conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - id: metric.http.server + - id: metric.http.server.response.size prefix: http type: metric extends: metric.http - brief: "Semantic Convention for HTTP Server" - constraints: - - any_of: - - [http.scheme, http.host, http.target] - - [http.scheme, http.server_name, net.host.port, http.target] - - [http.scheme, net.host.name, net.host.port, http.target] - - [http.url] + name: http.server.response.size + brief: "Measures the size of HTTP response messages (compressed)." + instrument: histogram + units: By attributes: - - ref: http.server_name + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.scheme + requirement_level: required + - ref: http.flavor + requirement_level: recommended - ref: net.host.name + requirement_level: required - ref: net.host.port - metrics: - - id: server.duration - brief: "Measures the duration of the inbound HTTP request." - instrument: Histogram - units: ms - - id: server.active_requests - brief: "Measures the number of concurrent HTTP requests that are currently in-flight." - instrument: UpDownCounter - units: "{requests}" + requirement_level: + conditionally_required: "if not default (`80` for `http`, `443` for `https`)." + + - id: metric.http.server.active_requests + prefix: http + type: metric + extends: metric.http + name: http.server.request.size + brief: "Measures the number of concurrent HTTP requests that are currently in-flight." + instrument: updowncounter + units: "{requests}" + attributes: + - ref: http.method + requirement_level: required + - ref: http.scheme + requirement_level: required + - ref: http.flavor + requirement_level: recommended + - ref: net.host.name + requirement_level: required diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml deleted file mode 100644 index ce509024..00000000 --- a/semantic-conventions/src/tests/data/yaml/http_metrics_new.yaml +++ /dev/null @@ -1,170 +0,0 @@ -groups: - - id: metric.http - prefix: http - type: metric - brief: "This document defines semantic convention for HTTP client and server metrics." - note: > - These conventions can be used for http and https schemes - and various HTTP versions like 1.1, 2 and SPDY. - - - id: metric.http.client.duration - prefix: http - type: metric - extends: metric.http - name: http.client.duration - brief: "Measures the duration of the outbound HTTP request." - instrument: Histogram - units: ms - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.flavor - requirement_level: recommended - - ref: net.peer.name - requirement_level: required - - ref: net.peer.port - requirement_level: - conditionally_required: If not default (`80` for `http`, `443` for `https`). - - ref: net.sock.peer.addr - requirement_level: recommended - - - id: metric.http.client.request.size - prefix: http - type: metric - extends: metric.http - name: http.client.request.size - brief: "Measures the size of HTTP request messages (compressed)." - instrument: Histogram - units: By - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.flavor - requirement_level: recommended - - ref: net.peer.name - requirement_level: required - - ref: net.peer.port - requirement_level: - conditionally_required: If not default (`80` for `http`, `443` for `https`). - - ref: net.sock.peer.addr - requirement_level: recommended - - - id: metric.http.client.response.size - prefix: http - type: metric - extends: metric.http - name: http.client.response.size - brief: "Measures the size of HTTP response messages (compressed)." - instrument: Histogram - units: By - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.flavor - requirement_level: recommended - - ref: net.peer.name - requirement_level: required - - ref: net.peer.port - requirement_level: - conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - ref: net.sock.peer.addr - requirement_level: recommended - - - id: metric.http.server.duration - prefix: http - type: metric - extends: metric.http - name: http.server.duration - brief: "Measures the duration inbound HTTP requests." - instrument: Histogram - units: ms - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.scheme - requirement_level: required - - ref: http.flavor - requirement_level: recommended - - ref: net.host.name - requirement_level: required - - ref: net.host.port - requirement_level: - conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - - id: metric.http.server.request.size - prefix: http - type: metric - extends: metric.http - name: http.server.request.size - brief: "Measures the size of HTTP request messages (compressed)." - instrument: Histogram - units: By - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.scheme - requirement_level: required - - ref: http.flavor - requirement_level: recommended - - ref: net.host.name - requirement_level: required - - ref: net.host.port - requirement_level: - conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - - id: metric.http.server.request.size - prefix: http - type: metric - extends: metric.http - name: http.server.response.size - brief: "Measures the size of HTTP response messages (compressed)." - instrument: Histogram - units: By - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.scheme - requirement_level: required - - ref: http.flavor - requirement_level: recommended - - ref: net.host.name - requirement_level: required - - ref: net.host.port - requirement_level: - conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - - id: metric.http.server.active_requests - prefix: http - type: metric - extends: metric.http - name: http.server.request.size - brief: "Measures the number of concurrent HTTP requests that are currently in-flight." - instrument: UpDownCounter - units: "{requests}" - attributes: - - ref: http.method - requirement_level: required - - ref: http.scheme - requirement_level: required - - ref: http.flavor - requirement_level: recommended - - ref: net.host.name - requirement_level: required diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index 4028c28a..caeebad6 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -198,12 +198,12 @@ def test_http(self): def test_metrics_http(self): semconv = SemanticConventionSet(debug=False) semconv.parse(self.load_file("yaml/http_metrics.yaml")) - self.assertEqual(len(semconv.models), 3) + self.assertEqual(len(semconv.models), 8) semconv.parse(self.load_file("yaml/general.yaml")) semconv.parse(self.load_file("yaml/http.yaml")) metric_semconvs = cast( - List[MetricSemanticConvention], list(semconv.models.values())[:3] + List[MetricSemanticConvention], list(semconv.models.values())[:2] ) expected = { @@ -211,53 +211,28 @@ def test_metrics_http(self): "prefix": "http", "extends": "", "n_constraints": 0, - "attributes": [ - "http.method", - "http.host", - "http.scheme", - "http.status_code", - ], + "attributes": [], } self.semantic_convention_check(metric_semconvs[0], expected) expected = { - "id": "metric.http.client", + "id": "metric.http.client.duration", "prefix": "http", "extends": "metric.http", - "n_constraints": 1, - "attributes": ["net.peer.name", "net.peer.port", "net.peer.ip"], - "metrics": [ - { - "id": "client.duration", - "instrument": "Histogram", - "units": "ms", - } + "n_constraints": 0, + "name": "http.client.duration", + "units": "ms", + "instrument": "histogram", + "attributes": [ + "http.method", + "http.status_code", + "http.flavor", + "net.peer.name", + "net.peer.port", + "net.sock.peer.addr", ], } self.semantic_convention_check(metric_semconvs[1], expected) - self.metric_check(metric_semconvs[1].metrics, expected.get("metrics")) - - expected = { - "id": "metric.http.server", - "prefix": "http", - "extends": "metric.http", - "n_constraints": 1, - "attributes": ["http.server_name", "net.host.name", "net.host.port"], - "metrics": [ - { - "id": "server.duration", - "instrument": "Histogram", - "units": "ms", - }, - { - "id": "server.active_requests", - "instrument": "UpDownCounter", - "units": "{requests}", - }, - ], - } - self.semantic_convention_check(metric_semconvs[2], expected) - self.metric_check(metric_semconvs[2].metrics, expected.get("metrics")) def test_resource(self): semconv = SemanticConventionSet(debug=False) @@ -753,12 +728,6 @@ def test_scope_attribute(self): } self.semantic_convention_check(list(semconv.models.values())[0], expected) - def metric_check(self, metrics: Tuple[MetricSemanticConvention.Metric], expected): - for m, ex in zip(metrics, expected): - self.assertEqual(m.id, ex["id"]) - self.assertEqual(str(m.instrument), ex["instrument"]) - self.assertEqual(m.units, ex["units"]) - _TEST_DIR = os.path.dirname(__file__) def load_file(self, filename): From fd33c98a22ccb96741f4bb7eaf85b8ec57b94e30 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 9 Sep 2022 15:06:29 +1000 Subject: [PATCH 31/54] foo bar test for new metric structure --- .../semconv/model/semantic_convention.py | 19 +- .../data/markdown/metrics_tables/expected.md | 82 ++------- .../data/markdown/metrics_tables/input.md | 38 +--- .../src/tests/data/yaml/http_metrics.yaml | 170 ------------------ .../src/tests/data/yaml/metrics.yaml | 24 +++ .../tests/semconv/model/test_correct_parse.py | 25 ++- .../tests/semconv/templating/test_markdown.py | 2 +- 7 files changed, 66 insertions(+), 294 deletions(-) delete mode 100644 semantic-conventions/src/tests/data/yaml/http_metrics.yaml create mode 100644 semantic-conventions/src/tests/data/yaml/metrics.yaml diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 1f69208f..69ed673c 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -16,7 +16,7 @@ import typing from dataclasses import dataclass, field from enum import Enum -from typing import Tuple, Union, Dict +from typing import Dict, Tuple, Union from ruamel.yaml import YAML @@ -250,7 +250,7 @@ class MetricSemanticConvention(BaseSemanticConvention): allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ( "name", "units", - "instrument" + "instrument", ) yaml_to_markdown_instrument_repr: Dict[str, str] = { @@ -260,14 +260,18 @@ class MetricSemanticConvention(BaseSemanticConvention): "gauge": "Gauge", } - allowed_instruments: Tuple[str, ...] = tuple(yaml_to_markdown_instrument_repr.keys()) + (None,) + allowed_instruments: Tuple[str, ...] = tuple( + yaml_to_markdown_instrument_repr.keys() + ) + (None,) def __init__(self, group): super().__init__(group) self.name = group.get("name") self.units = group.get("units") self.instrument = group.get("instrument") - self.instrument_markdown_fmt = self.yaml_to_markdown_instrument_repr.get(self.instrument) + self.instrument_markdown_fmt = self.yaml_to_markdown_instrument_repr.get( + self.instrument + ) self.validate() def validate(self): @@ -275,15 +279,18 @@ def validate(self): if not all(val_tuple) and any(val_tuple): raise ValidationError.from_yaml_pos( self._position, - "Either all or none of name, units, and instrument must be defined" + "Either all or none of name, units, and instrument must be defined", ) if self.instrument not in self.allowed_instruments: raise ValidationError.from_yaml_pos( self._position, - "Instrument '{}' is not a valid instrument name".format(self.instrument) + "Instrument '{}' is not a valid instrument name".format( + self.instrument + ), ) + @dataclass class SemanticConventionSet: """Contains the list of models. diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md index fd9bc5de..d9d49a8d 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -1,76 +1,18 @@ +# Test Markdown -## Common Attributes - -The following attributes SHOULD be included on all HTTP metrics for both server and client. - - -| Attribute | Type | Description | Examples | Requirement Level | -|---|---|---|---|---| -| `http.host` | string | The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). An empty Host header should also be reported, see note. [1] | `www.example.org` | Conditionally Required: See attribute alternatives | -| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required | -| `http.scheme` | string | The URI scheme identifying the used protocol. | `http`; `https` | Conditionally Required: See attribute alternative | -| `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. | - -**[1]:** When the header is present but empty the attribute SHOULD be set to the empty string. Note that this is a valid situation that is expected in certain cases, according the aforementioned [section of RFC 7230](https://tools.ietf.org/html/rfc7230#section-5.4). When the header is not set the attribute MUST NOT be set. - - -## HTTP Client - -### HTTP Client Metrics - - -| Name | Instrument | Unit (UCUM) | Description | -| -------- | ------------- | ----------- | -------------- | -| `http.client.duration` | Histogram | `ms` | Measures the duration of the outbound HTTP request. | - - -### HTTP Client Attributes - -The following attributes SHOULD be included on HTTP Client metrics, where applicable and available. - - -| Attribute | Type | Description | Examples | Requirement Level | -|---|---|---|---|---| -| `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 | -| `net.peer.port` | int | Remote port number. | `80`; `8080`; `443` | See below | - -**Additional attribute requirements:** At least one of the following sets of attributes is required: - -* `http.url` -* `http.scheme`, `http.host`, `http.target` -* `http.scheme`, `net.peer.name`, `net.peer.port`, `http.target` -* `http.scheme`, `net.peer.ip`, `net.peer.port`, `http.target` - - -## HTTP Server - -### HTTP Server Metrics - - -| Name | Instrument | Unit (UCUM) | Description | -| -------- | ------------- | ----------- | -------------- | -| `http.server.duration` | Histogram | `ms` | Measures the duration of the inbound HTTP request. | -| `http.server.active_requests` | UpDownCounter | `{requests}` | Measures the number of concurrent HTTP requests that are currently in-flight. | +**`foo.size`** + +| Name | Instrument Type | Unit (UCUM) | Description | +| -------- | --------------- | ----------- | -------------- | +| `foo.size` | Histogram | `{bars}` | Measures the size of foo. | - -### HTTP Server Attributes - -The following attributes SHOULD be included on HTTP Server metrics, where applicable and available. - - +**Attributes for `foo.size`** + | Attribute | Type | Description | Examples | Requirement Level | |---|---|---|---|---| -| `http.server_name` | string | The primary server name of the matched virtual host. This should be obtained via configuration. If no such configuration can be obtained, this attribute MUST NOT be set ( `net.host.name` should be used instead). [1] | `example.com` | See below | -| `net.host.name` | string | Local hostname or similar, see note below. | `localhost` | See below | -| `net.host.port` | int | Like `net.peer.port` but for the host port. | `35555` | See below | - -**[1]:** `http.url` is usually not readily available on the server side but would have to be assembled in a cumbersome and sometimes lossy process from other information (see e.g. open-telemetry/opentelemetry-python/pull/148). It is thus preferred to supply the raw data that is available. - -**Additional attribute requirements:** At least one of the following sets of attributes is required: +| `http.flavor` | string | Kind of HTTP protocol used. [1] | `1.0` | Recommended | +| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required | +| `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.scheme`, `http.host`, `http.target` -* `http.scheme`, `http.server_name`, `net.host.port`, `http.target` -* `http.scheme`, `net.host.name`, `net.host.port`, `http.target` -* `http.url` +**[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. diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md index e03f49e0..92bba763 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md @@ -1,36 +1,8 @@ +# Test Markdown -## Common Attributes - -The following attributes SHOULD be included on all HTTP metrics for both server and client. - - - - -## HTTP Client - -### HTTP Client Metrics - - - - -### HTTP Client Attributes - -The following attributes SHOULD be included on HTTP Client metrics, where applicable and available. - - - - -## HTTP Server - -### HTTP Server Metrics - - +**`foo.size`** + - -### HTTP Server Attributes - -The following attributes SHOULD be included on HTTP Server metrics, where applicable and available. - - - +**Attributes for `foo.size`** + diff --git a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml b/semantic-conventions/src/tests/data/yaml/http_metrics.yaml deleted file mode 100644 index 14f19601..00000000 --- a/semantic-conventions/src/tests/data/yaml/http_metrics.yaml +++ /dev/null @@ -1,170 +0,0 @@ -groups: - - id: metric.http - prefix: http - type: metric - brief: "This document defines semantic convention for HTTP client and server metrics." - note: > - These conventions can be used for http and https schemes - and various HTTP versions like 1.1, 2 and SPDY. - - - id: metric.http.client.duration - prefix: http - type: metric - extends: metric.http - name: http.client.duration - brief: "Measures the duration of the outbound HTTP request." - instrument: histogram - units: ms - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.flavor - requirement_level: recommended - - ref: net.peer.name - requirement_level: required - - ref: net.peer.port - requirement_level: - conditionally_required: If not default (`80` for `http`, `443` for `https`). - - ref: net.sock.peer.addr - requirement_level: recommended - - - id: metric.http.client.request.size - prefix: http - type: metric - extends: metric.http - name: http.client.request.size - brief: "Measures the size of HTTP request messages (compressed)." - instrument: histogram - units: By - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.flavor - requirement_level: recommended - - ref: net.peer.name - requirement_level: required - - ref: net.peer.port - requirement_level: - conditionally_required: If not default (`80` for `http`, `443` for `https`). - - ref: net.sock.peer.addr - requirement_level: recommended - - - id: metric.http.client.response.size - prefix: http - type: metric - extends: metric.http - name: http.client.response.size - brief: "Measures the size of HTTP response messages (compressed)." - instrument: histogram - units: By - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.flavor - requirement_level: recommended - - ref: net.peer.name - requirement_level: required - - ref: net.peer.port - requirement_level: - conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - ref: net.sock.peer.addr - requirement_level: recommended - - - id: metric.http.server.duration - prefix: http - type: metric - extends: metric.http - name: http.server.duration - brief: "Measures the duration inbound HTTP requests." - instrument: histogram - units: ms - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.scheme - requirement_level: required - - ref: http.flavor - requirement_level: recommended - - ref: net.host.name - requirement_level: required - - ref: net.host.port - requirement_level: - conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - - id: metric.http.server.request.size - prefix: http - type: metric - extends: metric.http - name: http.server.request.size - brief: "Measures the size of HTTP request messages (compressed)." - instrument: histogram - units: By - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.scheme - requirement_level: required - - ref: http.flavor - requirement_level: recommended - - ref: net.host.name - requirement_level: required - - ref: net.host.port - requirement_level: - conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - - id: metric.http.server.response.size - prefix: http - type: metric - extends: metric.http - name: http.server.response.size - brief: "Measures the size of HTTP response messages (compressed)." - instrument: histogram - units: By - attributes: - - ref: http.method - requirement_level: required - - ref: http.status_code - requirement_level: - conditionally_required: "if and only if one was received/sent." - - ref: http.scheme - requirement_level: required - - ref: http.flavor - requirement_level: recommended - - ref: net.host.name - requirement_level: required - - ref: net.host.port - requirement_level: - conditionally_required: "if not default (`80` for `http`, `443` for `https`)." - - - id: metric.http.server.active_requests - prefix: http - type: metric - extends: metric.http - name: http.server.request.size - brief: "Measures the number of concurrent HTTP requests that are currently in-flight." - instrument: updowncounter - units: "{requests}" - attributes: - - ref: http.method - requirement_level: required - - ref: http.scheme - requirement_level: required - - ref: http.flavor - requirement_level: recommended - - ref: net.host.name - requirement_level: required diff --git a/semantic-conventions/src/tests/data/yaml/metrics.yaml b/semantic-conventions/src/tests/data/yaml/metrics.yaml new file mode 100644 index 00000000..466fe361 --- /dev/null +++ b/semantic-conventions/src/tests/data/yaml/metrics.yaml @@ -0,0 +1,24 @@ +groups: + - id: metric.foo + prefix: foo + type: metric + brief: "This document defines foo." + note: > + Details about foo. + + - id: metric.foo.size + prefix: foo + type: metric + extends: metric.foo + name: foo.size + brief: "Measures the size of foo." + instrument: histogram + units: "{bars}" + attributes: + - ref: http.method + requirement_level: required + - ref: http.status_code + requirement_level: + conditionally_required: "if and only if one was received/sent." + - ref: http.flavor + requirement_level: recommended diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index caeebad6..b74af2a3 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -14,7 +14,7 @@ import os import unittest -from typing import List, Tuple, cast +from typing import List, cast from opentelemetry.semconv.model.constraints import AnyOf, Include from opentelemetry.semconv.model.semantic_attribute import StabilityLevel @@ -195,10 +195,10 @@ def test_http(self): } self.semantic_convention_check(list(semconv.models.values())[2], expected) - def test_metrics_http(self): + def test_metrics(self): semconv = SemanticConventionSet(debug=False) - semconv.parse(self.load_file("yaml/http_metrics.yaml")) - self.assertEqual(len(semconv.models), 8) + semconv.parse(self.load_file("yaml/metrics.yaml")) + self.assertEqual(len(semconv.models), 2) semconv.parse(self.load_file("yaml/general.yaml")) semconv.parse(self.load_file("yaml/http.yaml")) @@ -207,8 +207,8 @@ def test_metrics_http(self): ) expected = { - "id": "metric.http", - "prefix": "http", + "id": "metric.foo", + "prefix": "foo", "extends": "", "n_constraints": 0, "attributes": [], @@ -216,20 +216,17 @@ def test_metrics_http(self): self.semantic_convention_check(metric_semconvs[0], expected) expected = { - "id": "metric.http.client.duration", - "prefix": "http", - "extends": "metric.http", + "id": "metric.foo.size", + "prefix": "foo", + "extends": "metric.foo", "n_constraints": 0, - "name": "http.client.duration", - "units": "ms", + "name": "foo.size", + "units": "{bars}", "instrument": "histogram", "attributes": [ "http.method", "http.status_code", "http.flavor", - "net.peer.name", - "net.peer.port", - "net.sock.peer.addr", ], } self.semantic_convention_check(metric_semconvs[1], expected) diff --git a/semantic-conventions/src/tests/semconv/templating/test_markdown.py b/semantic-conventions/src/tests/semconv/templating/test_markdown.py index 66d65eb8..377bbec1 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_markdown.py +++ b/semantic-conventions/src/tests/semconv/templating/test_markdown.py @@ -126,7 +126,7 @@ def test_event_renamed(self): def test_metric_tables(self): self.check( "markdown/metrics_tables", - extra_yaml_files=["yaml/general.yaml", "yaml/http_metrics.yaml"], + extra_yaml_files=["yaml/general.yaml", "yaml/metrics.yaml"], ) def testSamplingRelevant(self): From 6536fb8d1a00f1090fe77763bf65dcccd8d9902c Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 9 Sep 2022 15:13:08 +1000 Subject: [PATCH 32/54] alter syntax definition in readme --- .../semconv/model/semantic_convention.py | 4 ++-- .../src/tests/data/yaml/metrics.yaml | 2 +- semantic-conventions/syntax.md | 20 ++++++++----------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 69ed673c..5a11cef2 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -248,7 +248,7 @@ class MetricSemanticConvention(BaseSemanticConvention): GROUP_TYPE_NAME = "metric" allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ( - "name", + "metric_name", "units", "instrument", ) @@ -266,7 +266,7 @@ class MetricSemanticConvention(BaseSemanticConvention): def __init__(self, group): super().__init__(group) - self.name = group.get("name") + self.name = group.get("metric_name") self.units = group.get("units") self.instrument = group.get("instrument") self.instrument_markdown_fmt = self.yaml_to_markdown_instrument_repr.get( diff --git a/semantic-conventions/src/tests/data/yaml/metrics.yaml b/semantic-conventions/src/tests/data/yaml/metrics.yaml index 466fe361..ea667291 100644 --- a/semantic-conventions/src/tests/data/yaml/metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/metrics.yaml @@ -10,7 +10,7 @@ groups: prefix: foo type: metric extends: metric.foo - name: foo.size + metric_name: foo.size brief: "Measures the size of foo." instrument: histogram units: "{bars}" diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index c716aad4..b7321628 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -40,14 +40,14 @@ All attributes are lower case. groups ::= semconv | semconv groups -semconv ::= id [convtype] brief [note] [prefix] [extends] [stability] [deprecated] attributes [constraints] [specificfields] [metrics] +semconv ::= id [convtype] brief [note] [prefix] [extends] [stability] [deprecated] attributes [constraints] [specificfields] [instrument] [metric_name] [units] id ::= string convtype ::= "span" # Default if not specified | "resource" # see spanspecificfields | "event" # see eventspecificfields - | "metric" # (currently non-functional) + | "metric" | "scope" brief ::= string @@ -119,18 +119,14 @@ span_kind ::= "client" events ::= id {id} # MUST point to an existing event group -name ::= string +metric_name ::= string -instrument ::= "Counter" - | "Histogram" - | "Gauge" - | "UpDownCounter" +instrument ::= "counter" + | "histogram" + | "gauge" + | "updowncounter" -units ::= string - -metric ::= id instrument units brief - -metrics ::= {metric} +units ::= string ``` ## Semantics From 9e7e7186bae9e5eedf99f03c46a0e933c67355f6 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 9 Sep 2022 15:15:29 +1000 Subject: [PATCH 33/54] simplify test slightly --- .../src/tests/data/markdown/metrics_tables/expected.md | 3 --- semantic-conventions/src/tests/data/yaml/metrics.yaml | 2 -- .../src/tests/semconv/model/test_correct_parse.py | 1 - 3 files changed, 6 deletions(-) diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md index d9d49a8d..cc09ad51 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -10,9 +10,6 @@ | Attribute | Type | Description | Examples | Requirement Level | |---|---|---|---|---| -| `http.flavor` | string | Kind of HTTP protocol used. [1] | `1.0` | Recommended | | `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required | | `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. | - -**[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. diff --git a/semantic-conventions/src/tests/data/yaml/metrics.yaml b/semantic-conventions/src/tests/data/yaml/metrics.yaml index ea667291..83c519a7 100644 --- a/semantic-conventions/src/tests/data/yaml/metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/metrics.yaml @@ -20,5 +20,3 @@ groups: - ref: http.status_code requirement_level: conditionally_required: "if and only if one was received/sent." - - ref: http.flavor - requirement_level: recommended diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index b74af2a3..fe8c8aa3 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -226,7 +226,6 @@ def test_metrics(self): "attributes": [ "http.method", "http.status_code", - "http.flavor", ], } self.semantic_convention_check(metric_semconvs[1], expected) From f6f1d5a323e8ad6b6d98257377d90c63b7f98e20 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 13 Sep 2022 11:21:48 +1000 Subject: [PATCH 34/54] implement several review items --- .../semconv/model/semantic_convention.py | 8 ++++---- .../semconv/templating/markdown/__init__.py | 4 ++-- .../src/tests/data/yaml/metrics.yaml | 2 +- .../src/tests/semconv/model/test_correct_parse.py | 7 +++++-- semantic-conventions/syntax.md | 15 ++++++--------- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 5a11cef2..2452660c 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -249,7 +249,7 @@ class MetricSemanticConvention(BaseSemanticConvention): allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ( "metric_name", - "units", + "unit", "instrument", ) @@ -266,8 +266,8 @@ class MetricSemanticConvention(BaseSemanticConvention): def __init__(self, group): super().__init__(group) - self.name = group.get("metric_name") - self.units = group.get("units") + self.metric_name = group.get("metric_name") + self.unit = group.get("unit") self.instrument = group.get("instrument") self.instrument_markdown_fmt = self.yaml_to_markdown_instrument_repr.get( self.instrument @@ -275,7 +275,7 @@ def __init__(self, group): self.validate() def validate(self): - val_tuple = (self.name, self.units, self.instrument) + val_tuple = (self.metric_name, self.unit, self.instrument) if not all(val_tuple) and any(val_tuple): raise ValidationError.from_yaml_pos( self._position, diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 3ac0cda3..39423c7f 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -237,9 +237,9 @@ def to_markdown_metric_table( ) output.write( "| `{}` | {} | `{}` | {} |\n".format( - semconv.name, + semconv.metric_name, semconv.instrument_markdown_fmt, - semconv.units, + semconv.unit, semconv.brief, ) ) diff --git a/semantic-conventions/src/tests/data/yaml/metrics.yaml b/semantic-conventions/src/tests/data/yaml/metrics.yaml index 83c519a7..aec383d8 100644 --- a/semantic-conventions/src/tests/data/yaml/metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/metrics.yaml @@ -13,7 +13,7 @@ groups: metric_name: foo.size brief: "Measures the size of foo." instrument: histogram - units: "{bars}" + unit: "{bars}" attributes: - ref: http.method requirement_level: required diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index fe8c8aa3..b58b2358 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -220,8 +220,8 @@ def test_metrics(self): "prefix": "foo", "extends": "metric.foo", "n_constraints": 0, - "name": "foo.size", - "units": "{bars}", + "metric_name": "foo.size", + "unit": "{bars}", "instrument": "histogram", "attributes": [ "http.method", @@ -229,6 +229,9 @@ def test_metrics(self): ], } self.semantic_convention_check(metric_semconvs[1], expected) + self.assertEqual(metric_semconvs[1].unit, expected["unit"]) + self.assertEqual(metric_semconvs[1].instrument, expected["instrument"]) + self.assertEqual(metric_semconvs[1].metric_name, expected["metric_name"]) def test_resource(self): semconv = SemanticConventionSet(debug=False) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index b7321628..767f57bf 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -178,15 +178,12 @@ The following is only valid if `type` is `event`: The following is only valid if `type` is `metric`: -- `metrics`, an optional list of metrics that belong to the semantic convention. - Each individual metric has the following semantics: - - `id`, the ID of the metric. The fully qualified name of the metric includes its parent - semantic convention prefix like so: `{parent.prefix}.{metric.id}`. - - `brief`, a brief description of the metric. - - `instrument`, the [instrument type]( https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics/semantic_conventions#instrument-types) - that *should* be used to record the metric. - - `units`, the units in which the metric is measured, which should adhere to - [UCUM](https://ucum.org/ucum.html). + - `metric_name`, the metric name as described by the [OpenTelemetry Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#timeseries-model). + - `instrument`, the [instrument type]( https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument) + that *should* be used to record the metric. These include `counter`, `gauge`, `updowncounter`, and `histogram`. Whether the instrument + is synchronous or asynchronous is an implementation detail and should not be specified. + - `unit`, the units in which the metric is measured, which should adhere to + [the guidelines](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics/semantic_conventions#instrument-units). ### Attributes From 3f3a104e2d837175be5ef2cdfc223a395e5704be Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 13 Sep 2022 11:32:09 +1000 Subject: [PATCH 35/54] units -> unit in some places I missed before --- semantic-conventions/syntax.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index 767f57bf..cf7c3bd3 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -40,7 +40,7 @@ All attributes are lower case. groups ::= semconv | semconv groups -semconv ::= id [convtype] brief [note] [prefix] [extends] [stability] [deprecated] attributes [constraints] [specificfields] [instrument] [metric_name] [units] +semconv ::= id [convtype] brief [note] [prefix] [extends] [stability] [deprecated] attributes [constraints] [specificfields] [instrument] [metric_name] [unit] id ::= string @@ -126,7 +126,7 @@ instrument ::= "counter" | "gauge" | "updowncounter" -units ::= string +unit ::= string ``` ## Semantics @@ -182,7 +182,7 @@ The following is only valid if `type` is `metric`: - `instrument`, the [instrument type]( https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument) that *should* be used to record the metric. These include `counter`, `gauge`, `updowncounter`, and `histogram`. Whether the instrument is synchronous or asynchronous is an implementation detail and should not be specified. - - `unit`, the units in which the metric is measured, which should adhere to + - `unit`, the unit in which the metric is measured, which should adhere to [the guidelines](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics/semantic_conventions#instrument-units). ### Attributes From f2f64b69b83a30eff6f632af40fbfdb9267c4424 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 13 Sep 2022 11:36:50 +1000 Subject: [PATCH 36/54] newline for readability --- .../src/tests/data/markdown/metrics_tables/expected.md | 1 + .../src/tests/data/markdown/metrics_tables/input.md | 1 + 2 files changed, 2 insertions(+) diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md index cc09ad51..2ea7a544 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -6,6 +6,7 @@ | -------- | --------------- | ----------- | -------------- | | `foo.size` | Histogram | `{bars}` | Measures the size of foo. | + **Attributes for `foo.size`** | Attribute | Type | Description | Examples | Requirement Level | diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md index 92bba763..dec7cc98 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md @@ -3,6 +3,7 @@ **`foo.size`** + **Attributes for `foo.size`** From 766cc5d2a7a02ab1b73035ac3e46fbd0c6ffd518 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 13 Sep 2022 11:42:30 +1000 Subject: [PATCH 37/54] remove unneeded yaml definition --- .../data/markdown/metrics_tables/http.yaml | 172 ------------------ .../tests/semconv/templating/test_markdown.py | 2 +- 2 files changed, 1 insertion(+), 173 deletions(-) delete mode 100644 semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml b/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml deleted file mode 100644 index ffa3f133..00000000 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/http.yaml +++ /dev/null @@ -1,172 +0,0 @@ -groups: - - id: http - prefix: http - brief: 'This document defines semantic conventions for HTTP client and server Spans.' - note: > - These conventions can be used for http and https schemes - and various HTTP versions like 1.1, 2 and SPDY. - attributes: - - id: method - type: string - requirement_level: required - brief: 'HTTP request method.' - sampling_relevant: true - examples: ["GET", "POST", "HEAD"] - - id: url - type: string - brief: > - 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. - note: > - `http.url` MUST NOT contain credentials passed via URL in form of `https://username:password@www.example.com/`. - In such case the attribute's value should be `https://www.example.com/`. - sampling_relevant: true - examples: ['https://www.foo.bar/search?q=OpenTelemetry#SemConv'] - - id: target - type: string - brief: 'The full request target as passed in a HTTP request line or equivalent.' - sampling_relevant: true - examples: ['/path/12314/?q=ddds#123'] - - id: host - type: string - brief: > - The value of the [HTTP host header](https://tools.ietf.org/html/rfc7230#section-5.4). - An empty Host header should also be reported, see note. - note: > - When the header is present but empty the attribute SHOULD be set to - the empty string. Note that this is a valid situation that is expected - in certain cases, according the aforementioned - [section of RFC 7230](https://tools.ietf.org/html/rfc7230#section-5.4). - When the header is not set the attribute MUST NOT be set. - sampling_relevant: true - examples: ['www.example.org'] - - id: scheme - type: string - brief: 'The URI scheme identifying the used protocol.' - sampling_relevant: true - examples: ["http", "https"] - - id: status_code - type: int - requirement_level: - conditionally_required: If and only if one was received/sent. - brief: '[HTTP response status code](https://tools.ietf.org/html/rfc7231#section-6).' - examples: [200] - - id: flavor - type: - # Default value: `true`. If false, it helps the code gen tool to - # encode checks that only accept the listed values. - allow_custom_values: true - members: - - id: http_1_0 - value: '1.0' - brief: 'HTTP 1.0' - - id: http_1_1 - value: '1.1' - brief: 'HTTP 1.1' - - id: http_2_0 - value: '2.0' - brief: 'HTTP 2' - - id: spdy - value: 'SPDY' - brief: 'SPDY protocol.' - - id: quic - value: 'QUIC' - brief: 'QUIC protocol.' - brief: 'Kind of HTTP protocol used.' - note: > - 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. - - id: user_agent - type: string - brief: 'Value of the [HTTP User-Agent](https://tools.ietf.org/html/rfc7231#section-5.5.3) header sent by the client.' - examples: ['CERN-LineMode/2.15 libwww/2.17b3'] - - id: request_content_length - type: int - brief: > - The size of the request payload body in bytes. This is the number of bytes transferred excluding headers and - is often, but not always, present as the [Content-Length](https://tools.ietf.org/html/rfc7230#section-3.3.2) - header. For requests using transport encoding, this should be the compressed size. - examples: 3495 - - id: request_content_length_uncompressed - type: int - brief: > - The size of the uncompressed request payload body after transport decoding. Not set if transport encoding not used. - examples: 5493 - - id: response_content_length - type: int - brief: > - The size of the response payload body in bytes. This is the number of bytes transferred excluding headers and - is often, but not always, present as the [Content-Length](https://tools.ietf.org/html/rfc7230#section-3.3.2) - header. For requests using transport encoding, this should be the compressed size. - examples: 3495 - - id: response_content_length_uncompressed - type: int - brief: > - The size of the uncompressed response payload body after transport decoding. Not set if transport encoding not used. - examples: 5493 - - ref: net.peer.name - sampling_relevant: true - - ref: net.peer.ip - sampling_relevant: true - - ref: net.peer.port - sampling_relevant: true - constraints: - - include: network - - - id: http.client - prefix: http - extends: http - span_kind: client - brief: 'Semantic Convention for HTTP Client' - constraints: - - any_of: - - [http.url] - - [http.scheme, http.host, http.target] - - [http.scheme, net.peer.name, net.peer.port, http.target] - - [http.scheme, net.peer.ip, net.peer.port, http.target] - - - id: http.server - prefix: http - extends: http - span_kind: server - brief: 'Semantic Convention for HTTP Server' - attributes: - - id: server_name - type: string - brief: > - The primary server name of the matched virtual host. This should be obtained via configuration. - If no such configuration can be obtained, this attribute MUST NOT be set ( `net.host.name` should be used instead). - note: > - `http.url` is usually not readily available on the server side but would have to be assembled in a cumbersome and - sometimes lossy process from other information (see e.g. open-telemetry/opentelemetry-python/pull/148). - It is thus preferred to supply the raw data that is available. - examples: ['example.com'] - - id: route - type: string - brief: > - The matched route (path template). - examples: '/users/:userID?' - - id: client_ip - type: string - brief: > - The IP address of the original client behind all proxies, if - known (e.g. from [X-Forwarded-For](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For)). - note: | - This is not necessarily the same as `net.peer.ip`, which would - identify the network-level peer, which may be a proxy. - - This attribute should be set when a source of information different - from the one used for `net.peer.ip`, is available even if that other - source just confirms the same value as `net.peer.ip`. - Rationale: For `net.peer.ip`, one typically does not know if it - comes from a proxy, reverse proxy, or the actual client. Setting - `http.client_ip` when it's the same as `net.peer.ip` means that - one is at least somewhat confident that the address is not that of - the closest proxy. - examples: '83.164.160.102' - constraints: - - any_of: - - [http.scheme, http.host, http.target] - - [http.scheme, http.server_name, net.host.port, http.target] - - [http.scheme, net.host.name, net.host.port, http.target] - - [http.url] diff --git a/semantic-conventions/src/tests/semconv/templating/test_markdown.py b/semantic-conventions/src/tests/semconv/templating/test_markdown.py index 377bbec1..9d2222b4 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_markdown.py +++ b/semantic-conventions/src/tests/semconv/templating/test_markdown.py @@ -126,7 +126,7 @@ def test_event_renamed(self): def test_metric_tables(self): self.check( "markdown/metrics_tables", - extra_yaml_files=["yaml/general.yaml", "yaml/metrics.yaml"], + extra_yaml_files=["yaml/general.yaml", "yaml/http.yaml", "yaml/metrics.yaml"], ) def testSamplingRelevant(self): From f1f7e73ddcda0215f1d344cf72b3f80867a7e2e4 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 13 Sep 2022 11:55:25 +1000 Subject: [PATCH 38/54] add JSON schema definition for MetricSemanticConvention --- semantic-conventions/semconv.schema.json | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/semantic-conventions/semconv.schema.json b/semantic-conventions/semconv.schema.json index 9c9dd991..7ffad0d3 100644 --- a/semantic-conventions/semconv.schema.json +++ b/semantic-conventions/semconv.schema.json @@ -16,6 +16,9 @@ }, { "allOf": [{"$ref": "#/definitions/EventSemanticConvention"}] + }, + { + "allOf": [{"$ref": "#/definitions/MetricSemanticConvention"}] } ] } @@ -164,6 +167,33 @@ {"required": ["name"]} ] }, + "MetricSemanticConvention": { + "allOf": [{ "$ref": "#/definitions/SemanticConventionBase" }], + "properties": { + "instrument": { + "type": "string", + "description": "The instrument used to record the metric.", + "enum": [ + "counter", + "gauge", + "histogram", + "updowncounter" + ] + }, + "metric_name": { + "type": "string", + "description": "The name of the metric." + }, + "type": { + "type": "string", + "const": "metric" + }, + "unit": { + "type": "string", + "description": "The unit in which the metric is measured in." + } + } + }, "SemanticConvention": { "allOf": [{ "$ref": "#/definitions/SemanticConventionBase" }], "required": ["type"], From 167ba006a978d1bfcdae57fe8d98625697b8e7fe Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 13 Sep 2022 11:59:22 +1000 Subject: [PATCH 39/54] add changelog --- semantic-conventions/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/semantic-conventions/CHANGELOG.md b/semantic-conventions/CHANGELOG.md index b110419b..1fcfd370 100644 --- a/semantic-conventions/CHANGELOG.md +++ b/semantic-conventions/CHANGELOG.md @@ -4,7 +4,8 @@ Please update the changelog as part of any significant pull request. ## Unreleased -- ... +- Add definitions for metrics in YAML semconv, plus generation to markdown + ([#79](https://github.com/open-telemetry/build-tools/pull/79)) ## v0.14.0 From 5c380936d85c63a0609702d58fe4ae01ee9f974f Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 13 Sep 2022 12:00:25 +1000 Subject: [PATCH 40/54] appease linter --- .../src/tests/semconv/templating/test_markdown.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/semantic-conventions/src/tests/semconv/templating/test_markdown.py b/semantic-conventions/src/tests/semconv/templating/test_markdown.py index 9d2222b4..e96fcbc4 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_markdown.py +++ b/semantic-conventions/src/tests/semconv/templating/test_markdown.py @@ -126,7 +126,11 @@ def test_event_renamed(self): def test_metric_tables(self): self.check( "markdown/metrics_tables", - extra_yaml_files=["yaml/general.yaml", "yaml/http.yaml", "yaml/metrics.yaml"], + extra_yaml_files=[ + "yaml/general.yaml", + "yaml/http.yaml", + "yaml/metrics.yaml", + ], ) def testSamplingRelevant(self): From 8fb5af0eb71618c5eed02828dec57efd32266dee Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 16 Sep 2022 10:43:56 +1000 Subject: [PATCH 41/54] address comment regarding instrument types --- semantic-conventions/syntax.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index cf7c3bd3..cc6ff080 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -180,8 +180,9 @@ The following is only valid if `type` is `metric`: - `metric_name`, the metric name as described by the [OpenTelemetry Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#timeseries-model). - `instrument`, the [instrument type]( https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument) - that *should* be used to record the metric. These include `counter`, `gauge`, `updowncounter`, and `histogram`. Whether the instrument - is synchronous or asynchronous is an implementation detail and should not be specified. + that should be used to record the metric. Note that the semantic conventions must be written + using the names of the synchronous instrument types (`counter`, `gauge`, `updowncounter` and `histogram`). + For more details: [Metrics semantic conventions - Instrument types](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics/semantic_conventions#instrument-types). - `unit`, the unit in which the metric is measured, which should adhere to [the guidelines](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics/semantic_conventions#instrument-units). From 3a775d34dc2a07c3eaa8436864aed18e7313348a Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 6 Oct 2022 09:39:05 +1100 Subject: [PATCH 42/54] update docs on metric_table semconv generator example --- semantic-conventions/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/README.md b/semantic-conventions/README.md index 876e8a76..96109de0 100644 --- a/semantic-conventions/README.md +++ b/semantic-conventions/README.md @@ -83,8 +83,8 @@ convention that have the tag `network`. `` will print the constraints and attributes of both `http` and `http.server` semantic conventions that have the tag `network`. -`` will print a table of metrics with all metrics prefixed with -`metric.http.server`. +`` will print a table describing a single metric +`http.server.active_requests`. ## Code Generator From d237bf2283fe41d0c8c0ec4ac6f00af5b44abee9 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 20 Oct 2022 09:12:46 +1100 Subject: [PATCH 43/54] restore event name in syntax.md which erroneously removed --- semantic-conventions/syntax.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index cc6ff080..94492adf 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -119,6 +119,8 @@ span_kind ::= "client" events ::= id {id} # MUST point to an existing event group +name ::= string + metric_name ::= string instrument ::= "counter" From e8259dd2592dc6caaef99d7dfc1a22120cc320d2 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 20 Oct 2022 10:24:35 +1100 Subject: [PATCH 44/54] add more metrics/attributes to test, clarifying intentions --- .../data/markdown/metrics_tables/expected.md | 15 ++++++++++++++ .../data/markdown/metrics_tables/input.md | 8 ++++++++ .../src/tests/data/yaml/metrics.yaml | 20 +++++++++++++++++++ .../tests/semconv/model/test_correct_parse.py | 4 ++-- 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md index 2ea7a544..e23262d9 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -14,3 +14,18 @@ | `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Required | | `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. | + +**`foo.active_eggs`** + +| Name | Instrument Type | Unit (UCUM) | Description | +| -------- | --------------- | ----------- | -------------- | +| `foo.active_eggs` | UpDownCounter | `{cartons}` | Measures how many eggs are currently active. | + + +**Attributes for `foo.active_eggs`** + +| Attribute | Type | Description | Examples | Requirement Level | +|---|---|---|---|---| +| `foo.egg.type` | string | Type of egg. | `chicken`; `emu`; `dragon` | Conditionally Required: if available to instrumentation. | +| `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Optional | + diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md index dec7cc98..55e98373 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/input.md @@ -7,3 +7,11 @@ **Attributes for `foo.size`** + +**`foo.active_eggs`** + + + +**Attributes for `foo.active_eggs`** + + diff --git a/semantic-conventions/src/tests/data/yaml/metrics.yaml b/semantic-conventions/src/tests/data/yaml/metrics.yaml index aec383d8..6095f80f 100644 --- a/semantic-conventions/src/tests/data/yaml/metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/metrics.yaml @@ -5,6 +5,11 @@ groups: brief: "This document defines foo." note: > Details about foo. + attributes: + - id: egg.type + type: string + brief: 'Type of egg.' + examples: ["chicken", "emu", "dragon"] - id: metric.foo.size prefix: foo @@ -20,3 +25,18 @@ groups: - ref: http.status_code requirement_level: conditionally_required: "if and only if one was received/sent." + + - id: metric.foo.active_eggs + prefix: foo + type: metric + extends: metric.foo + metric_name: foo.active_eggs + brief: "Measures how many eggs are currently active." + instrument: updowncounter + unit: "{cartons}" + attributes: + - ref: http.method + requirement_level: optional + - ref: foo.egg.type + requirement_level: + conditionally_required: "if available to instrumentation." diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index b58b2358..02bafffa 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -198,7 +198,7 @@ def test_http(self): def test_metrics(self): semconv = SemanticConventionSet(debug=False) semconv.parse(self.load_file("yaml/metrics.yaml")) - self.assertEqual(len(semconv.models), 2) + self.assertEqual(len(semconv.models), 3) semconv.parse(self.load_file("yaml/general.yaml")) semconv.parse(self.load_file("yaml/http.yaml")) @@ -211,7 +211,7 @@ def test_metrics(self): "prefix": "foo", "extends": "", "n_constraints": 0, - "attributes": [], + "attributes": ["foo.egg.type"], } self.semantic_convention_check(metric_semconvs[0], expected) From 6a67c74f478f2dc70c07ae2c62183e78110c1e1e Mon Sep 17 00:00:00 2001 From: James Moessis Date: Mon, 24 Oct 2022 11:22:19 +1100 Subject: [PATCH 45/54] use different prefix in tests for clarity --- .../src/tests/data/markdown/metrics_tables/expected.md | 2 +- semantic-conventions/src/tests/data/yaml/metrics.yaml | 4 ++-- .../src/tests/semconv/model/test_correct_parse.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md index e23262d9..3b2b3228 100644 --- a/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md +++ b/semantic-conventions/src/tests/data/markdown/metrics_tables/expected.md @@ -26,6 +26,6 @@ | Attribute | Type | Description | Examples | Requirement Level | |---|---|---|---|---| -| `foo.egg.type` | string | Type of egg. | `chicken`; `emu`; `dragon` | Conditionally Required: if available to instrumentation. | +| `bar.egg.type` | string | Type of egg. | `chicken`; `emu`; `dragon` | Conditionally Required: if available to instrumentation. | | `http.method` | string | HTTP request method. | `GET`; `POST`; `HEAD` | Optional | diff --git a/semantic-conventions/src/tests/data/yaml/metrics.yaml b/semantic-conventions/src/tests/data/yaml/metrics.yaml index 6095f80f..eaeb32a5 100644 --- a/semantic-conventions/src/tests/data/yaml/metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/metrics.yaml @@ -1,6 +1,6 @@ groups: - id: metric.foo - prefix: foo + prefix: bar type: metric brief: "This document defines foo." note: > @@ -37,6 +37,6 @@ groups: attributes: - ref: http.method requirement_level: optional - - ref: foo.egg.type + - ref: bar.egg.type requirement_level: conditionally_required: "if available to instrumentation." diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index 02bafffa..69fb8981 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -208,10 +208,10 @@ def test_metrics(self): expected = { "id": "metric.foo", - "prefix": "foo", + "prefix": "bar", "extends": "", "n_constraints": 0, - "attributes": ["foo.egg.type"], + "attributes": ["bar.egg.type"], } self.semantic_convention_check(metric_semconvs[0], expected) From e524ab454f344692efe22d90d9beb78338a1edd6 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Mon, 24 Oct 2022 11:49:56 +1100 Subject: [PATCH 46/54] differentiate parent and concrete metric types with different semantic convention types metric and metric_group --- .../semconv/model/semantic_convention.py | 13 +++++++++---- .../src/tests/data/yaml/metrics.yaml | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 2452660c..3d28607e 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -244,7 +244,11 @@ def __init__(self, group): self.members = UnitMember.parse(group.get("members")) -class MetricSemanticConvention(BaseSemanticConvention): +class MetricGroupSemanticConvention(BaseSemanticConvention): + GROUP_TYPE_NAME = "metric_group" + + +class MetricSemanticConvention(MetricGroupSemanticConvention): GROUP_TYPE_NAME = "metric" allowed_keys: Tuple[str, ...] = BaseSemanticConvention.allowed_keys + ( @@ -262,7 +266,7 @@ class MetricSemanticConvention(BaseSemanticConvention): allowed_instruments: Tuple[str, ...] = tuple( yaml_to_markdown_instrument_repr.keys() - ) + (None,) + ) def __init__(self, group): super().__init__(group) @@ -276,10 +280,10 @@ def __init__(self, group): def validate(self): val_tuple = (self.metric_name, self.unit, self.instrument) - if not all(val_tuple) and any(val_tuple): + if not all(val_tuple): raise ValidationError.from_yaml_pos( self._position, - "Either all or none of name, units, and instrument must be defined", + "All of metric_name, units, and instrument must be defined", ) if self.instrument not in self.allowed_instruments: @@ -584,6 +588,7 @@ def attributes(self): SpanSemanticConvention, ResourceSemanticConvention, EventSemanticConvention, + MetricGroupSemanticConvention, MetricSemanticConvention, UnitSemanticConvention, ScopeSemanticConvention, diff --git a/semantic-conventions/src/tests/data/yaml/metrics.yaml b/semantic-conventions/src/tests/data/yaml/metrics.yaml index eaeb32a5..dfc339a0 100644 --- a/semantic-conventions/src/tests/data/yaml/metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/metrics.yaml @@ -1,7 +1,7 @@ groups: - id: metric.foo prefix: bar - type: metric + type: metric_group brief: "This document defines foo." note: > Details about foo. From 4a8aabdb6a0a3429a41566043a0bfb9783229064 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 26 Oct 2022 13:52:05 +1100 Subject: [PATCH 47/54] update syntax.md and JSON schema for metric_group addition --- semantic-conventions/semconv.schema.json | 17 +++++++++++++++++ semantic-conventions/syntax.md | 24 +++++++++++++++++------- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/semantic-conventions/semconv.schema.json b/semantic-conventions/semconv.schema.json index 7ffad0d3..97a5cedc 100644 --- a/semantic-conventions/semconv.schema.json +++ b/semantic-conventions/semconv.schema.json @@ -54,6 +54,7 @@ "span", "resource", "metric", + "metric_group", "event", "scope" ], @@ -167,8 +168,24 @@ {"required": ["name"]} ] }, + "MetricGroupSemanticConvention": { + "allOf": [{ "$ref": "#/definitions/SemanticConventionBase" }], + "required": ["type"], + "properties": { + "type": { + "type": "string", + "const": "metric_group" + } + } + }, "MetricSemanticConvention": { "allOf": [{ "$ref": "#/definitions/SemanticConventionBase" }], + "required": [ + "type", + "metric_name", + "instrument", + "unit" + ], "properties": { "instrument": { "type": "string", diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index 94492adf..bed840f6 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -40,14 +40,15 @@ All attributes are lower case. groups ::= semconv | semconv groups -semconv ::= id [convtype] brief [note] [prefix] [extends] [stability] [deprecated] attributes [constraints] [specificfields] [instrument] [metric_name] [unit] +semconv ::= id [convtype] brief [note] [prefix] [extends] [stability] [deprecated] attributes [constraints] [specificfields] id ::= string convtype ::= "span" # Default if not specified | "resource" # see spanspecificfields | "event" # see eventspecificfields - | "metric" + | "metric" # see metricfields + | "metric_group" | "scope" brief ::= string @@ -107,6 +108,7 @@ include ::= id specificfields ::= spanfields | eventfields + | metricfields spanfields ::= [events] [span_kind] eventfields ::= [name] @@ -121,13 +123,13 @@ events ::= id {id} # MUST point to an existing event group name ::= string -metric_name ::= string +metricfields ::= metric_name instrument unit +metric_name ::= string instrument ::= "counter" | "histogram" | "gauge" | "updowncounter" - unit ::= string ``` @@ -176,16 +178,24 @@ The following is only valid if `type` is `event`: If not specified, the `prefix` is used. If `prefix` is empty (or unspecified), `name` is required. +#### Metric Group semantic convention + +Metric group inherits all from the base semantic convention, and does not +add any additional fields. Its only requirement is that `type` is `metric_group`. + +The metric group semconv is intended to be a group where metric attributes +can be defined and then referenced from other `metric` groups. + #### Metric semantic convention The following is only valid if `type` is `metric`: - - `metric_name`, the metric name as described by the [OpenTelemetry Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#timeseries-model). - - `instrument`, the [instrument type]( https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument) + - `metric_name`, required, the metric name as described by the [OpenTelemetry Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#timeseries-model). + - `instrument`, required, the [instrument type]( https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument) that should be used to record the metric. Note that the semantic conventions must be written using the names of the synchronous instrument types (`counter`, `gauge`, `updowncounter` and `histogram`). For more details: [Metrics semantic conventions - Instrument types](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics/semantic_conventions#instrument-types). - - `unit`, the unit in which the metric is measured, which should adhere to + - `unit`, required, the unit in which the metric is measured, which should adhere to [the guidelines](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics/semantic_conventions#instrument-units). ### Attributes From 9943b8f7b74f122df8d5ea4b535b5d01128668d4 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 30 Nov 2022 10:53:57 +1100 Subject: [PATCH 48/54] address comment regarding wording in changelog --- semantic-conventions/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/semantic-conventions/CHANGELOG.md b/semantic-conventions/CHANGELOG.md index 1fcfd370..74e075e5 100644 --- a/semantic-conventions/CHANGELOG.md +++ b/semantic-conventions/CHANGELOG.md @@ -4,7 +4,7 @@ Please update the changelog as part of any significant pull request. ## Unreleased -- Add definitions for metrics in YAML semconv, plus generation to markdown +- Add a semantic convention type for Metrics ("metric" and "metric_group") ([#79](https://github.com/open-telemetry/build-tools/pull/79)) ## v0.14.0 From cf470892bacc8e2a92440080613cba5883df922b Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 30 Nov 2022 11:04:34 +1100 Subject: [PATCH 49/54] remove unnecessary 'extends' from test case --- semantic-conventions/src/tests/data/yaml/metrics.yaml | 2 -- .../src/tests/semconv/model/test_correct_parse.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/semantic-conventions/src/tests/data/yaml/metrics.yaml b/semantic-conventions/src/tests/data/yaml/metrics.yaml index dfc339a0..b6ef1627 100644 --- a/semantic-conventions/src/tests/data/yaml/metrics.yaml +++ b/semantic-conventions/src/tests/data/yaml/metrics.yaml @@ -14,7 +14,6 @@ groups: - id: metric.foo.size prefix: foo type: metric - extends: metric.foo metric_name: foo.size brief: "Measures the size of foo." instrument: histogram @@ -29,7 +28,6 @@ groups: - id: metric.foo.active_eggs prefix: foo type: metric - extends: metric.foo metric_name: foo.active_eggs brief: "Measures how many eggs are currently active." instrument: updowncounter diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index 69fb8981..f0c822c7 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -218,7 +218,7 @@ def test_metrics(self): expected = { "id": "metric.foo.size", "prefix": "foo", - "extends": "metric.foo", + "extends": "", "n_constraints": 0, "metric_name": "foo.size", "unit": "{bars}", From 086b6162375e1e01dce08d756d8c3371f0275f0d Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 30 Nov 2022 11:22:38 +1100 Subject: [PATCH 50/54] address comments about clarifying and rewording definitions in syntax.md --- semantic-conventions/syntax.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/semantic-conventions/syntax.md b/semantic-conventions/syntax.md index bed840f6..f9b9e2e9 100644 --- a/semantic-conventions/syntax.md +++ b/semantic-conventions/syntax.md @@ -181,10 +181,10 @@ The following is only valid if `type` is `event`: #### Metric Group semantic convention Metric group inherits all from the base semantic convention, and does not -add any additional fields. Its only requirement is that `type` is `metric_group`. +add any additional fields. -The metric group semconv is intended to be a group where metric attributes -can be defined and then referenced from other `metric` groups. +The metric group semconv is a group where related metric attributes +can be defined and then referenced from other `metric` groups using `ref`. #### Metric semantic convention From 9da7198d7ce9a4e0749c3d5b522a511f64cb2137 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 30 Nov 2022 11:23:38 +1100 Subject: [PATCH 51/54] remove field that stores instrument markdown repr, instead calculate while rendering markdown --- .../src/opentelemetry/semconv/model/semantic_convention.py | 7 ++----- .../opentelemetry/semconv/templating/markdown/__init__.py | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 3d28607e..f0373f8d 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -257,7 +257,7 @@ class MetricSemanticConvention(MetricGroupSemanticConvention): "instrument", ) - yaml_to_markdown_instrument_repr: Dict[str, str] = { + canonical_instrument_name_by_yaml_name: Dict[str, str] = { "counter": "Counter", "updowncounter": "UpDownCounter", "histogram": "Histogram", @@ -265,7 +265,7 @@ class MetricSemanticConvention(MetricGroupSemanticConvention): } allowed_instruments: Tuple[str, ...] = tuple( - yaml_to_markdown_instrument_repr.keys() + canonical_instrument_name_by_yaml_name.keys() ) def __init__(self, group): @@ -273,9 +273,6 @@ def __init__(self, group): self.metric_name = group.get("metric_name") self.unit = group.get("unit") self.instrument = group.get("instrument") - self.instrument_markdown_fmt = self.yaml_to_markdown_instrument_repr.get( - self.instrument - ) self.validate() def validate(self): diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 39423c7f..d2cbdfd7 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -238,7 +238,7 @@ def to_markdown_metric_table( output.write( "| `{}` | {} | `{}` | {} |\n".format( semconv.metric_name, - semconv.instrument_markdown_fmt, + MetricSemanticConvention.canonical_instrument_name_by_yaml_name[semconv.instrument], semconv.unit, semconv.brief, ) From cff3949aa16e4b90bde76ea1a47940cb477b9e86 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Wed, 30 Nov 2022 11:34:48 +1100 Subject: [PATCH 52/54] appease lint --- .../src/opentelemetry/semconv/templating/markdown/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index d2cbdfd7..94c98f00 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -238,7 +238,9 @@ def to_markdown_metric_table( output.write( "| `{}` | {} | `{}` | {} |\n".format( semconv.metric_name, - MetricSemanticConvention.canonical_instrument_name_by_yaml_name[semconv.instrument], + MetricSemanticConvention.canonical_instrument_name_by_yaml_name[ + semconv.instrument + ], semconv.unit, semconv.brief, ) From fdd98f3da02b6a2dd6adda1d698f437ed0ce5008 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 1 Dec 2022 11:01:08 +1100 Subject: [PATCH 53/54] appease new linter f-string requirements --- .../semconv/model/semantic_convention.py | 4 +--- .../semconv/templating/markdown/__init__.py | 11 ++++------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 719e3a29..8421124f 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -284,9 +284,7 @@ def validate(self): if self.instrument not in self.allowed_instruments: raise ValidationError.from_yaml_pos( self._position, - "Instrument '{}' is not a valid instrument name".format( - self.instrument - ), + f"Instrument '{self.instrument}' is not a valid instrument name", ) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 80c4e7b4..e8b43414 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -194,9 +194,7 @@ def to_markdown_attribute_table( attr_to_print.append(attr) if self.render_ctx.group_key is not None and not attr_to_print: raise ValueError( - "No attributes retained for '{}' filtering by '{}'".format( - semconv.semconv_id, self.render_ctx.group_key - ) + f"No attributes retained for '{semconv.semconv_id}' filtering by '{self.render_ctx.group_key}'" ) if attr_to_print: output.write(MarkdownRenderer.table_headers) @@ -216,9 +214,7 @@ def to_markdown_metric_table( """ if not isinstance(semconv, MetricSemanticConvention): raise ValueError( - "semconv `{}` was specified with `metric_table`, but it is not a metric convention".format( - semconv.semconv_id - ) + f"semconv `{semconv.semconv_id}` was specified with `metric_table`, but it is not a metric convention" ) output.write( @@ -226,6 +222,7 @@ def to_markdown_metric_table( "| -------- | --------------- | ----------- | -------------- |\n" ) output.write( + # pylint: disable=C0209 "| `{}` | {} | `{}` | {} |\n".format( semconv.metric_name, MetricSemanticConvention.canonical_instrument_name_by_yaml_name[ @@ -478,7 +475,7 @@ def _render_group(self, semconv, parameters, output): self.to_markdown_metric_table(semconv, output) else: if isinstance(semconv, EventSemanticConvention): - output.write("The event name MUST be `{}`.\n\n".format(semconv.name)) + output.write(f"The event name MUST be `{semconv.name}`.\n\n") self.to_markdown_attribute_table(semconv, output) self.to_markdown_notes(output) From 0f086b02f5601e6faceff0ee5048aba5fca4b584 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 6 Dec 2022 14:21:51 +1100 Subject: [PATCH 54/54] use f-string and inline a variable --- .../semconv/templating/markdown/__init__.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index e8b43414..fc57d9ac 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -217,20 +217,15 @@ def to_markdown_metric_table( f"semconv `{semconv.semconv_id}` was specified with `metric_table`, but it is not a metric convention" ) + instrument = MetricSemanticConvention.canonical_instrument_name_by_yaml_name[ + semconv.instrument + ] output.write( "| Name | Instrument Type | Unit (UCUM) | Description |\n" "| -------- | --------------- | ----------- | -------------- |\n" ) output.write( - # pylint: disable=C0209 - "| `{}` | {} | `{}` | {} |\n".format( - semconv.metric_name, - MetricSemanticConvention.canonical_instrument_name_by_yaml_name[ - semconv.instrument - ], - semconv.unit, - semconv.brief, - ) + f"| `{semconv.metric_name}` | {instrument} | `{semconv.unit}` | {semconv.brief} |\n" ) def to_markdown_anyof(self, anyof: AnyOf, output: io.StringIO):