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.

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 flexible registry generate command #45

Merged
merged 8 commits into from
Mar 13, 2024

Conversation

lquerel
Copy link
Contributor

@lquerel lquerel commented Mar 12, 2024

This PR integrates MiniJinja with jaq filters (a jq-compatible Rust implementation) to specify what to extract and process from the resolved registry, as well as how to apply the Jinja2 templates.

All errors are collected and displayed at the end of the command execution. For each error, the command shows the location in the file, the reason for the error, and the content of the evaluation context. This information helps template authors understand the issues. Similar errors are grouped together.

Note 1: I had to update multiple files not related to this PR in order to make the last version of Clippy happy.

Note 2: Why use jaq filters instead of JSONPath or similar selectors? jaq filters are extremely powerful; they can be used to sort or group registry signals (e.g., by prefix) and much more. Such features are not supported by JSONPath.

Example of error returned by the command.

✖ undefined value (in group.md:3)
---------------------------------- group.md -----------------------------------
   1 | {%- set file_name = ctx.id | file_name -%}
   2 | {{- template.set_file_name("group/" ~ file_name ~ ".md") -}}
   3 > {{ rrr.rr }}
     i    ^^^^^^ undefined value
   4 | # Group `{{ ctx.id }}` ({{ ctx.type }})
   5 |
   6 | ## Brief
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Referenced variables: {
    ctx: {
        "attributes": [
            {
                "brief": "A unique id to identify a session.",
                "examples": "00112233-4455-6677-8899-aabbccddeeff",
                "name": "session.id",
                "requirement_level": "opt_in",
                "stability": "experimental",
                "type": "string",
            },
            {
                "brief": "The previous `session.id` for this user, when known.",
                "examples": "00112233-4455-6677-8899-aabbccddeeff",
                "name": "session.previous_id",
                "requirement_level": "opt_in",
                "stability": "experimental",
                "type": "string",
            },
        ],
        "brief": "Session is defined as the period of time encompassing all activities performed by the application and the actions executed by the end user.\nConsequently, a Session is represented as a collection of Logs, Events, and Spans emitted by the Client Application throughout the Session's duration. Each Session is assigned a unique identifier, which is included as an attribute in the Logs, Events, and Spans generated during the Session's lifecycle.\nWhen a session reaches end of life, typically due to user inactivity or session timeout, a new session identifier will be assigned. The previous session identifier may be provided by the instrumentation so that telemetry backends can link the two sessions.\n",
        "events": [],
        "id": "session-id",
        "instrument": none,
        "lineage": {
            "provenance": "https://github.com/open-telemetry/semantic-conventions.git/model/session.yaml",
        },
        "metric_name": none,
        "name": none,
        "prefix": "session",
        "span_kind": none,
        "type": "attribute_group",
        "unit": none,
    },
    file_name: "SessionId",
    template: TemplateObject {
        file_name: Mutex {
            data: "group/SessionId.md",
            poisoned: false,
            ..
        },
    },
}
-------------------------------------------------------------------------------

Found 295 similar errors

@lquerel lquerel requested a review from jsuereth as a code owner March 12, 2024 00:28
@lquerel lquerel self-assigned this Mar 12, 2024
@lquerel lquerel added the enhancement New feature or request label Mar 12, 2024
@lquerel lquerel linked an issue Mar 12, 2024 that may be closed by this pull request
@lquerel lquerel linked an issue Mar 12, 2024 that may be closed by this pull request

### Attributes

{% for attribute in group.attributes %}
{% for attribute in ctx.attributes %}
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for moving from group to ctx

)
.expect("Failed to generate registry assets");
let template_registry =
TemplateRegistry::try_from_resolved_registry(&schema.registries[0], &schema.catalog)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a huge fun of always indexing the first registry here. Did you have a bug open to clean that up at some point or alter the representation?

I think you could even just pass the entire &schema directly to try_from_resolved_registry to hide that complexity until we clean it up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will open an issue to improve this as it's indeed far from optimal. It was like an intermediary/unfinished step, the final goal being at some point to support multiple registries from the same app telemetry schema (e.g. OTel official registry + 0 to n vendor or internal registries).

@lquerel lquerel merged commit 9599f72 into open-telemetry:main Mar 13, 2024
14 checks passed
@lquerel lquerel deleted the make-generate-more-flexible branch April 4, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement an example of weaver generate registry subcommand weaver resolve registry subcommand
2 participants