breaking: View selector and stream should be required

[jack-berg]

A view without a selector or stream is nonsensical.

[marcalff]

Makes sense.

C++ already treat these as required, sorry I missed this in the schema (the yaml parser is hand written, not generated)

[jack-berg]

C++ already treat these as required,

Likewise in java

sorry I missed this in the schema (the yaml parser is hand written, not generated)

I think there even with generated parser there are likely other oversights hiding in the schema. You've identified a number of important ones, and I've been finding the new meta schema / schema-docs.md work to be helpful. I'll try to do a couple more passes on every time in the schema to look for flaws.

[marcalff]

I think there even with generated parser there are likely other oversights hiding in the schema. You've identified a number of important ones, and I've been finding the new meta schema / schema-docs.md work to be helpful. I'll try to do a couple more passes on every time in the schema to look for flaws.

For this, #357 will help a lot.

If it's hard to write a reasonable description for what happens with a null field, maybe the field should be required.

[marcalff]

@jack-berg

I have a question about tooling and yaml schema ...

Is there a tool that:

• Given a schema as input
• Can produce a collection of valid yaml files used as test cases
• That stress the schema for every nodes: field present, field null, etc.

The generated yaml does not need to be semantically correct, for example an endpoint can be "value-for-endpoint".

The goal here is to feed this collection of yaml files to each language parser, as a stress test.

Code that expects a node to be required in the SDK while it may be null in the schema should crash then when building an SDK, exposing implementation bugs.

PS: We have a similar tool for MySQL, that generates random queries from a bison parser, doing the opposite of bison (take the grammar and generate stress tests, instead of parsing text using the grammar).

[codeboten]

This should be marked as breaking change correct?

[marcalff]

This should be marked as breaking change correct?

I think so. Do we have a label for that ?

[codeboten]

We mark things as breaking in the changelog, but we should probably be better about updating the changelog in the PR itself rather than putting this responsibility on the release process

[MrAlias]

LGTM. +1 to @codeboten's comment about needing to track this as a breaking change.

[jack-berg]

Yes we should track as a breaking change. Right now, I've been calling out breaking changes in the release notes and including migration notes for each occurrence, e.g: https://github.com/open-telemetry/opentelemetry-configuration/releases/tag/v0.4.0

I propose we add some additional lightweight structure around this:

• Create a tag which indicates a PR includes a breaking change
• Add release documentation that indicates that all breaking change PRs (as indicated with the label) must be called out in the release notes with migration steps