Skip to content

Conversation

@jack-berg
Copy link
Member

Alternative solution for #398 based off conversation in #407 and #400

Idea:

  • Define JSON schema files in YAML instead of JSON, for better multi line editing
  • This will allow us to merge meta_schema_types.yaml into the schema files so everything is in one place
  • Add build tooling to convert YAML source schema files to JSON output schema files, and adjust build validation to work against the JSON files
  • On release, publish the JSON version of the schema files

If we like this idea I would follow up with a PR to merge meta_schema_types.yaml into the source YAML files, and update all project tooling, documentation, etc.

@jack-berg jack-berg force-pushed the yaml-source-of-truth branch from c33083a to 0394f95 Compare November 14, 2025 22:45
@jack-berg jack-berg force-pushed the yaml-source-of-truth branch from 0394f95 to 8e73ca3 Compare November 14, 2025 22:48
@jack-berg jack-berg marked this pull request as ready for review November 17, 2025 21:06
@jack-berg jack-berg requested a review from a team as a code owner November 17, 2025 21:06
@jack-berg
Copy link
Member Author

Marking ready for review after seeing #400 and #396 closed

<summary>JSON Schema</summary>

[JSON Schema Source File](./schema/meter_provider.json)
[JSON Schema Source File](./schema/meter_provider.yaml)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun fact - you can tell its a strict refactoring of build tooling because schema-docs.md basically doesn't change besides this trivial bit pointing to the new source location.

@marcalff
Copy link
Member

Ok in principle.

The concern is to avoid merge collisions if another PR changes the schema in the mean time, but the generated schema-docs.md file serves as a verification for that, so it looks safe.

Object.keys(fileContentByFile).forEach(file => {
const jsonFile = file.replace('.yaml', '.json');
Object.entries(fileContentByFile).forEach(([otherFile, otherContent]) => {
fileContentByFile[otherFile] = otherContent.replaceAll(`$ref: ${file}`, `$ref: ${jsonFile}`);
Copy link
Member Author

@jack-berg jack-berg Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that maintaining "pre compiled" source for JSON schema definitions complicates the picture for $refs - do refs point at the .yaml or .json (which doesn't exist until you run make compile-schema?

Solved this by adjusting all $refs to point .yaml for a more intuitive editing experience, and replacing .yaml to .json as a part of the process of writing out the JSON schema files.

At the same time, I realized this is a good place to force standardization for the $id and $schema properties each file needs to contain. (see below)

But this got me thinking and worrying (once again) about $id and resolving $refs for tooling. Once solution would be to publish a single .json file containing all of the types in the schema, eliminating the need to do $ref resolution. A single file is also more convenient than a directory, since there's no need to zip it (and unzip it after download). Of course the downside is that one big file with all the types in it is cumbersome to maintain, but.. 1. its not that bad 2. we can improve things with tooling (i.e. lexographical sorting so types are where you expect them).

@jack-berg jack-berg merged commit 01bcc9c into open-telemetry:main Nov 19, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants