Skip to content

Commit d948c68

Browse files
jsuerethlquerel
andauthored
Fix error messages for unversioned files. version: ... syntax will need fixed up later (#880)
* Fix error messages for unversioned files. * Fix error messages. * Added changelog. --------- Co-authored-by: Laurent Quérel <l.querel@f5.com>
1 parent 74d649b commit d948c68

File tree

5 files changed

+85
-34
lines changed

5 files changed

+85
-34
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
All notable changes to this project will be documented in this file.
44

5+
# Unreleased
6+
7+
- Fix error messages to ignore new version variants ([#880](https://github.com/open-telemetry/weaver/pull/880) by @jsuereth)
8+
59
# [0.17.0] - 2025-08-08
610

711
- Filter based on deprecation, stability, and annotations in signal JQ helpers

crates/weaver_resolver/src/lib.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,8 @@ impl SchemaResolver {
322322

323323
let local_path = registry_repo.path().to_path_buf();
324324
let registry_path_repr = registry_repo.registry_path_repr();
325-
let validator = JsonSchemaValidator::new();
325+
let versioned_validator = JsonSchemaValidator::new_versioned();
326+
let unversioned_validator = JsonSchemaValidator::new_unversioned();
326327

327328
// Loads the semantic convention specifications from the git repo.
328329
// All yaml files are recursively loaded and parsed in parallel from
@@ -342,7 +343,8 @@ impl SchemaResolver {
342343
vec![SemConvRegistry::semconv_spec_from_file(
343344
&registry_repo.id(),
344345
entry.path(),
345-
&validator,
346+
&unversioned_validator,
347+
&versioned_validator,
346348
|path| {
347349
// Replace the local path with the git URL combined with the relative path
348350
// of the semantic convention file.

crates/weaver_semconv/src/json_schema.rs

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22

33
//! JSON Schema validator for semantic convention files.
44
5-
use crate::semconv::SemConvSpec;
5+
use crate::semconv::{SemConvSpec, SemConvSpecV1, Versioned};
66
use crate::Error::{CompoundError, InvalidSemConvSpec, InvalidXPath};
77
use crate::{Error, InvalidSemConvSpecError};
88
use itertools::Itertools;
99
use jsonschema::error::{TypeKind, ValidationErrorKind};
1010
use jsonschema::{JsonType, JsonTypeSet};
1111
use miette::{NamedSource, SourceSpan};
1212
use saphyr::{LoadableYamlNode, MarkedYaml};
13+
use schemars::JsonSchema;
1314
use std::borrow::Cow;
1415

1516
/// Parsed simple XPath components
@@ -28,25 +29,36 @@ pub struct JsonSchemaValidator {
2829
validator: jsonschema::Validator,
2930
}
3031

31-
impl Default for JsonSchemaValidator {
32-
fn default() -> Self {
33-
Self::new()
34-
}
35-
}
36-
3732
impl JsonSchemaValidator {
3833
/// Creates a new JSON schema validator.
3934
#[must_use]
40-
pub fn new() -> Self {
35+
pub fn new_all_versions() -> Self {
36+
Self::new_for::<SemConvSpec>()
37+
}
38+
39+
/// Creates a new JSON schema validator that ONLY works when `version` is not specified.
40+
#[must_use]
41+
pub fn new_unversioned() -> Self {
42+
Self::new_for::<SemConvSpecV1>()
43+
}
44+
45+
/// Creates a new JSON schema validator that ONLY works when `version` is specified.
46+
#[must_use]
47+
pub fn new_versioned() -> Self {
48+
Self::new_for::<Versioned>()
49+
}
50+
51+
/// Creates a new JSON schema validator that works for any type T.
52+
#[must_use]
53+
fn new_for<T: JsonSchema>() -> Self {
4154
// Generate the JSON schema for the SemConvSpec struct using Schemars
42-
let root_schema = schemars::schema_for!(SemConvSpec);
55+
let root_schema = schemars::schema_for!(T);
4356
let json_schema = serde_json::to_value(&root_schema)
4457
// Should never happen as we expert Schemars to work
4558
.expect("Failed to convert schema to JSON value");
4659
let validator = jsonschema::Validator::new(&json_schema)
4760
// Should never happen as we expert Schemars to work
4861
.expect("Failed to create JSON schema validator");
49-
5062
JsonSchemaValidator {
5163
json_schema,
5264
validator,

crates/weaver_semconv/src/registry.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ impl SemConvRegistry {
8181
non_fatal_errors: &mut Vec<Error>,
8282
) -> Result<SemConvRegistry, Error> {
8383
let mut registry = SemConvRegistry::new(registry_id);
84-
let validator = JsonSchemaValidator::new();
84+
let versioned_validator = JsonSchemaValidator::new_versioned();
85+
let unversioned_validator = JsonSchemaValidator::new_unversioned();
8586
for sc_entry in
8687
glob::glob(path_pattern).map_err(|e| Error::InvalidRegistryPathPattern {
8788
path_pattern: path_pattern.to_owned(),
@@ -95,7 +96,8 @@ impl SemConvRegistry {
9596
let (semconv_spec, nfes) = SemConvSpecWithProvenance::from_file(
9697
registry_id,
9798
path_buf.as_path(),
98-
&validator,
99+
&unversioned_validator,
100+
&versioned_validator,
99101
)
100102
.into_result_with_non_fatal()?;
101103
registry.add_semconv_spec(semconv_spec);
@@ -203,7 +205,8 @@ impl SemConvRegistry {
203205
pub fn semconv_spec_from_file<P, F>(
204206
registry_id: &str,
205207
semconv_path: P,
206-
validator: &JsonSchemaValidator,
208+
unversioned_validator: &JsonSchemaValidator,
209+
versioned_validator: &JsonSchemaValidator,
207210
path_fixer: F,
208211
) -> WResult<SemConvSpecWithProvenance, Error>
209212
where
@@ -213,7 +216,8 @@ impl SemConvRegistry {
213216
SemConvSpecWithProvenance::from_file_with_mapped_path(
214217
registry_id,
215218
semconv_path,
216-
validator,
219+
unversioned_validator,
220+
versioned_validator,
217221
path_fixer,
218222
)
219223
}

crates/weaver_semconv/src/semconv.rs

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,16 @@ impl SemConvSpecWithProvenance {
250250
pub fn from_file<P: AsRef<Path>>(
251251
registry_id: &str,
252252
path: P,
253-
validator: &JsonSchemaValidator,
253+
unversioned_validator: &JsonSchemaValidator,
254+
versioned_validator: &JsonSchemaValidator,
254255
) -> WResult<SemConvSpecWithProvenance, Error> {
255-
Self::from_file_with_mapped_path(registry_id, path, validator, |path| path)
256+
Self::from_file_with_mapped_path(
257+
registry_id,
258+
path,
259+
unversioned_validator,
260+
versioned_validator,
261+
|path| path,
262+
)
256263
}
257264
/// Creates a semantic convention spec with provenance from a file.
258265
///
@@ -267,7 +274,8 @@ impl SemConvSpecWithProvenance {
267274
pub fn from_file_with_mapped_path<P, F>(
268275
registry_id: &str,
269276
path: P,
270-
validator: &JsonSchemaValidator,
277+
unversioned_validator: &JsonSchemaValidator,
278+
versioned_validator: &JsonSchemaValidator,
271279
path_fixer: F,
272280
) -> WResult<SemConvSpecWithProvenance, Error>
273281
where
@@ -277,7 +285,8 @@ impl SemConvSpecWithProvenance {
277285
fn from_file_or_fatal(
278286
path: &Path,
279287
provenance: &str,
280-
json_schema_validator: &JsonSchemaValidator,
288+
unversioned_validator: &JsonSchemaValidator,
289+
versioned_validator: &JsonSchemaValidator,
281290
) -> Result<SemConvSpec, Error> {
282291
use serde_yaml::Value;
283292
use std::io::Seek;
@@ -299,7 +308,18 @@ impl SemConvSpecWithProvenance {
299308
let original_error = e.to_string();
300309
let value: Result<Value, _> = serde_yaml::from_reader(&mut semconv_file);
301310
if let Ok(yaml_value) = value {
302-
json_schema_validator.validate_yaml(yaml_value, provenance, e)?;
311+
// TODO - Check if we should use versioned or unversioned validator.
312+
if yaml_value
313+
.as_mapping()
314+
.and_then(|m| m.get("version"))
315+
.map(|v| v.is_string())
316+
.unwrap_or(false)
317+
{
318+
// Use versioned validator.
319+
versioned_validator.validate_yaml(yaml_value, provenance, e)?;
320+
} else {
321+
unversioned_validator.validate_yaml(yaml_value, provenance, e)?;
322+
}
303323
}
304324

305325
// Fallback: return original serde error
@@ -312,7 +332,12 @@ impl SemConvSpecWithProvenance {
312332
}
313333
let path = path.as_ref().display().to_string();
314334
let provenance = Provenance::new(registry_id, &path_fixer(path.clone()));
315-
let raw_spec = match from_file_or_fatal(path.as_ref(), &path, validator) {
335+
let raw_spec = match from_file_or_fatal(
336+
path.as_ref(),
337+
&path,
338+
unversioned_validator,
339+
versioned_validator,
340+
) {
316341
Ok(semconv_spec) => {
317342
// Important note: the resolution process expects this step of validation to be done for
318343
// each semantic convention spec.
@@ -397,26 +422,29 @@ mod tests {
397422

398423
#[test]
399424
fn test_semconv_spec_from_file() {
400-
let validator = JsonSchemaValidator::new();
425+
let validator = JsonSchemaValidator::new_all_versions();
401426
// Existing file
402427
let path = PathBuf::from("data/database.yaml");
403428

404-
let semconv_spec = SemConvSpecWithProvenance::from_file("test", path, &validator)
405-
.into_result_failing_non_fatal()
406-
.unwrap();
429+
let semconv_spec =
430+
SemConvSpecWithProvenance::from_file("test", path, &validator, &validator)
431+
.into_result_failing_non_fatal()
432+
.unwrap();
407433
assert_eq!(semconv_spec.spec.into_v1("test").groups.len(), 10);
408434

409435
// Non-existing file
410436
let path = PathBuf::from("data/non-existing.yaml");
411-
let semconv_spec = SemConvSpecWithProvenance::from_file("test", path, &validator)
412-
.into_result_failing_non_fatal();
437+
let semconv_spec =
438+
SemConvSpecWithProvenance::from_file("test", path, &validator, &validator)
439+
.into_result_failing_non_fatal();
413440
assert!(semconv_spec.is_err());
414441
assert!(matches!(semconv_spec.unwrap_err(), RegistryNotFound { .. }));
415442

416443
// Invalid file structure
417444
let path = PathBuf::from("data/invalid/invalid-semconv.yaml");
418-
let semconv_spec = SemConvSpecWithProvenance::from_file("test", path, &validator)
419-
.into_result_failing_non_fatal();
445+
let semconv_spec =
446+
SemConvSpecWithProvenance::from_file("test", path, &validator, &validator)
447+
.into_result_failing_non_fatal();
420448
assert!(semconv_spec.is_err());
421449
assert!(matches!(
422450
semconv_spec.unwrap_err(),
@@ -604,11 +632,12 @@ mod tests {
604632

605633
#[test]
606634
fn test_semconv_spec_with_provenance_from_file() {
607-
let validator = JsonSchemaValidator::new();
635+
let validator = JsonSchemaValidator::new_all_versions();
608636
let path = PathBuf::from("data/database.yaml");
609-
let semconv_spec = SemConvSpecWithProvenance::from_file("main", &path, &validator)
610-
.into_result_failing_non_fatal()
611-
.unwrap();
637+
let semconv_spec =
638+
SemConvSpecWithProvenance::from_file("main", &path, &validator, &validator)
639+
.into_result_failing_non_fatal()
640+
.unwrap();
612641
assert_eq!(semconv_spec.spec.into_v1("test").groups.len(), 10);
613642
assert_eq!(semconv_spec.provenance.path, path.display().to_string());
614643
}

0 commit comments

Comments
 (0)