-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add component JSON schema generation to mdatagen #14288
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?
Add component JSON schema generation to mdatagen #14288
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (68.07%) 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 #14288 +/- ##
==========================================
- Coverage 91.80% 91.56% -0.24%
==========================================
Files 676 679 +3
Lines 42415 42793 +378
==========================================
+ Hits 38937 39182 +245
- Misses 2420 2514 +94
- Partials 1058 1097 +39 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
This is a good example to look at the generated schema
c7b9168 to
a88d7b8
Compare
CodSpeed Performance ReportMerging this PR will not alter performanceComparing
|
|
I really like that this approach attempts to capture the |
|
It seems that we are working on solving the same problem, but using slightly different methods. I have started working on a script that generates config schemas based on Go structs using AST parsing. This is part of a larger plan to introduce schemas to the OpenTelemetry collector. You can find more details in my PRs:
I think it makes sense to start a discussion on the preferred solution. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
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.
I like this approach better and I think this can be merged roughly as is
cmd/mdatagen/internal/samplereceiver/internal/metadata/schemas/config_schema.json
Outdated
Show resolved
Hide resolved
receiver/otlpreceiver/internal/metadata/schemas/config_schema.json
Outdated
Show resolved
Hide resolved
a166f90 to
f190d98
Compare
|
Given that we’ve gone through many iterations, and that a similar solution has recently been merged (the schemagen tool), I’d like to propose a call to discuss the end result we want to have, align on a single approach, and avoid stepping on each other’s toes. There are a few remaining points we need to clarify:
@pavolloffay @mx-psi @evan-bradley @jkoronaAtCisco how does it sound? |
Signed-off-by: Pavol Loffay <[email protected]>
3ba482b to
05fa4e0
Compare
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Replaces open-telemetry#13784 This RFC proposes a roadmap for introducing configuration schemas to OpenTelemetry Collector components. It establishes a schema-first approach in which Go structs, JSON schemas, and documentation are all generated from a single YAML source of truth. This RFC is the result of discussions among contributors involved in this effort: - @atoulme - @evan-bradley - @jkoronaAtCisco - @mx-psi - @pavolloffay Related Issues / PRs: - open-telemetry/opentelemetry-collector-contrib#42214 - open-telemetry#9769 - open-telemetry#14288 - open-telemetry/opentelemetry-collector-contrib#27003
Replaces open-telemetry#13784 This RFC proposes a roadmap for introducing configuration schemas to OpenTelemetry Collector components. It establishes a schema-first approach in which Go structs, JSON schemas, and documentation are all generated from a single YAML source of truth. This RFC is the result of discussions among contributors involved in this effort: - @atoulme - @evan-bradley - @jkoronaAtCisco - @mx-psi - @pavolloffay Related Issues / PRs: - open-telemetry/opentelemetry-collector-contrib#42214 - open-telemetry#9769 - open-telemetry#14288 - open-telemetry/opentelemetry-collector-contrib#27003
Replaces open-telemetry#13784 This RFC proposes a roadmap for introducing configuration schemas to OpenTelemetry Collector components. It establishes a schema-first approach in which Go structs, JSON schemas, and documentation are all generated from a single YAML source of truth. This RFC is the result of discussions among contributors involved in this effort: - @atoulme - @evan-bradley - @jkoronaAtCisco - @mx-psi - @pavolloffay Related Issues / PRs: - open-telemetry/opentelemetry-collector-contrib#42214 - open-telemetry#9769 - open-telemetry#14288 - open-telemetry/opentelemetry-collector-contrib#27003
|
We had a call to discuss overall goal and agreed on a approach described in this PRF |
…4433) Replaces #13784 This RFC proposes a roadmap for introducing configuration schemas to OpenTelemetry Collector components. It establishes a schema-first approach in which Go structs, JSON schemas, and documentation are all generated from a single YAML source of truth. This RFC is the result of discussions among contributors involved in this effort: - @atoulme - @evan-bradley - @iblancasa - @jkoronaAtCisco - @mx-psi - @pavolloffay Related Issues / PRs: - open-telemetry/opentelemetry-collector-contrib#42214 - #9769 - #14288 - open-telemetry/opentelemetry-collector-contrib#27003
Description
This is an alternative PR #14261
metadata.yaml:Notable implementation details
format: durationDurationtime.Duration(Default): Marshals into an integer representing nanoseconds (e.g.,300000000000)."type": "string", "format": "duration"you need a custom wrapper around time.Duration that implements the json.Marshaler interface to convert the time into ISO 8601 format.Solution
{ "type": "string", "pattern": "^([+-]?(\\d+(\\.\\d*)?|\\.\\d+)(ns|us|µs|ms|s|m|h))+$", "example": "1h30m10s", "description": "A duration string (e.g., '10s', '1.5h'). Valid units: ns, us, ms, s, m, h." }mapstructure:",squashConfig object uses non-standard name
If the config object uses non standard name (e.g.
MyCfg) or comes from a different package it can be specified in the configExample: https://github.com/open-telemetry/opentelemetry-collector-contrib/compare/main...pavolloffay:schema-example?expand=1
additionalPropertiesadditionalPropertiesis set to true, which forbids properties that are not in the config and aligns with the collector behavior.Open questions
How to handle required fields?
Validation is handled via
Validatorinterface e.g.func (cfg *Config) Validate() error {.How to handle default fields?
The
factory.CreateDefaultConfig()creates a config with default fields.There are 2 possible approaches:
jsonschema:"default=localhost:4317"factory.CreateDefaultConfig(), stores the output which is used by the mdatagen schema generation.Link to tracking issue
Fixes #9769
Fixes open-telemetry/opentelemetry-collector-contrib#42214
Implements: #13784
Updates: open-telemetry/opentelemetry-collector-contrib#24189
Testing
olpreceiverin this repo has enabled schema generationDocumentation