-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[chore] add a RFC to codify the work on JSON schema configuration support #13784
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13784 +/- ##
==========================================
+ Coverage 91.38% 91.40% +0.01%
==========================================
Files 646 646
Lines 42605 42605
==========================================
+ Hits 38936 38943 +7
+ Misses 2844 2839 -5
+ Partials 825 823 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
||
We work to add initial schemas by hand in libraries in opentelemetry-collector. | ||
|
||
The schemas are added to the library metadata.yaml files. |
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.
Is it possible not to couple the solution with metadata.yaml? I.e. it can be used as a source of the schema but ideally the tooling should be able to accept alternative location of config schemas.
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.
It's a JSON schema and in a yaml file, so maybe a couple ways:
- We use a yaml anchor and you define the config key in a different section.
- You use a JSON schema with a single reference to another schema.
Note if we move the schema to a separate location, we need to be able to resolve it if we lint with checkapi.
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.
Looked at your draft PR, and I'm worried that these schema blocks are too big for the metadata.yaml file. Could we have this be a path relative to the metadata.yaml? Could a go.mod link a dependency that embeds the schema? Could we refer to versioned URLs in other repositories? https://github.com/open-telemetry/opentelemetry-collector/pull/13726/files#r2334122066
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.
@jmacd I think you might be right that metadata.yaml becomes a bit big with this, but as I mention above, we can build a reference model so it's possible to link to another file. I also think we need to try, see how difficult or messy it gets, and get wiser.
My goal is to leverage our git tags for any references to versioned schemas, I guess.
|
||
We work to add initial schemas by hand in libraries in opentelemetry-collector. | ||
|
||
The schemas are added to the library metadata.yaml files. |
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.
Looked at your draft PR, and I'm worried that these schema blocks are too big for the metadata.yaml file. Could we have this be a path relative to the metadata.yaml? Could a go.mod link a dependency that embeds the schema? Could we refer to versioned URLs in other repositories? https://github.com/open-telemetry/opentelemetry-collector/pull/13726/files#r2334122066
Description
Add a RFC to discuss supporting expressing the configuration of a component as a JSON schema.
Link to tracking issue
Relates to #9769 and open-telemetry/opentelemetry-collector-contrib#42214