[chore] Add implementation details to Component Configuration Management RFC - #14486
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14486 +/- ##
=======================================
Coverage 91.27% 91.27%
=======================================
Files 709 709
Lines 46224 46224
=======================================
Hits 42192 42192
Misses 2817 2817
Partials 1215 1215 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Added descriptions for configuration schema properties and clarified generated code for custom fields.
Change the handling of referenced schemas in JSON Schema.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
@open-telemetry/collector-approvers PTAL. I guess we need more approvals before merging RFC amendments |
|
Can you add to the PR description how you have fulfilled the requirements regarding announcing this RFC/RFC Amendment? |
This comment was marked as outdated.
This comment was marked as outdated.
| in a single JSON Schema file. | ||
| - Each referenced schema will be embedded inline. | ||
| - The `$id` field will be set to the component's import path for easy identification. | ||
| - The custom types will be represented as `any` type in JSON Schema. |
There was a problem hiding this comment.
How about having a way to provide schemas for custom types?
There was a problem hiding this comment.
You can still do it. Let's say that you wan to use external type github.com/foo.bar in your config. Then you can do that in your metadata.yaml config:
config:
type: object
properties:
bar:
go:
type: github.com/foo.bar
type: object
properties:
x:
type: integer
y:
type: integerThat would generate json schema like:
{
"type": "object",
"properties": {
"bar": {
"type": "object",
"properties": {
"x": {
"type": "integer"
},
"y": {
"type": "integer"
}
}
}
}
}But generated go struct would be:
import "github.com/foo.bar"
struct Config {
bar: foo.Bar `mapstructure:"bar"`
}|
|
||
| **Objective**: Use OCB to bind all components schemas to single schema that describes entire collector config file | ||
|
|
||
| **Success Criteria:** |
There was a problem hiding this comment.
I don't quite understand this requirement. Can you elaborate?
There was a problem hiding this comment.
Yes. Each component has a $id field.
For the global schema, we need a "global" $id property too. That is needed to differentiate between 2 schema files.
There was a problem hiding this comment.
For each component json schema we're going to create $id using following pattern:
<namespace>/<path>/<component_name>
for example: go.opentelemetry.io/collector/cmd/mdatagen/internal/samplescraper
For global id we can use just namespace, e.g. go.opentelemetry.io/collector. What do you think?
| } | ||
| ``` | ||
|
|
||
| #### Generated JSON Schema details |
There was a problem hiding this comment.
Some components have an alias now because their old names are deprecated. I think we should support validating with the aliases for the time they are there. So, they need to be added to the schemas.
There was a problem hiding this comment.
I'm not familiar with this problem. Can you provide some examples?
There was a problem hiding this comment.
You can find the full context here open-telemetry/opentelemetry-collector-contrib#45339
| } | ||
| ``` | ||
|
|
||
| #### Generated JSON Schema details |
There was a problem hiding this comment.
We also need to make duration/custom-type validation portable across validators
There was a problem hiding this comment.
Mdatagen would add pattern validator automatically to those fields.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This is entering final comment period and will be merged on or after June 2nd if it fulfills all criteria for merging. @jkoronaAtCisco
|
|
We also need to make CI pass to be able to merge this |
85d214b to
9afeca0
Compare
Description
Added implementation details and examples to RFC document
Changes were announced on
#otel-collector-devslack channel.Link to tracking issue
Extends #14433