Skip to content

Conversation

@jsuereth
Copy link
Contributor

@jsuereth jsuereth commented Aug 14, 2025

Fixes unversioned file error messages, versioned files still get horrible results.

Example new (well old, but returned) error message:

Diagnostic report:

invalid_semconv_group

  × Missing required property: "brief".
   ╭─[/home/joshuasuereth/test/bad.yaml:3:5]
 1 │ my_bad_yaml: test
 2 │ groups:
 3 │   - id: test
   ·     ▲
   ·     ╰── somewhere in this block
 4 │     
   ╰────
  help: A group defines an attribute group, an entity, or a signal. Supported group types are: `attribute_group`, `span`, `event`, `metric`, `entity`, `scope`.
        Mandatory fields are: `id` and `brief`.
        
        Note: The `resource` type is no longer used and is an alias for `entity`.


invalid_semconv_group

  × Object contains unexpected properties: my_bad_yaml. These properties are not defined in the schema.
  help: A semantic convention file as defined [here](https://github.com/open-telemetry/build-tools/blob/main/semantic-conventions/syntax.md) A semconv file is a
        collection of semantic convention groups (i.e. [`GroupSpec`]).

However, once you add the version flag, you're back to bad error messages:

$ ../src/open-telemetry/weaver/target/debug/weaver registry check -r $(pwd)
Weaver Registry Check
Checking registry `/home/joshuasuereth/test`
ℹ No registry manifest found: /home/joshuasuereth/test/registry_manifest.yaml

Diagnostic report:

invalid_semconv_group

  × Value {"groups":[{"id":"test"}],"my_bad_yaml":"test","version":"1"} does not match any schema in a 'oneOf' group; it must match exactly one.
  help: A versioned semantic convention file.

or even worse:

$ ../src/open-telemetry/weaver/target/debug/weaver registry check -r $(pwd)
Weaver Registry Check
Checking registry `/home/joshuasuereth/test`
ℹ No registry manifest found: /home/joshuasuereth/test/registry_manifest.yaml

Diagnostic report:

  × data did not match any variant of untagged enum SemConvSpec

Will fix this up as part of V2 schema efforts.

@jsuereth jsuereth changed the title [DO NOT MERGE] Fix error messages for unversioned files. Fix error messages for unversioned files. version: ... syntax will need fixed up later Aug 14, 2025
@jsuereth jsuereth marked this pull request as ready for review August 14, 2025 22:30
@jsuereth jsuereth requested a review from a team as a code owner August 14, 2025 22:30
Copy link
Contributor

@jerbly jerbly left a comment

Choose a reason for hiding this comment

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

This makes sense to bring back the prior behaviour. Needs a changelog entry.

Copy link
Contributor

@lquerel lquerel left a comment

Choose a reason for hiding this comment

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

LGTM

@lquerel lquerel enabled auto-merge (squash) August 15, 2025 16:16
@lquerel lquerel merged commit d948c68 into open-telemetry:main Aug 15, 2025
20 checks passed
@jsuereth jsuereth deleted the wip-fix-error-messages-v2 branch August 15, 2025 16:22
@codecov
Copy link

codecov bot commented Aug 15, 2025

Codecov Report

❌ Patch coverage is 70.96774% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.9%. Comparing base (74d649b) to head (7f6aa47).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/weaver_semconv/src/semconv.rs 53.3% 7 Missing ⚠️
crates/weaver_semconv/src/registry.rs 50.0% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main    #880     +/-   ##
=======================================
+ Coverage   77.7%   77.9%   +0.2%     
=======================================
  Files         76      76             
  Lines       5936    5956     +20     
=======================================
+ Hits        4616    4644     +28     
+ Misses      1320    1312      -8     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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