Skip to content
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

Generate JSON Schema for both Resolved Telemetry Schema and Resolved Registry #187

Merged
merged 10 commits into from
Jun 3, 2024

Conversation

lquerel
Copy link
Contributor

@lquerel lquerel commented May 30, 2024

This PR uses schemars to automatically generate JSON schema from Serde-annotated Rust structs.

This PR will serve as a basis for future discussions and the generation of documentation on the topic of resolved registry and schema.

Two types of resolved assets coexist:

  • Resolved Registry: A resolved view of a semantic convention registry. This is basically a resolved/flattened version of the semantic convention files. The command weaver registry resolve can be used to generate this type of resolved asset. The command weaver registry json-schema generates the JSON Schema representation of a ResolvedRegistry data structure.
  • Resolved Telemetry Schema: An extension of the existing Telemetry Schema containing a catalog of attributes, registries, etc., in addition to the original versions section. This type of schema is still in discussion.

This PR also improves the output of commands such as weaver registry resolve, which can now be directly piped to a jq command without having to use the --quiet parameter (see #189).

Note 1: TemplateRegistry and TemplateGroup have been renamed to ResolvedRegistry and ResolvedGroup because it's a more logical name. This renaming has no impact on the templates and the policies. I might move these definitions into a dedicated crate.

Note 2: A conversation has been initiated to talk about resolved registry and resolved telemetry schema, see https://cloud-native.slack.com/archives/C0697EXNTL3/p1717183349294419?thread_ts=1717104224.284179&cid=C0697EXNTL3

lquerel added 2 commits May 29, 2024 16:00
…solvedRegistry and ResolvedTelemetrySchema

TemplateRegistry had been renamed ResolvedRegistry.
@lquerel lquerel self-assigned this May 30, 2024
@lquerel lquerel added the enhancement New feature or request label May 30, 2024
Copy link

codecov bot commented May 30, 2024

Codecov Report

Attention: Patch coverage is 19.23077% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 74.5%. Comparing base (45a5338) to head (9df9a42).

Files Patch % Lines
crates/weaver_common/src/lib.rs 9.5% 19 Missing ⚠️
crates/weaver_common/src/in_memory.rs 0.0% 1 Missing ⚠️
crates/weaver_common/src/quiet.rs 0.0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main    #187     +/-   ##
=======================================
- Coverage   75.0%   74.5%   -0.6%     
=======================================
  Files         44      44             
  Lines       2871    2889     +18     
=======================================
- Hits        2156    2155      -1     
- Misses       715     734     +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/main.rs Fixed Show fixed Hide fixed
src/main.rs Fixed Show fixed Hide fixed
src/registry/stats.rs Fixed Show fixed Hide fixed
src/registry/stats.rs Fixed Show fixed Hide fixed
src/registry/stats.rs Fixed Show fixed Hide fixed
src/schema/resolve.rs Fixed Show fixed Hide fixed
src/schema/resolve.rs Fixed Show fixed Hide fixed
src/schema/resolve.rs Fixed Show fixed Hide fixed
src/schema/resolve.rs Fixed Show fixed Hide fixed
src/schema/resolve.rs Fixed Show fixed Hide fixed
@lquerel lquerel changed the title [WIP] Generate JSON Schema for both Resolved Telemetry Schema and Resolved Registry Generate JSON Schema for both Resolved Telemetry Schema and Resolved Registry May 30, 2024
@lquerel lquerel marked this pull request as ready for review May 30, 2024 21:22
@lquerel lquerel requested a review from jsuereth as a code owner May 30, 2024 21:22
@@ -47,6 +47,9 @@ pub trait Logger {

/// Logs a message without icon.
fn log(&self, message: &str);

/// Mute all the messages except for the warnings and errors.
fn mute(&self);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Introduced this new function to fix #184 and to simplify testing.

@@ -3,7 +3,7 @@
//! Initializes a `diagnostic_templates` directory to define or override diagnostic output formats.

use crate::diagnostic::{Error, DEFAULT_DIAGNOSTIC_TEMPLATES};
use crate::DiagnosticArgs;
use crate::{DiagnosticArgs, ExitDirectives};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ExitDirectives is a way for commands to inform the main program on how it should behave once the command has been executed (e.g., suggested exit code, quiet mode).


/// Supported output formats for the resolved schema
#[derive(Debug, Clone, ValueEnum)]
pub(crate) enum Format {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved Format in a more global location as this structure is now used from multiple sub-commands.

@@ -78,7 +65,10 @@ pub(crate) fn command(
logger: impl Logger + Sync + Clone,
cache: &Cache,
args: &RegistryResolveArgs,
) -> Result<(), DiagnosticMessages> {
) -> Result<ExitDirectives, DiagnosticMessages> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now this command can only display the resolved registry. See weaver schema resolve to display the resolved telemetry schema (this part is still experimental).

Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

Are we declaring the difference between schema + registry the difference between using optimisation on attributes vs. not?

At this point, I'm still a bit confused on terminology and I think maybe we should discuss that first.

Also have a minor nit about threading choices.

crates/weaver_common/src/lib.rs Outdated Show resolved Hide resolved
crates/weaver_common/src/lib.rs Outdated Show resolved Hide resolved
src/schema/mod.rs Outdated Show resolved Hide resolved
@lquerel
Copy link
Contributor Author

lquerel commented May 31, 2024

At this point, I'm still a bit confused on terminology and I think maybe we should discuss that first.

Yes, I think the best way to discuss this is through chat or via Zoom.

@lquerel lquerel merged commit 7c6e66b into open-telemetry:main Jun 3, 2024
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants