-
Notifications
You must be signed in to change notification settings - Fork 32
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
Generate assets from a semantic convention registry and a set of jinja2 templates #43
Conversation
…y define the file name of the output.
…o support include clause.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving but a few caveats for the future:
- Next time I'd prefer a separate PR that updates the resolver for changes in stability, e.g. That'd be a lot simpler to review and merge if not bundled with the template change.
- I think you and I may need to sit down and figure out priorities for this project. You can see my comment on templating and feature set. I think we both agree we need to get to a "resolved schema" file format ASAP and propose this in the specification, but we may disagree on how to get there. My current thinking is showcase that weaver is 1:1 compatible w/ buld-tools for just the semconv-repo itself and outline the resolved schema it produces along with an accompanying specification.
In any case, nice work with this! Exciting to see that the template language isn't a huge jump away from JINJA.
/// present and stability differs from deprecated, this will result in an | ||
/// error. | ||
#[serde(skip_serializing_if = "Option::is_none")] | ||
pub stability: Option<Stability>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're working to make stability a required field everywhere. I believe that would apply to this structure to, eventually.
At a minimum, stability will be required on all attributes/metrics.
@@ -0,0 +1,152 @@ | |||
# Template Engine | |||
|
|||
OTel Weaver is capable of generating documentation or code from a semantic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before we spend more time going down this path, we should validate this is desired from the rest of SEmconv maintainers.
There are several types of codegen I think we'll want going forward:
- We have human-written instructions for semantic conventions where-in we embed rendered markdown outlining the state of the semconv registry, via
<-- semconv( render command) -->
. I'd prefer if we worked towards feature parity on this behavior first. - We have basic rendering of the "registry itself" as a generic "{language}doc" like output. While I don't think that's exactly what you had in mind in this PR, it's the second thing I'd like to see.
- We will need to generate "{langauge}doc" like output for "application" instrumentation. I believe this is what you're creating here, and I really like the direction, but think we should first make sure 1+2 are solved.
- We will need to generate code.
I actually think we should keep these four use cases as separate things for weaver.
TL;DR; I think this currently makes another good demo, but we need to start socializing it so it can go further. Additionally, let's see if we can replicate some key use cases of semconv generation today.
This PR sets up the framework for generating code or documentation for semantic conventions. Integration into the command line is not part of this PR, but a unit test in
weaver_forge
demonstrates and tests the generation possibilities.Documentation on using the template engine is available in the
docs/template-engine.md
directory.The template engine is based on the
minijinja
crate. This template engine is largely compatible with the Jinja2 engine (Python ecosystem).