Generate the batch processor config from schema

[ptodev]

trafficstars

Description

This is following up on #10694. Unlike the previous PR, this one is much smaller because it only adds schema for the batch processor. Also, thanks to @omissis's amazing support go-jsonschema has a lot more features now and we might be able to just use the upstream go-jsonschema without a need for a temporary fork 🙏

I'm hoping that we can implement this for the batch processor first, and then gradually expand into more and more components. If for some reason the autogeneration becomes an issue, it can be disabled by commenting it out in the metadata.yaml file and editing config.go manually.

The PR is still in draft stage because there are a few TODOs for which I need feedback from maintainers.

Link to tracking issue

Fixes #9769

Testing

There is a test in mdatagen, but the things it tests are limited to the functionality required for the batch processor.

Documentation

I only edited the mdatagen readme file.

[omissis]

Hi, thanks for the mention. Just a word of advice: some of the PRs I merged to add support for this project caused some BC breaks that I am going to address in the next release (v0.21.0) and that will revert some of the behaviors to the previous version of the library. the new behaviors will need to be explicitly activated: this will probably break your code, but it'll just be a matter of activating a flag to fix it. please use main if you want to incorporate those changes early on

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 62.42775% with 65 lines in your changes missing coverage. Please review.

Project coverage is 91.34%. Comparing base (8d71881) to head (b828a3d). Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/mdatagen/internal/configgen.go	65.38%	28 Missing and 8 partials :warning:
cmd/mdatagen/internal/command.go	0.00%	20 Missing and 1 partial :warning:
processor/batchprocessor/config.go	66.66%	5 Missing and 3 partials :warning:

:x: Your patch check has failed because the patch coverage (62.42%) 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   #13155      +/-   ##
==========================================
- Coverage   91.51%   91.34%   -0.17%     
==========================================
  Files         522      524       +2     
  Lines       28988    29144     +156     
==========================================
+ Hits        26529    26623      +94     
- Misses       1939     1990      +51     
- Partials      520      531      +11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ptodev]

Hi, @atoulme and @yurishkuro 👋 Would you like to opine on this PR please, since you were active on the previous one?

[yurishkuro]

looks good. Is the goJSONSchema annotation extensible without doing the library changes? I'd hope the library could take some mapper function, so that, for example, we could support Optional.

[ptodev]

@yurishkuro unfortunately, go-jsonschema doesn't have this level of flexibility. It an interesting idea though, and it could be worth exploring if there are concrete use cases worth looking into.

[yurishkuro]

A concrete use case is the Optional type recently introduced in the collector. It's unknown to the library, so the lib needs to be extensible to allow custom mappings. For example, in gogoproto you could use annotation to declare that a given proto field maps to a custom Go type.

[ptodev]

It is currently possible to override the type for a field. For example, the earlier PoC PR would use configopaque.String using schema like this:

cert_pem:
  type: string
  default: ""
  goJSONSchema:
    type: configopaque.String
    imports:
    - go.opentelemetry.io/collector/config/configopaque

I suppose configoptional.Optional would work in a similar way.

[yurishkuro]

Sounds promising. Would be interesting to see example with Optional. I don't think the defaulting would work though.

Also, one of the primary objectives for me for schema first config was to be able to auto-generate documentation. Have you tried that?

[ptodev]

I added a simple example with Optional here. It is true that the default values wouldn't work with Optional. I didn't include defaults in my example either - for now it would either need to be something that's done in a separate helper function or I would need to think how to add support for it before the initial schema is merged to OTel's main branch.

The only solution I can think of is that go-jsonschema would need to own the Optional type, and it'd need to be something you could toggle through the goJSONSchema field:

goJSONSchema:
  useOptional: true

Then you'd be able to set the schema for the default value in the usual way, using the default field. I don't know if owning an Optional type would be ok for @omissis though. Maybe I can think of another way to do this.

Also, one of the primary objectives for me for schema first config was to be able to auto-generate documentation. Have you tried that?

Not yet, but it's on my todo list for next week.

It would help me a bit if we could clarify which bits of work would be necessary for me to complete prior to merging an initial PR? For example, would it be ok to schematise the batch processor as-is, or should we wait for Optional to be rolled out?

[github-actions[bot]]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions[bot]]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions[bot]]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

[github-actions[bot]]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions[bot]]

Closed as inactive. Feel free to reopen if this PR is still being worked on.