[chore][cmd/mdatagen] Use metadata.yaml as a schema source for references - #15272
Conversation
Merging this PR will not alter performance
|
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (94.11%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #15272 +/- ##
==========================================
+ Coverage 91.23% 91.24% +0.01%
==========================================
Files 704 704
Lines 45926 45948 +22
==========================================
+ Hits 41899 41926 +27
+ Misses 2822 2819 -3
+ Partials 1205 1203 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| type: {{.Type}}/metadata | ||
| parent: {{.Type}} | ||
|
|
||
| status: | ||
| disable_codecov_badge: true | ||
| class: pkg | ||
|
|
||
| config: |
There was a problem hiding this comment.
Why do we need all this extra yaml? Why cant we generate the exact schema that goes under metrics_config in json format?
There was a problem hiding this comment.
That was leftover from my previous attempt to fix this, now as it's no longer needed I cleaned up the template. However I'd like to keep it in yaml as it's better to have it consistent with mdatagen-specific data model for code and docs generation.
There was a problem hiding this comment.
However I'd like to keep it in yaml as it's better to have it consistent with mdatagen-specific data model for code and docs generation
But we need to generate json.schema not ConfigMetadata. We are fortunate they are the same for now, but it's not going to be the case going forward. I don't think we should use it
There was a problem hiding this comment.
ConfigMetadata becomes underlying model - the source for json schema, go code and docs. MetricsBuilderConfig becomes part of the config which is relevant for go code and docs as well as for json schema. So that's why I'd like to keep it consistent. At the end I won't output ConfigMetadata anywhere. It's just a working model.
There was a problem hiding this comment.
ConfigMetadata becomes underlying model
Why? Do you want to remove https://github.com/open-telemetry/opentelemetry-collector/blob/main/cmd/mdatagen/internal/templates/config.go.tmpl as the next step? My concern is that generating metadata.yaml from metadata.yaml might be confusing and error prone
There was a problem hiding this comment.
I'm merging this PR for now. But we need to address this going forward one way or another
c05a471
…nces (open-telemetry#15272) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Previously, the schemagen loader used `config.schema.yaml` as the standalone file format for schema references. This PR changes the source of truth for cross-component schema references to `metadata.yaml` — the same file that already exists in every component directory. Instead of parsing a bare ConfigMetadata JSON-schema object, the loader now reads the config: section from a component's `metadata.yaml`, matching the structure that mdatagen already generates for every component. The generated output is likewise renamed to metadata.yaml (placed under internal/metadata/) and wrapped with the standard `metadata.yaml` envelope (type, status, config), so it is usable as a reference target by other components in the same way. <!-- Issue number if applicable --> #### Link to tracking issue Fixes open-telemetry#15259 <!--Describe what testing was performed and which tests were added.--> #### Testing Existing unit tests in `internal/schemagen` and `cmd/mdatagen/internal` were updated to reflect the new file name and Metadata wrapper format. The persistToFile test was strengthened to verify a full round-trip through loadFromFile. All tests pass.
Description
Previously, the schemagen loader used
config.schema.yamlas the standalone file format for schema references. This PR changes the source of truth for cross-component schema references tometadata.yaml— the same file that already exists in every component directory.Instead of parsing a bare ConfigMetadata JSON-schema object, the loader now reads the config: section from a component's
metadata.yaml, matching the structure that mdatagen already generates for every component. The generated output is likewise renamed to metadata.yaml (placed under internal/metadata/) and wrapped with the standardmetadata.yamlenvelope (type, status, config), so it is usable as a reference target by other components in the same way.Link to tracking issue
Fixes #15259
Testing
Existing unit tests in
internal/schemagenandcmd/mdatagen/internalwere updated to reflect the new file name and Metadata wrapper format. The persistToFile test was strengthened to verify a full round-trip through loadFromFile. All tests pass.