Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More semconv features and cleanups #97

Merged
merged 7 commits into from
Apr 12, 2024
Merged

More semconv features and cleanups #97

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1ff50d3
Debug is much nicer experience for template generation failures.
jsuereth Apr 10, 2024
6900d59
Add helper for dealing with seperated ids in semconv.
jsuereth Apr 10, 2024
6c2ce55
Fix test suite for running on windows
jsuereth Apr 11, 2024
f9bd33d
Fixes from code review.
jsuereth Apr 11, 2024
b33000b
Update crates/weaver_forge/src/lib.rs
jsuereth Apr 11, 2024
b5a90bd
Revert "Update crates/weaver_forge/src/lib.rs"
jsuereth Apr 11, 2024
32ea520
Merge branch 'main' into wip-features-for-semconv-registry
jsuereth Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ COPY data /build/data
COPY src /build/src
COPY templates /build/templates
COPY tests build/tests
RUN cargo build --release
# Don't build release, so we get template debugging output.
jsuereth marked this conversation as resolved.
Show resolved Hide resolved
RUN cargo build

# The runtime image
FROM alpine:3.18.3
LABEL maintainer="The OpenTelemetry Authors"
WORKDIR /weaver
COPY --from=weaver-build /build/target/release/weaver /weaver/weaver
COPY --from=weaver-build /build/target/debug/weaver /weaver/weaver
ENTRYPOINT ["/weaver/weaver"]
2 changes: 1 addition & 1 deletion crates/weaver_forge/expected_output/attribute_group/attributes_jvm_memory.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## Group `attributes.jvm.memory` (attribute_group)
## Group `attributes_jvm_memory` (attribute_group)

### Brief

Expand Down
21 changes: 1 addition & 20 deletions crates/weaver_forge/expected_output/attribute_group/registry_db.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## Group `registry.db` (attribute_group)
## Group `registry_db` (attribute_group)

### Brief

Expand All @@ -14,7 +14,6 @@ prefix: db
The data center of the coordinating node for a query.



- Requirement Level: Recommended

- Tag: tech-specific-cassandra
Expand All @@ -28,7 +27,6 @@ The data center of the coordinating node for a query.
The ID of the coordinating node for a query.



- Requirement Level: Recommended

- Tag: tech-specific-cassandra
Expand All @@ -42,7 +40,6 @@ The ID of the coordinating node for a query.
The consistency level of the query. Based on consistency values from [CQL](https://docs.datastax.com/en/cassandra-oss/3.0/cassandra/dml/dmlConfigConsistency.html).



- Requirement Level: Recommended

- Tag: tech-specific-cassandra
Expand All @@ -55,7 +52,6 @@ The consistency level of the query. Based on consistency values from [CQL](https
Whether or not the query is idempotent.



- Requirement Level: Recommended

- Tag: tech-specific-cassandra
Expand All @@ -68,7 +64,6 @@ Whether or not the query is idempotent.
The fetch size used for paging, i.e. how many rows will be returned at once.



- Requirement Level: Recommended

- Tag: tech-specific-cassandra
Expand All @@ -84,7 +79,6 @@ The fetch size used for paging, i.e. how many rows will be returned at once.
The number of times a query was speculatively executed. Not set or `0` if the query was not executed speculatively.



- Requirement Level: Recommended

- Tag: tech-specific-cassandra
Expand Down Expand Up @@ -116,7 +110,6 @@ This mirrors the db.sql.table attribute but references cassandra rather than sql
The connection string used to connect to the database. It is recommended to remove embedded credentials.



- Requirement Level: Recommended

- Tag: db-generic
Expand Down Expand Up @@ -240,7 +233,6 @@ Cosmos DB sub status code.
Represents the identifier of an Elasticsearch cluster.



- Requirement Level: Recommended

- Tag: tech-specific-elasticsearch
Expand All @@ -256,7 +248,6 @@ Represents the identifier of an Elasticsearch cluster.
Represents the human-readable identifier of the node/instance to which a request was routed.



- Requirement Level: Recommended

- Tag: tech-specific-elasticsearch
Expand All @@ -272,7 +263,6 @@ Represents the human-readable identifier of the node/instance to which a request
A dynamic value in the url path.



Many Elasticsearch url paths allow dynamic values. These SHOULD be recorded in span attributes in the format `db.elasticsearch.path_parts.<key>`, where `<key>` is the url path part name. The implementation SHOULD reference the [elasticsearch schema](https://raw.githubusercontent.com/elastic/elasticsearch-specification/main/output/schema/schema.json) in order to map the path part values to their names.

- Requirement Level: Recommended
Expand All @@ -291,7 +281,6 @@ Many Elasticsearch url paths allow dynamic values. These SHOULD be recorded in s
The fully-qualified class name of the [Java Database Connectivity (JDBC)](https://docs.oracle.com/javase/8/docs/technotes/guides/jdbc/) driver used to connect.



- Requirement Level: Recommended

- Tag: tech-specific-jdbc
Expand All @@ -308,7 +297,6 @@ The fully-qualified class name of the [Java Database Connectivity (JDBC)](https:
The MongoDB collection being accessed within the database stated in `db.name`.



- Requirement Level: Recommended

- Tag: tech-specific-mongodb
Expand All @@ -325,7 +313,6 @@ The MongoDB collection being accessed within the database stated in `db.name`.
The Microsoft SQL Server [instance name](https://docs.microsoft.com/sql/connect/jdbc/building-the-connection-url?view=sql-server-ver15) connecting to. This name is used to determine the port of a named instance.



If setting a `db.mssql.instance_name`, `server.port` is no longer required (but still recommended if non-standard).

- Requirement Level: Recommended
Expand All @@ -341,7 +328,6 @@ If setting a `db.mssql.instance_name`, `server.port` is no longer required (but
This attribute is used to report the name of the database being accessed. For commands that switch the database, this should be set to the target database (even if the command fails).



In some SQL databases, the database name to be used is called "schema name". In case there are multiple layers that could be considered for database name (e.g. Oracle instance name and schema name), the database name to be used is the more specific layer (e.g. Oracle schema name).

- Requirement Level: Recommended
Expand All @@ -360,7 +346,6 @@ In some SQL databases, the database name to be used is called "schema name". In
The name of the operation being executed, e.g. the [MongoDB command name](https://docs.mongodb.com/manual/reference/command/#database-operations) such as `findAndModify`, or the SQL keyword.



When setting this to an SQL keyword, it is not recommended to attempt any client-side parsing of `db.statement` just to get this property, but it should be set if the operation name is provided by the library being instrumented. If the SQL statement has an ambiguous operation, or performs more than one operation, this value may be omitted.

- Requirement Level: Recommended
Expand All @@ -380,7 +365,6 @@ When setting this to an SQL keyword, it is not recommended to attempt any client
The index of the database being accessed as used in the [`SELECT` command](https://redis.io/commands/select), provided as an integer. To be used instead of the generic `db.name` attribute.



- Requirement Level: Recommended

- Tag: tech-specific-redis
Expand Down Expand Up @@ -416,7 +400,6 @@ It is not recommended to attempt any client-side parsing of `db.statement` just
The database statement being executed.



- Requirement Level: Recommended

- Tag: db-generic
Expand Down Expand Up @@ -445,7 +428,6 @@ An identifier for the database management system (DBMS) product being used. See
Username for accessing the database.



- Requirement Level: Recommended

- Tag: db-generic
Expand All @@ -462,7 +444,6 @@ Username for accessing the database.
An identifier (address, unique name, or any other identifier) of the database instance that is executing queries or mutations on the current connection. This is useful in cases where the database is running in a clustered environment and the instrumentation is able to record the node executing the query. The client may obtain this value in databases like MySQL using queries like `select @@hostname`.



- Requirement Level: Recommended

- Tag: db-generic
Expand Down
8 changes: 1 addition & 7 deletions crates/weaver_forge/expected_output/attribute_group/registry_http.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## Group `registry.http` (attribute_group)
## Group `registry_http` (attribute_group)

### Brief

Expand All @@ -14,7 +14,6 @@ prefix: http
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://www.rfc-editor.org/rfc/rfc9110.html#field.content-length) header. For requests using transport encoding, this should be the compressed size.



- Requirement Level: Recommended

- Type: int
Expand All @@ -28,7 +27,6 @@ The size of the request payload body in bytes. This is the number of bytes trans
HTTP request headers, `<key>` being the normalized HTTP Header name (lowercase), the value being the header values.



Instrumentations SHOULD require an explicit configuration of which headers are to be captured. Including all request headers can be a security risk - explicit configuration helps avoid leaking sensitive information.
The `User-Agent` header is already captured in the `user_agent.original` attribute. Users MAY explicitly configure instrumentations to capture them even though it is not recommended.
The attribute value MUST consist of either multiple header values as an array of strings or a single-item array containing a possibly comma-concatenated string, depending on the way the HTTP library provides access to headers.
Expand Down Expand Up @@ -98,7 +96,6 @@ Original HTTP method sent by the client in the request line.
The ordinal number of request resending attempt (for any reason, including redirects).



The resend count SHOULD be updated each time an HTTP request gets resent by the client, regardless of what was the cause of the resending (e.g. redirection, authorization failure, 503 Server Unavailable, network issues, or any other).

- Requirement Level: Recommended
Expand All @@ -114,7 +111,6 @@ The resend count SHOULD be updated each time an HTTP request gets resent by the
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://www.rfc-editor.org/rfc/rfc9110.html#field.content-length) header. For requests using transport encoding, this should be the compressed size.



- Requirement Level: Recommended

- Type: int
Expand All @@ -128,7 +124,6 @@ The size of the response payload body in bytes. This is the number of bytes tran
HTTP response headers, `<key>` being the normalized HTTP Header name (lowercase), the value being the header values.



Instrumentations SHOULD require an explicit configuration of which headers are to be captured. Including all response headers can be a security risk - explicit configuration helps avoid leaking sensitive information.
Users MAY explicitly configure instrumentations to capture them even though it is not recommended.
The attribute value MUST consist of either multiple header values as an array of strings or a single-item array containing a possibly comma-concatenated string, depending on the way the HTTP library provides access to headers.
Expand Down Expand Up @@ -164,7 +159,6 @@ The attribute value MUST consist of either multiple header values as an array of
The matched route, that is, the path template in the format used by the respective server framework.



MUST NOT be populated when this is not supported by the HTTP server framework as the route attribute should have low-cardinality and the URI path can NOT substitute it.
SHOULD include the [application root](/docs/http/http-spans.md#http-server-definitions) if there is one.

Expand Down
3 changes: 1 addition & 2 deletions crates/weaver_forge/expected_output/attribute_group/registry_network.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## Group `registry.network` (attribute_group)
## Group `registry_network` (attribute_group)

### Brief

Expand Down Expand Up @@ -176,7 +176,6 @@ Version of the protocol specified in `network.protocol.name`.
[OSI transport layer](https://osi-model.com/transport-layer/) or [inter-process communication method](https://wikipedia.org/wiki/Inter-process_communication).



The value SHOULD be normalized to lowercase.

Consider always setting the transport when setting a port number, since
Expand Down
2 changes: 1 addition & 1 deletion crates/weaver_forge/expected_output/attribute_group/registry_url.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## Group `registry.url` (attribute_group)
## Group `registry_url` (attribute_group)

### Brief

Expand Down
3 changes: 1 addition & 2 deletions crates/weaver_forge/expected_output/attribute_group/registry_user_agent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## Group `registry.user_agent` (attribute_group)
## Group `registry_user_agent` (attribute_group)

### Brief

Expand All @@ -14,7 +14,6 @@ prefix: user_agent
Value of the [HTTP User-Agent](https://www.rfc-editor.org/rfc/rfc9110.html#field.user-agent) header sent by the client.



- Requirement Level: Recommended

- Type: string
Expand Down
47 changes: 36 additions & 11 deletions crates/weaver_forge/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@
case_converter(self.target_config.field_name.clone()),
);
env.add_filter("flatten", flatten);
env.add_filter("split_id", split_id);

// env.add_filter("unique_attributes", extensions::unique_attributes);
// env.add_filter("instrument", extensions::instrument);
Expand Down Expand Up @@ -421,6 +422,23 @@
Ok(Value::from(result))
}

// Helper function to take an "id" and split it by '.' into namespaces.
fn split_id(value: Value) -> Result<Vec<Value>, minijinja::Error> {
match value.as_str() {
Some(id) => {
let values: Vec<Value> = id
.split('.')
.map(|s| Value::from_safe_string(s.to_owned()))
.collect();
Ok(Value::from(values))
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
}
None => Err(minijinja::Error::new(
minijinja::ErrorKind::InvalidOperation,
format!("Expected string, found: {value}"),
)),
}
}

#[cfg(test)]
mod tests {
use std::collections::HashSet;
Expand Down Expand Up @@ -509,8 +527,10 @@

// Compare files in both sets
for file in expected_files.intersection(&observed_files) {
let file1_content = fs::read_to_string(expected_dir.as_ref().join(file))?;
let file2_content = fs::read_to_string(observed_dir.as_ref().join(file))?;
let file1_content =
fs::read_to_string(expected_dir.as_ref().join(file))?.replace("\r\n", "\n");
let file2_content =
fs::read_to_string(observed_dir.as_ref().join(file))?.replace("\r\n", "\n");

if file1_content != file2_content {
are_identical = false;
Expand All @@ -527,18 +547,23 @@
break;
}
}

// If any file is unique to one directory, they are not identical
if !expected_files
let not_in_observed = expected_files
.difference(&observed_files)
.collect::<Vec<_>>()
.is_empty()
|| !observed_files
.difference(&expected_files)
.collect::<Vec<_>>()
.is_empty()
{
.collect::<Vec<_>>();
if !not_in_observed.is_empty() {
are_identical = false;
eprintln!("Observed output is missing files: {:?}", not_in_observed);
}
let not_in_expected = observed_files
.difference(&expected_files)
.collect::<Vec<_>>();
if !not_in_expected.is_empty() {
are_identical = false;
eprintln!(
"Observed output has unexpected files: {:?}",
not_in_expected
);
}

Ok(are_identical)
Expand Down
4 changes: 2 additions & 2 deletions crates/weaver_forge/templates/test/attribute_group.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{%- set file_name = ctx.id | file_name -%}
{{- template.set_file_name("attribute_group/" ~ file_name ~ ".md") -}}

## Group `{{ ctx.id }}` ({{ ctx.type }})
## Group `{{ ctx.id | split_id | list | join("_") }}` ({{ ctx.type }})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why split_id does not directly return a Vec to avoid the list filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My cluelessness in using Minijinja. If I can just return a Vec and it works, great. I thought that Value::from_seq + returning a Vec were basically the same with one being less obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to vec. Wierdly minijinja still is confused on values.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the code of the list filter (see below) you need to wrap the vec into a value.

#[cfg_attr(docsrs, doc(cfg(feature = "builtins")))]
pub fn list(state: &State, value: Value) -> Result<Value, Error> {
    let iter = ok!(state.undefined_behavior().try_iter(value).map_err(|err| {
        Error::new(ErrorKind::InvalidOperation, "cannot convert value to list").with_source(err)
    }));
    Ok(Value::from(iter.collect::<Vec<_>>()))
}


### Brief

Expand All @@ -14,7 +14,7 @@ prefix: {{ ctx.prefix }}
{% for attribute in ctx.attributes %}
#### Attribute `{{ attribute.name }}`

{{ attribute.brief }}
{{ attribute.brief | trim }}

{% if attribute.note %}
{{ attribute.note | trim }}
Expand Down
2 changes: 1 addition & 1 deletion crates/weaver_forge/templates/test/attribute_type.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{%- if attribute.type is mapping %}
- Type: Enum [{{ attribute.type.members | map(attribute="value") | join(", ") }}]
- Type: Enum [{{ attribute.type.members | map(attribute="value") | join(", ") | trim }}]
{%- else %}
- Type: {{ attribute.type }}
{%- endif %}
2 changes: 1 addition & 1 deletion crates/weaver_forge/templates/test/group.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ prefix: {{ ctx.prefix }}

## Lineage

Source file: {{ ctx.lineage.source_file }}
Source file: {{ ctx.lineage.source_file | replace("\\", "/") }}

{% for item in ctx.lineage.attributes -%}
attribute: {{ item.id }}
Expand Down
Loading