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

[WIP] Registry manifest and Schema diff #400

Draft
wants to merge 35 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
edab834
feat(semconv): create registry manifest
lquerel Oct 3, 2024
fe7d7b5
feat(schema): Parse OTEL schema 1.1
lquerel Oct 4, 2024
76db2aa
feat(cli): Add registry diff command (WIP)
lquerel Oct 4, 2024
ad77910
Merge branch 'main' into generate-otel-schema
lquerel Oct 4, 2024
6de07ee
feat(schema): Support new deprecated field format
lquerel Oct 17, 2024
5efe6b1
Merge remote-tracking branch 'upstream/main' into generate-otel-schema
lquerel Oct 23, 2024
6cdfbfa
feat(diff): Add registry diff command.
lquerel Oct 23, 2024
880649f
feat(diff): Add diff for metrics
lquerel Oct 24, 2024
a87807b
feat(diff): Add diff for events, spans, and resources
lquerel Oct 24, 2024
8f8bdaf
feat(diff): Add template rendering capability
lquerel Oct 25, 2024
a818f4e
feat(diff): Add diff-format for ansi, ansi-stats, json, and yaml
lquerel Oct 28, 2024
05256ea
feat(diff): Add diff-format for markdown
lquerel Oct 29, 2024
c584f30
chore(diff): Fix unit tests
lquerel Oct 29, 2024
374def2
feat(cli): Prepare registry update-schema command
lquerel Oct 30, 2024
1b7c9c2
Merge branch 'main' into generate-otel-schema
lquerel Oct 30, 2024
2abd024
feat(cli): Prepare registry update-schema command
lquerel Oct 30, 2024
83f45eb
feat(cli): Prepare registry update-schema command
lquerel Oct 31, 2024
6312e42
chore(typo): fix typo in comment
lquerel Dec 2, 2024
6532bf3
Merge remote-tracking branch 'upstream/main' into generate-otel-schema
lquerel Dec 2, 2024
f5efacc
chore(build): Resolve merging with main upstream
lquerel Dec 2, 2024
9bda8cb
chore(build): Clean up code to prepare PR
lquerel Dec 2, 2024
edf85b0
chore(build): Clean up code to prepare PR
lquerel Dec 2, 2024
3b41c58
Merge branch 'main' into generate-otel-schema
lquerel Dec 6, 2024
bde3eef
chore(build): Prepare resolve_telemetry_schema to return WResult
lquerel Dec 6, 2024
904e856
Merge branch 'generate-otel-schema' of https://github.com/lquerel/ote…
lquerel Dec 6, 2024
3c4b6dd
chore(build): Fix `registry diff` to return a diff even when there ar…
lquerel Dec 9, 2024
c0609b5
Merge branch 'open-telemetry:main' into generate-otel-schema
lquerel Dec 17, 2024
6ba427c
chore(merge): Merge with main branch
lquerel Dec 17, 2024
ebcd15f
feat(diff): Test schema diff feature
lquerel Dec 19, 2024
8dcd311
feat(diff): Remove complex logic infering the type of deprecation bas…
lquerel Dec 19, 2024
b58d200
feat(diff): Rename the field Deprecated::Renamed::new_name into Depre…
lquerel Dec 19, 2024
a1ab93a
chore(doc): Fix documentation of the ResolvedTelemetrySchema::groups …
lquerel Dec 19, 2024
396450f
chore(doc): Fix documentation based on @jsuereth feedback
lquerel Dec 19, 2024
12c79c2
Merge remote-tracking branch 'upstream/main' into generate-otel-schema
lquerel Dec 19, 2024
0a63cd8
chore(merge): Merge with upstream main branch
lquerel Dec 19, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ weaver_semconv_gen = { path = "crates/weaver_semconv_gen" }
weaver_cache = { path = "crates/weaver_cache" }
weaver_forge = { path = "crates/weaver_forge" }
weaver_checker = { path = "crates/weaver_checker" }
weaver_otel_schema = { path = "crates/weaver_otel_schema" }

clap = { version = "4.5.23", features = ["derive"] }
rayon = "1.10.0"
Expand Down
15 changes: 14 additions & 1 deletion crates/weaver_cache/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
const TAR_GZ_EXT: &str = ".tar.gz";
/// The extension for a zip archive.
const ZIP_EXT: &str = ".zip";
/// The name of the registry manifest file.
const REGISTRY_MANIFEST: &'static str = "registry_manifest.yaml";
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed

/// An error that can occur while creating or using a cache.
#[derive(thiserror::Error, Debug, Clone, Serialize, Diagnostic)]
Expand Down Expand Up @@ -99,7 +101,7 @@
/// - A simple wrapper around a local directory
/// - Initialized from a Git repository
/// - Initialized from a Git archive
#[derive(Default)]
#[derive(Default, Debug)]
pub struct RegistryRepo {
// A unique identifier for the registry (e.g. main, baseline, etc.)
id: String,
Expand Down Expand Up @@ -509,6 +511,17 @@
&self.registry_path
}

/// Returns the path to the `registry_manifest.yaml` file (if any).
#[must_use]
pub fn manifest_path(&self) -> Option<PathBuf> {
let manifest_path = self.path.join(REGISTRY_MANIFEST);
if manifest_path.exists() {
Some(manifest_path)
} else {
None
}
}

/// Creates a temporary directory for the registry repository and returns the path.
/// The temporary directory is created in the `.weaver/semconv_registry_cache`.
fn create_tmp_repo() -> Result<TempDir, Error> {
Expand Down
14 changes: 5 additions & 9 deletions crates/weaver_codegen_test/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use weaver_semconv::registry::SemConvRegistry;

const SEMCONV_REGISTRY_PATH: &str = "./semconv_registry/";
const TEMPLATES_PATH: &str = "./templates/registry/";
const REGISTRY_ID: &str = "test";
const TARGET: &str = "rust";
const FOLLOW_SYMLINKS: bool = false;

Expand All @@ -50,7 +49,8 @@ fn main() {
.ignore(|e| matches!(e.severity(), Some(miette::Severity::Warning)))
.into_result_failing_non_fatal()
.unwrap_or_else(|e| process_error(&logger, e));
let mut registry = SemConvRegistry::from_semconv_specs(REGISTRY_ID, semconv_specs);
let mut registry = SemConvRegistry::from_semconv_specs(&registry_repo, semconv_specs)
.unwrap_or_else(|e| process_error(&logger, e));
let schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry)
.unwrap_or_else(|e| process_error(&logger, e));

Expand All @@ -62,13 +62,9 @@ fn main() {
engine
.import_jq_package(SEMCONV_JQ)
.unwrap_or_else(|e| process_error(&logger, e));
let template_registry = ResolvedRegistry::try_from_resolved_registry(
schema
.registry(REGISTRY_ID)
.expect("Failed to get the registry from the resolved schema"),
schema.catalog(),
)
.unwrap_or_else(|e| process_error(&logger, e));
let template_registry =
ResolvedRegistry::try_from_resolved_registry(&schema.registry, schema.catalog())
.unwrap_or_else(|e| process_error(&logger, e));
let target_dir: PathBuf = target_dir.into();
engine
.generate(
Expand Down
3 changes: 1 addition & 2 deletions crates/weaver_common/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ impl DiagnosticMessages {
Self(vec![DiagnosticMessage::new(error)])
}

/// Returns true if all the diagnostic messages are explicitly marked as
/// warnings or advices.
/// Returns true if at least one diagnostic message has an error severity.
#[must_use]
pub fn has_error(&self) -> bool {
let non_error_count = self
Expand Down
41 changes: 18 additions & 23 deletions crates/weaver_forge/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,13 +755,12 @@ mod tests {
let registry = SemConvRegistry::try_from_path_pattern(registry_id, "data/*.yaml")
.into_result_failing_non_fatal()
.expect("Failed to load registry");
prepare_test_with_registry(target, cli_params, registry_id, registry)
prepare_test_with_registry(target, cli_params, registry)
}

fn prepare_test_with_registry(
target: &str,
cli_params: Params,
registry_id: &str,
mut registry: SemConvRegistry,
) -> (
TestLogger,
Expand All @@ -779,16 +778,14 @@ mod tests {
let schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry)
.expect("Failed to resolve registry");

let template_registry = ResolvedRegistry::try_from_resolved_registry(
schema.registry(registry_id).expect("registry not found"),
schema.catalog(),
)
.unwrap_or_else(|e| {
panic!(
"Failed to create the context for the template evaluation: {:?}",
e
)
});
let template_registry =
ResolvedRegistry::try_from_resolved_registry(&schema.registry, schema.catalog())
.unwrap_or_else(|e| {
panic!(
"Failed to create the context for the template evaluation: {:?}",
e
)
});

// Delete all the files in the observed_output/target directory
// before generating the new files.
Expand Down Expand Up @@ -959,16 +956,14 @@ mod tests {
let schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry)
.expect("Failed to resolve registry");

let template_registry = ResolvedRegistry::try_from_resolved_registry(
schema.registry(registry_id).expect("registry not found"),
schema.catalog(),
)
.unwrap_or_else(|e| {
panic!(
"Failed to create the context for the template evaluation: {:?}",
e
)
});
let template_registry =
ResolvedRegistry::try_from_resolved_registry(&schema.registry, schema.catalog())
.unwrap_or_else(|e| {
panic!(
"Failed to create the context for the template evaluation: {:?}",
e
)
});

engine
.generate(
Expand Down Expand Up @@ -1093,7 +1088,7 @@ mod tests {
.into_result_failing_non_fatal()
.expect("Failed to load registry");
let (logger, engine, template_registry, observed_output, expected_output) =
prepare_test_with_registry("comment_format", Params::default(), registry_id, registry);
prepare_test_with_registry("comment_format", Params::default(), registry);

engine
.generate(
Expand Down
3 changes: 2 additions & 1 deletion crates/weaver_forge/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use weaver_resolved_schema::catalog::Catalog;
use weaver_resolved_schema::lineage::GroupLineage;
use weaver_resolved_schema::registry::{Constraint, Group, Registry};
use weaver_semconv::any_value::AnyValueSpec;
use weaver_semconv::deprecated::Deprecated;
use weaver_semconv::group::{GroupType, InstrumentSpec, SpanKindSpec};
use weaver_semconv::stability::Stability;

Expand Down Expand Up @@ -63,7 +64,7 @@ pub struct ResolvedGroup {
/// provided as `description` MUST specify why it's deprecated and/or what
/// to use instead. See also stability.
#[serde(skip_serializing_if = "Option::is_none")]
pub deprecated: Option<String>,
pub deprecated: Option<Deprecated>,
/// Additional constraints.
/// Allow to define additional requirements on the semantic convention.
/// It defaults to an empty list.
Expand Down
2 changes: 1 addition & 1 deletion crates/weaver_forge/templates/test/resource.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Brief: {{ resource.brief }}
- Sampling relevant: {{ attribute.sampling_relevant }}
{%- endif %}
{%- if attribute.deprecated %}
- Deprecated: {{ attribute.deprecated }}
- Deprecated: {{ attribute.deprecated.note }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is/will this be a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is a breaking change at the rendering level because, without the .note, Jinja renders the new JSON of the deprecated field instead of the text that now corresponds to the note. I couldn’t find another way to handle this at the rendering level. However, at the semconv parsing level, it is not a breaking change since both formats remain supported.

Copy link
Contributor

Choose a reason for hiding this comment

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

Related comment: https://github.com/open-telemetry/weaver/pull/400/files#r1868713601 ( I thought we'd only keep the note on the attribute without introducing one on deprecated).

But if we do have deprecated.note, it'd still optional thing that does not need to describe the action.

I think codegen would need to generate something like

/*
 * ...
 * @deprecated renamed to {@link
 *     io.opentelemetry.semconv.FooAttributes#FOO_BAR}.
 *  Here's an additional note.
 */

from

deprecated: 
  action: renamed
  new_name: foo.bar
note: Here's an additional note.

so maybe we should eventually provide some helper filters to render those.

In the scope of this PR, if someone uses {{ attr.deprecated }}, I wonder if jinja can call to_string on the original deprecated object to return a custom string? We'd format it similarly to "Renamed to foo.bar", but someone who wants to use new structured info would be able to obtain it with attr.deprecated.action, etc

WDYT?

{%- endif %}
{% if attribute.stability %}
- Stability: {{ attribute.stability | capitalize }}
Expand Down
2 changes: 1 addition & 1 deletion crates/weaver_forge/templates/test/resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Brief: {{ resource.brief }}
- Sampling relevant: {{ attribute.sampling_relevant }}
{%- endif %}
{%- if attribute.deprecated %}
- Deprecated: {{ attribute.deprecated }}
- Deprecated: {{ attribute.deprecated.note }}
{%- endif %}
{% if attribute.stability %}
- Stability: {{ attribute.stability | capitalize }}
Expand Down
21 changes: 21 additions & 0 deletions crates/weaver_otel_schema/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[package]
name = "weaver_otel_schema"
version = "0.10.0"
authors.workspace = true
repository.workspace = true
license.workspace = true
publish.workspace = true
edition.workspace = true
rust-version.workspace = true

[lints]
workspace = true

[dependencies]
weaver_version = { path = "../weaver_version" }
weaver_common = { path = "../weaver_common" }

thiserror.workspace = true
serde.workspace = true
serde_yaml.workspace = true
miette.workspace = true
3 changes: 3 additions & 0 deletions crates/weaver_otel_schema/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# OpenTelemetry Schema Data Model

This crate describes the data model for the OpenTelemetry telemetry schema.
6 changes: 6 additions & 0 deletions crates/weaver_otel_schema/allowed-external-types.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0
# This is used with cargo-check-external-types to reduce the surface area of downstream crates from
# the public API. Ideally this can have a few exceptions as possible.
allowed_external_types = [
]
93 changes: 93 additions & 0 deletions crates/weaver_otel_schema/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// SPDX-License-Identifier: Apache-2.0

//! OpenTelemetry Schema Definitions
//! Please refer to the [OpenTelemetry Schema OTEP](https://github.com/open-telemetry/oteps/blob/main/text/0152-telemetry-schemas.md)
//! for more information.

use crate::Error::{InvalidTelemetrySchema, TelemetrySchemaNotFound};
use miette::Diagnostic;
use serde::{Deserialize, Serialize};
use weaver_common::diagnostic::{DiagnosticMessage, DiagnosticMessages};
use weaver_version::Versions;

/// Errors emitted by this crate.
#[derive(thiserror::Error, Debug, Clone, Deserialize, Serialize, Diagnostic)]
pub enum Error {
/// OTel Telemetry schema not found.
#[error("OTel telemetry schema not found (path_or_url: {path_or_url:?}).")]
TelemetrySchemaNotFound {
/// The path or the url to the telemetry schema file.
path_or_url: String,
},

/// Invalid OTel Telemetry schema.
#[error("Invalid OTel telemetry schema (path_or_url: {path_or_url:?}). {error}")]
InvalidTelemetrySchema {
/// The path or the url to the telemetry schema file.
path_or_url: String,
/// The error that occurred.
error: String,
},
}

impl From<Error> for DiagnosticMessages {
fn from(error: Error) -> Self {
DiagnosticMessages::new(vec![DiagnosticMessage::new(error)])
}
}

/// An OpenTelemetry Telemetry Schema.
#[derive(Serialize, Deserialize, Debug)]
#[serde(deny_unknown_fields)]
pub struct TelemetrySchema {
/// Version of the file structure.
pub file_format: String,
/// Schema URL that this file is published at.
pub schema_url: String,
/// Definitions for each schema version in this family.
/// Note: the ordering of versions is defined according to semver
/// version number ordering rules.
/// This section is described in more details in the OTEP 0152 and in a dedicated
/// section below.
/// <https://github.com/open-telemetry/oteps/blob/main/text/0152-telemetry-schemas.md>
#[serde(skip_serializing_if = "Option::is_none")]
pub versions: Option<Versions>,
}

impl TelemetrySchema {
/// Attempts to load a telemetry schema from a file.
pub fn try_from_file<P: AsRef<std::path::Path>>(path: P) -> Result<Self, Error> {
let schema_path_buf = path.as_ref().to_path_buf();

if !schema_path_buf.exists() {
return Err(TelemetrySchemaNotFound {
path_or_url: schema_path_buf.as_path().to_string_lossy().to_string(),
});
}

let file = std::fs::File::open(path).map_err(|e| InvalidTelemetrySchema {
path_or_url: schema_path_buf.as_path().to_string_lossy().to_string(),
error: e.to_string(),
})?;
let reader = std::io::BufReader::new(file);
let schema: TelemetrySchema =
serde_yaml::from_reader(reader).map_err(|e| InvalidTelemetrySchema {
path_or_url: schema_path_buf.as_path().to_string_lossy().to_string(),
error: e.to_string(),
})?;

Ok(schema)
}
}

#[cfg(test)]
mod tests {
use crate::TelemetrySchema;

#[test]
fn test_try_from_file() {
let schema = TelemetrySchema::try_from_file("tests/test_data/1.27.0.yaml").unwrap();
assert_eq!(schema.file_format, "1.1.0");
assert_eq!(schema.schema_url, "https://opentelemetry.io/schemas/1.27.0");
}
}
Loading
Loading