From 255982052cb59d056ca8424e1cef5d609c474709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laurent=20Qu=C3=A9rel?= Date: Fri, 23 Aug 2024 08:37:28 -0700 Subject: [PATCH] Refactor handling of non-fatal errors, including warnings. (#337) * chore(nfes): Refactor handling of non-fatal errors, including warnings. * chore(nfes): Remove unused method * chore(nfes): Improve code doc * chore(nfes): Move WResult to a 3 variants enum for more clarity * chore(nfes): Rename some method names to be more explicit * chore(nfes): Propagate WResult in validation process and policy enforcement * chore(build): Bump weaver from v0.9.0 to v0.9.1 * feat(common): Make ignore_warnings more generic. * chore(build): Fix Cargo.lock issue that occurred after automatic merge in GH --- CHANGELOG.md | 8 + Cargo.lock | 154 +++++++----- Cargo.toml | 2 +- crates/weaver_cache/Cargo.toml | 2 +- crates/weaver_checker/Cargo.toml | 2 +- crates/weaver_codegen_test/Cargo.toml | 5 +- crates/weaver_codegen_test/build.rs | 3 + crates/weaver_common/Cargo.toml | 2 +- crates/weaver_common/src/diagnostic.rs | 12 + crates/weaver_common/src/lib.rs | 1 + crates/weaver_common/src/result.rs | 233 ++++++++++++++++++ crates/weaver_diff/Cargo.toml | 2 +- crates/weaver_forge/Cargo.toml | 2 +- crates/weaver_forge/README.md | 2 +- crates/weaver_forge/src/lib.rs | 3 + crates/weaver_resolved_schema/Cargo.toml | 2 +- crates/weaver_resolver/Cargo.toml | 2 +- crates/weaver_resolver/src/lib.rs | 71 ++---- crates/weaver_resolver/src/registry.rs | 5 +- crates/weaver_semconv/Cargo.toml | 2 +- .../allowed-external-types.toml | 2 +- crates/weaver_semconv/src/attribute.rs | 164 +++++++++--- crates/weaver_semconv/src/group.rs | 81 +++--- crates/weaver_semconv/src/lib.rs | 17 +- crates/weaver_semconv/src/registry.rs | 88 ++++--- crates/weaver_semconv/src/semconv.rs | 174 +++++++------ crates/weaver_semconv_gen/Cargo.toml | 2 +- crates/weaver_semconv_gen/src/lib.rs | 3 +- crates/weaver_version/Cargo.toml | 2 +- justfile | 3 +- src/registry/check.rs | 89 ++++--- src/registry/generate.rs | 25 +- src/registry/resolve.rs | 25 +- src/registry/search.rs | 4 +- src/registry/stats.rs | 5 +- src/util.rs | 79 ++---- test_data/compatibility_check.rego | 2 +- tests/resolution_process.rs | 11 +- 38 files changed, 876 insertions(+), 415 deletions(-) create mode 100644 crates/weaver_common/src/result.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ce986aff..a24ef223 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to this project will be documented in this file. +## [0.9.1] - 2024-08-22 + +Fixes + +* Warnings detected in the baseline registry are now ignored and non-fatal errors will not + interrupt any command before it completes + ([#337](https://github.com/open-telemetry/weaver/pull/337) by lquerel). + ## [0.9.0] - 2024-08-19 What's Changed diff --git a/Cargo.lock b/Cargo.lock index 00517bcc..76a01403 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -150,9 +150,9 @@ checksum = "69f7f8c3906b62b754cd5326047894316021dcfe5a194c8ea52bdd94934a3457" [[package]] name = "arrayvec" -version = "0.7.4" +version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" +checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" [[package]] name = "assert_cmd" @@ -244,12 +244,6 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "349f9b6a179ed607305526ca489b34ad0a41aed5f7980fa90eb03160b69598fb" -[[package]] -name = "bitflags" -version = "1.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" - [[package]] name = "bitflags" version = "2.6.0" @@ -347,9 +341,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.1.13" +version = "1.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72db2f7947ecee9b03b510377e8bb9077afa27176fdbff55c51027e976fdcc48" +checksum = "50d2eb3cd3d1bf4529e31c215ee6f93ec5a3d536d9f578f93d9d33ee19562932" dependencies = [ "jobserver", "libc", @@ -608,7 +602,7 @@ version = "0.28.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "829d955a0bb380ef178a640b91779e3987da38c9aea133b20614cfed8cdea9c6" dependencies = [ - "bitflags 2.6.0", + "bitflags", "crossterm_winapi", "mio", "parking_lot", @@ -1100,7 +1094,7 @@ version = "0.14.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "03f76169faa0dec598eac60f83d7fcdd739ec16596eca8fb144c88973dbe6f8c" dependencies = [ - "bitflags 2.6.0", + "bitflags", "bstr", "gix-path", "libc", @@ -1225,7 +1219,7 @@ version = "0.16.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74908b4bbc0a0a40852737e5d7889f676f081e340d5451a16e5b4c50d592f111" dependencies = [ - "bitflags 2.6.0", + "bitflags", "bstr", "gix-features", "gix-path", @@ -1271,7 +1265,7 @@ version = "0.35.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0cd4203244444017682176e65fd0180be9298e58ed90bd4a8489a357795ed22d" dependencies = [ - "bitflags 2.6.0", + "bitflags", "bstr", "filetime", "fnv", @@ -1310,7 +1304,7 @@ version = "0.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4063bf329a191a9e24b6f948a17ccf6698c0380297f5e169cee4f1d2ab9475b" dependencies = [ - "bitflags 2.6.0", + "bitflags", "gix-commitgraph", "gix-date", "gix-hash", @@ -1423,7 +1417,7 @@ version = "0.7.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d23bf239532b4414d0e63b8ab3a65481881f7237ed9647bb10c1e3cc54c5ceb" dependencies = [ - "bitflags 2.6.0", + "bitflags", "bstr", "gix-attributes", "gix-config-value", @@ -1544,7 +1538,7 @@ version = "0.10.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fe4d52f30a737bbece5276fab5d3a8b276dc2650df963e293d0673be34e7a5f" dependencies = [ - "bitflags 2.6.0", + "bitflags", "gix-path", "libc", "windows-sys 0.52.0", @@ -1567,9 +1561,9 @@ dependencies = [ [[package]] name = "gix-tempfile" -version = "14.0.1" +version = "14.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "006acf5a613e0b5cf095d8e4b3f48c12a60d9062aa2b2dd105afaf8344a5600c" +checksum = "046b4927969fa816a150a0cda2e62c80016fe11fb3c3184e4dddf4e542f108aa" dependencies = [ "gix-fs", "libc", @@ -1609,7 +1603,7 @@ version = "0.41.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "030da39af94e4df35472e9318228f36530989327906f38e27807df305fccb780" dependencies = [ - "bitflags 2.6.0", + "bitflags", "gix-commitgraph", "gix-date", "gix-hash", @@ -1715,9 +1709,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.4.5" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa82e28a107a8cc405f0839610bdc9b15f1e25ec7d696aa5cf173edbcb1486ab" +checksum = "524e8ac6999421f49a846c2d4411f337e53497d8ec55d67753beffa43c5d9205" dependencies = [ "atomic-waker", "bytes", @@ -2193,9 +2187,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" [[package]] name = "libc" -version = "0.2.156" +version = "0.2.158" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a5f43f184355eefb8d17fc948dbecf6c13be3c141f20d834ae842193a448c72a" +checksum = "d8adc4bb1803a324070e64a98ae98f38934d91957a99cfb3a43dcbc01bc56439" [[package]] name = "libm" @@ -2209,7 +2203,7 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" dependencies = [ - "bitflags 2.6.0", + "bitflags", "libc", "redox_syscall", ] @@ -2875,9 +2869,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.36" +version = "1.0.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fa76aaf39101c457836aec0ce2316dbdc3ab723cdda1c6bd4e6ad4208acaca7" +checksum = "b5b9d34b8991d19d98081b46eacdd8eb58c6f2b201139f7c5f643cc155a633af" dependencies = [ "proc-macro2", ] @@ -2948,7 +2942,7 @@ version = "0.28.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5ba6a365afbe5615999275bea2446b970b10a41102500e27ce7678d50d978303" dependencies = [ - "bitflags 2.6.0", + "bitflags", "cassowary", "compact_str", "crossterm", @@ -2999,14 +2993,14 @@ version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a908a6e00f1fdd0dfd9c0eb08ce85126f6d8bbda50017e74bc4a4b7d4a926a4" dependencies = [ - "bitflags 2.6.0", + "bitflags", ] [[package]] name = "redox_users" -version = "0.4.5" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd283d9651eeda4b2a83a43c1c91b266c40fd76ecd39a50a8c630ae69dc72891" +checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" dependencies = [ "getrandom", "libredox", @@ -3085,9 +3079,9 @@ dependencies = [ [[package]] name = "reqwest" -version = "0.12.5" +version = "0.12.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c7d6d2a27d57148378eb5e111173f4276ad26340ecc5c49a4a2152167a2d6a37" +checksum = "f8f4955649ef5c38cc7f9e8aa41761d48fb9677197daea9984dc54f56aad5e63" dependencies = [ "base64 0.22.1", "bytes", @@ -3126,7 +3120,7 @@ dependencies = [ "wasm-bindgen-futures", "web-sys", "webpki-roots", - "winreg", + "windows-registry", ] [[package]] @@ -3162,7 +3156,7 @@ version = "0.38.34" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "70dc5ec042f7a43c4a73241207cecc9873a06d45debb38b329f8541d85c2730f" dependencies = [ - "bitflags 2.6.0", + "bitflags", "errno", "libc", "linux-raw-sys", @@ -3574,9 +3568,9 @@ checksum = "b7401a30af6cb5818bb64852270bb722533397edcfc7344954a38f420819ece2" [[package]] name = "syn" -version = "2.0.74" +version = "2.0.75" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fceb41e3d546d0bd83421d3409b1460cc7444cd389341a4c880fe7a042cb3d7" +checksum = "f6af063034fc1935ede7be0122941bafa9bacb949334d090b77ca98b5817c7d9" dependencies = [ "proc-macro2", "quote", @@ -3588,23 +3582,26 @@ name = "sync_wrapper" version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a7065abeca94b6a8a577f9bd45aa0867a2238b74e8eb67cf10d492bc39351394" +dependencies = [ + "futures-core", +] [[package]] name = "system-configuration" -version = "0.5.1" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba3a3adc5c275d719af8cb4272ea1c4a6d668a777f37e115f6d11ddbc1c8e0e7" +checksum = "3c879d448e9d986b661742763247d3693ed13609438cf3d006f51f5368a5ba6b" dependencies = [ - "bitflags 1.3.2", + "bitflags", "core-foundation", "system-configuration-sys", ] [[package]] name = "system-configuration-sys" -version = "0.5.0" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a75fb188eb626b924683e3b95e3a48e63551fcfb51949de2f06a9d91dbee93c9" +checksum = "8e1d1b10ced5ca923a1fcb8d03e96b8d3268065d724548c0211415ff6ac6bac4" dependencies = [ "core-foundation-sys", "libc", @@ -3739,9 +3736,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.39.2" +version = "1.39.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "daa4fb1bc778bd6f04cbfc4bb2d06a7396a8f299dc33ea1900cedaa316f467b1" +checksum = "9babc99b9923bfa4804bd74722ff02c0381021eafa4db9949217e3be8e84fff5" dependencies = [ "backtrace", "bytes", @@ -3952,9 +3949,9 @@ checksum = "0336d538f7abc86d282a4189614dfaa90810dfc2c6f6427eaf88e16311dd225d" [[package]] name = "unicode-xid" -version = "0.2.4" +version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" +checksum = "229730647fbc343e3a80e463c1db7f78f3855d3f3739bee0dda773c9a037c90a" [[package]] name = "unsafe-libyaml" @@ -4140,7 +4137,7 @@ dependencies = [ [[package]] name = "weaver" -version = "0.9.0" +version = "0.9.1" dependencies = [ "assert_cmd", "clap", @@ -4171,7 +4168,7 @@ dependencies = [ [[package]] name = "weaver_cache" -version = "0.9.0" +version = "0.9.1" dependencies = [ "dirs", "flate2", @@ -4192,7 +4189,7 @@ dependencies = [ [[package]] name = "weaver_checker" -version = "0.9.0" +version = "0.9.1" dependencies = [ "globset", "miette", @@ -4207,9 +4204,10 @@ dependencies = [ [[package]] name = "weaver_codegen_test" -version = "0.9.0" +version = "0.9.1" dependencies = [ "dirs", + "miette", "opentelemetry 0.23.0", "walkdir", "weaver_cache", @@ -4221,7 +4219,7 @@ dependencies = [ [[package]] name = "weaver_common" -version = "0.9.0" +version = "0.9.1" dependencies = [ "miette", "paris", @@ -4232,7 +4230,7 @@ dependencies = [ [[package]] name = "weaver_diff" -version = "0.9.0" +version = "0.9.1" dependencies = [ "similar", "walkdir", @@ -4240,7 +4238,7 @@ dependencies = [ [[package]] name = "weaver_forge" -version = "0.9.0" +version = "0.9.1" dependencies = [ "convert_case", "dirs", @@ -4277,7 +4275,7 @@ dependencies = [ [[package]] name = "weaver_resolved_schema" -version = "0.9.0" +version = "0.9.1" dependencies = [ "ordered-float", "schemars", @@ -4290,7 +4288,7 @@ dependencies = [ [[package]] name = "weaver_resolver" -version = "0.9.0" +version = "0.9.1" dependencies = [ "glob", "miette", @@ -4308,7 +4306,7 @@ dependencies = [ [[package]] name = "weaver_semconv" -version = "0.9.0" +version = "0.9.1" dependencies = [ "glob", "miette", @@ -4323,7 +4321,7 @@ dependencies = [ [[package]] name = "weaver_semconv_gen" -version = "0.9.0" +version = "0.9.1" dependencies = [ "miette", "nom", @@ -4340,7 +4338,7 @@ dependencies = [ [[package]] name = "weaver_version" -version = "0.9.0" +version = "0.9.1" dependencies = [ "schemars", "semver", @@ -4408,6 +4406,36 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows-registry" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e400001bb720a623c1c69032f8e3e4cf09984deec740f007dd2b03ec864804b0" +dependencies = [ + "windows-result", + "windows-strings", + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-result" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d1043d8214f791817bab27572aaa8af63732e11bf84aa21a45a78d6c317ae0e" +dependencies = [ + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-strings" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cd9b125c486025df0eabcb585e62173c6c9eddcec5d117d3b6e8c30e2ee4d10" +dependencies = [ + "windows-result", + "windows-targets 0.52.6", +] + [[package]] name = "windows-sys" version = "0.48.0" @@ -4565,16 +4593,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "winreg" -version = "0.52.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a277a57398d4bfa075df44f501a17cfdf8542d224f0d36095a2adc7aee4ef0a5" -dependencies = [ - "cfg-if", - "windows-sys 0.48.0", -] - [[package]] name = "xattr" version = "1.3.1" diff --git a/Cargo.toml b/Cargo.toml index c515c260..92122fae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver" -version = "0.9.0" +version = "0.9.1" authors = ["OpenTelemetry"] edition = "2021" repository = "https://github.com/open-telemetry/weaver" diff --git a/crates/weaver_cache/Cargo.toml b/crates/weaver_cache/Cargo.toml index bc087a70..35e94483 100644 --- a/crates/weaver_cache/Cargo.toml +++ b/crates/weaver_cache/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_cache" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_checker/Cargo.toml b/crates/weaver_checker/Cargo.toml index 79bfb2ae..5f8d6dbb 100644 --- a/crates/weaver_checker/Cargo.toml +++ b/crates/weaver_checker/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_checker" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_codegen_test/Cargo.toml b/crates/weaver_codegen_test/Cargo.toml index 27ef45b4..877c784b 100644 --- a/crates/weaver_codegen_test/Cargo.toml +++ b/crates/weaver_codegen_test/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_codegen_test" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true @@ -22,6 +22,7 @@ weaver_resolver = { path = "../weaver_resolver" } weaver_semconv = { path = "../weaver_semconv" } walkdir.workspace = true dirs.workspace = true +miette.workspace = true [dependencies] -opentelemetry.workspace = true \ No newline at end of file +opentelemetry.workspace = true diff --git a/crates/weaver_codegen_test/build.rs b/crates/weaver_codegen_test/build.rs index f2eaa21d..d6c82733 100644 --- a/crates/weaver_codegen_test/build.rs +++ b/crates/weaver_codegen_test/build.rs @@ -6,6 +6,7 @@ //! specified by Cargo. This generation step occurs before the standard build of the crate. The //! generated code, along with the standard crate code, will be compiled together. +use miette::Diagnostic; use std::collections::HashMap; use std::io::Write; use std::path::{Component, Path, PathBuf}; @@ -45,6 +46,8 @@ fn main() { let registry_repo = RegistryRepo::try_new("main", ®istry_path).unwrap_or_else(|e| process_error(&logger, e)); let semconv_specs = SchemaResolver::load_semconv_specs(®istry_repo) + .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 schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry) diff --git a/crates/weaver_common/Cargo.toml b/crates/weaver_common/Cargo.toml index 62bb541e..4ae5d87b 100644 --- a/crates/weaver_common/Cargo.toml +++ b/crates/weaver_common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_common" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_common/src/diagnostic.rs b/crates/weaver_common/src/diagnostic.rs index d63b7f3c..9f0fcab7 100644 --- a/crates/weaver_common/src/diagnostic.rs +++ b/crates/weaver_common/src/diagnostic.rs @@ -116,6 +116,12 @@ impl DiagnosticMessage { diagnostic, } } + + /// Returns true if the diagnostic message is a warning + #[must_use] + pub fn is_warning(&self) -> bool { + self.diagnostic.severity == Some(Severity::Warning) + } } impl DiagnosticMessages { @@ -137,6 +143,12 @@ impl DiagnosticMessages { self.0.extend(diag_msgs.0); } + /// Extends the current `DiagnosticMessages` with the provided + /// `Vec`. + pub fn extend_from_vec(&mut self, diag_msgs: Vec) { + self.0.extend(diag_msgs); + } + /// Logs all the diagnostic messages pub fn log(&self, logger: impl Logger) { self.0 diff --git a/crates/weaver_common/src/lib.rs b/crates/weaver_common/src/lib.rs index f9917ad5..e0237792 100644 --- a/crates/weaver_common/src/lib.rs +++ b/crates/weaver_common/src/lib.rs @@ -6,6 +6,7 @@ pub mod diagnostic; pub mod error; pub mod in_memory; pub mod quiet; +pub mod result; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; diff --git a/crates/weaver_common/src/result.rs b/crates/weaver_common/src/result.rs new file mode 100644 index 00000000..9dde0ba2 --- /dev/null +++ b/crates/weaver_common/src/result.rs @@ -0,0 +1,233 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Weaver Result type supporting both non-fatal errors (NFEs) and fatal errors. +//! +//! NFEs do not prevent the next operations from completing successfully. For example, +//! if a semconv file is invalid, we generate a non-fatal error and continue processing +//! the other files. +//! +//! NFEs in Weaver are standard Rust errors. + +use crate::diagnostic::{DiagnosticMessage, DiagnosticMessages}; +use crate::error::WeaverError; +use miette::Diagnostic; +use serde::Serialize; +use std::error::Error; + +/// Weaver Result type supporting both non-fatal errors (NFEs) and fatal errors. +#[must_use] +pub enum WResult { + /// The operation was successful, the result T is returned. + Ok(T), + /// The operation was successful, the result T is returned along with + /// a list of non-fatal errors. + OkWithNFEs(T, Vec), + /// The operation failed with a fatal error. By definition, we can only have + /// one fatal error. + FatalErr(E), +} + +impl WResult +where + E: WeaverError + Error + Diagnostic + Serialize + Send + Sync + 'static, +{ + /// Creates a new [`WResult`] with a successful result. + pub fn with_non_fatal_errors(result: T, non_fatal_errors: Vec) -> Self { + if non_fatal_errors.is_empty() { + WResult::Ok(result) + } else { + WResult::OkWithNFEs(result, non_fatal_errors) + } + } + + /// Converts a [`WResult`] into a standard [`Result`], optionally capturing non-fatal errors. + pub fn capture_non_fatal_errors(self, diag_msgs: &mut DiagnosticMessages) -> Result { + match self { + WResult::Ok(result) => Ok(result), + WResult::OkWithNFEs(result, nfes) => { + let msgs = nfes.into_iter().map(DiagnosticMessage::new).collect(); + diag_msgs.extend_from_vec(msgs); + Ok(result) + } + WResult::FatalErr(fatal_err) => Err(fatal_err), + } + } + + /// Capture the warnings into the provided vector and return a [`WResult`] + /// without the warnings. + pub fn capture_warnings(self, diag_msgs: &mut DiagnosticMessages) -> WResult { + if let WResult::OkWithNFEs(result, nfes) = self { + let (warnings, errors): (Vec<_>, Vec<_>) = nfes + .into_iter() + .partition(|e| matches!(e.severity(), Some(miette::Severity::Warning))); + let warnings: Vec<_> = warnings.into_iter().map(DiagnosticMessage::new).collect(); + diag_msgs.extend_from_vec(warnings); + if errors.is_empty() { + WResult::Ok(result) + } else { + WResult::OkWithNFEs(result, errors) + } + } else { + self + } + } + + /// Return a [`WResult`] without the non-fatal errors with severity=warning. + pub fn ignore(self, ignore: F) -> WResult + where + F: Fn(&E) -> bool, + { + match self { + WResult::OkWithNFEs(result, non_fatal_errors) => { + // Remove warnings from the non-fatal errors. + let errors: Vec<_> = non_fatal_errors + .into_iter() + .filter(|e| !ignore(e)) + .collect(); + if errors.is_empty() { + WResult::Ok(result) + } else { + WResult::OkWithNFEs(result, errors) + } + } + _ => self, + } + } + + /// Calls a function with a reference to the contained value if [`Ok`]. + /// + /// Returns the original result. + pub fn inspect)>(self, f: F) -> Self { + match &self { + WResult::Ok(result) => f(result, None), + WResult::OkWithNFEs(result, nfes) => f(result, Some(nfes)), + WResult::FatalErr(_) => {} + } + + self + } + + /// Converts a [`WResult`] into a standard [`Result`], potentially + /// aggregating non-fatal errors into a single error. + pub fn into_result_failing_non_fatal(self) -> Result { + match self { + WResult::Ok(result) => Ok(result), + WResult::OkWithNFEs(result, errors) => { + if errors.is_empty() { + Ok(result) + } else if errors.len() == 1 { + Err(errors + .into_iter() + .next() + .expect("should never happen as we checked the length")) + } else { + let compound_error = E::compound(errors); + Err(compound_error) + } + } + WResult::FatalErr(e) => Err(e), + } + } + + /// Converts a [`WResult`] into a standard [`Result`], returning the result + /// alongside any non-fatal errors. + pub fn into_result_with_non_fatal(self) -> Result<(T, Vec), E> { + match self { + WResult::Ok(result) => Ok((result, Vec::new())), + WResult::OkWithNFEs(result, errors) => Ok((result, errors)), + WResult::FatalErr(e) => Err(e), + } + } + + /// Maps a [`WResult`] to [`WResult`] by applying a function to a + /// contained [`Ok`] value, leaving an [`Err`] value untouched. + pub fn map U>(self, op: F) -> WResult { + match self { + WResult::Ok(t) => WResult::Ok(op(t)), + WResult::OkWithNFEs(t, nfes) => WResult::OkWithNFEs(op(t), nfes), + WResult::FatalErr(err) => WResult::FatalErr(err), + } + } +} + +#[cfg(test)] +mod tests { + use crate::diagnostic::DiagnosticMessages; + use crate::error::WeaverError; + use crate::result::WResult; + use miette::Diagnostic; + use serde::Serialize; + + #[derive(thiserror::Error, Debug, PartialEq, Serialize, Diagnostic)] + enum TestError { + #[error("Warning")] + #[diagnostic(severity(Warning))] + Warning, + #[error("Error")] + Error, + #[error("Compound error")] + CompoundError(Vec), + } + + impl WeaverError for TestError { + fn compound(errors: Vec) -> Self { + TestError::CompoundError(errors) + } + } + + #[test] + fn test_werror() -> Result<(), TestError> { + let mut diag_msgs = DiagnosticMessages::empty(); + let result: Result = WResult::Ok(42) + .inspect(|r, _| assert_eq!(*r, 42)) + .capture_warnings(&mut diag_msgs) + .into_result_failing_non_fatal(); + + assert_eq!(result, Ok(42)); + assert_eq!(diag_msgs.len(), 0); + + let mut diag_msgs = DiagnosticMessages::empty(); + let result: Result = WResult::Ok(42) + .inspect(|r, _| assert_eq!(*r, 42)) + .capture_warnings(&mut diag_msgs) + .into_result_failing_non_fatal(); + + assert_eq!(result, Ok(42)); + assert_eq!(diag_msgs.len(), 0); + + let mut diag_msgs = DiagnosticMessages::empty(); + let result: Result = + WResult::OkWithNFEs(42, vec![TestError::Warning, TestError::Error]) + .inspect(|r, nfes| { + assert_eq!(*r, 42); + assert_eq!(nfes.unwrap().len(), 2); + }) + .capture_warnings(&mut diag_msgs) + .into_result_failing_non_fatal(); + + assert_eq!(result, Err(TestError::Error)); + assert_eq!(diag_msgs.len(), 1); + + let mut diag_msgs = DiagnosticMessages::empty(); + let result = WResult::OkWithNFEs(42, vec![TestError::Warning, TestError::Error]) + .inspect(|r, nfes| { + assert_eq!(*r, 42); + assert_eq!(nfes.unwrap().len(), 2); + }) + .capture_non_fatal_errors(&mut diag_msgs)?; + + assert_eq!(result, 42); + assert_eq!(diag_msgs.len(), 2); + + let (result, nfes) = WResult::OkWithNFEs(42, vec![TestError::Warning, TestError::Error]) + .inspect(|r, nfes| { + assert_eq!(*r, 42); + assert_eq!(nfes.unwrap().len(), 2); + }) + .into_result_with_non_fatal()?; + assert_eq!(result, 42); + assert_eq!(nfes.len(), 2); + + Ok(()) + } +} diff --git a/crates/weaver_diff/Cargo.toml b/crates/weaver_diff/Cargo.toml index 173fe5d9..fdca92cf 100644 --- a/crates/weaver_diff/Cargo.toml +++ b/crates/weaver_diff/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_diff" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_forge/Cargo.toml b/crates/weaver_forge/Cargo.toml index c9dee0b8..743e5ece 100644 --- a/crates/weaver_forge/Cargo.toml +++ b/crates/weaver_forge/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_forge" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_forge/README.md b/crates/weaver_forge/README.md index a72fb86f..888e3f96 100644 --- a/crates/weaver_forge/README.md +++ b/crates/weaver_forge/README.md @@ -856,7 +856,7 @@ In addition, OTel Weaver provides the following custom function: prefixed with "Notes: ". If the attribute note is not defined then the comment will only contain the brief description without the prefix "Notes: ". - `{{ [attr.brief, concat_if("\n\nNotes: ", attr.note) | comment }}` + `{{ [attr.brief, concat_if("\n\nNotes: ", attr.note)] | comment }}` ### Jinja Tests Reference diff --git a/crates/weaver_forge/src/lib.rs b/crates/weaver_forge/src/lib.rs index ed657a6c..2f0ef647 100644 --- a/crates/weaver_forge/src/lib.rs +++ b/crates/weaver_forge/src/lib.rs @@ -753,6 +753,7 @@ mod tests { ) { let registry_id = "default"; 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) } @@ -949,6 +950,7 @@ mod tests { let registry_id = "default"; let mut registry = SemConvRegistry::try_from_path_pattern(registry_id, "data/*.yaml") + .into_result_failing_non_fatal() .expect("Failed to load registry"); let schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry) .expect("Failed to resolve registry"); @@ -1084,6 +1086,7 @@ mod tests { registry_id, "data/mini_registry_for_comments/*.yaml", ) + .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); diff --git a/crates/weaver_resolved_schema/Cargo.toml b/crates/weaver_resolved_schema/Cargo.toml index 610280a9..3a91e5ff 100644 --- a/crates/weaver_resolved_schema/Cargo.toml +++ b/crates/weaver_resolved_schema/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_resolved_schema" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_resolver/Cargo.toml b/crates/weaver_resolver/Cargo.toml index 61a142ef..97a30e10 100644 --- a/crates/weaver_resolver/Cargo.toml +++ b/crates/weaver_resolver/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_resolver" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_resolver/src/lib.rs b/crates/weaver_resolver/src/lib.rs index f6e15642..5ce6e280 100644 --- a/crates/weaver_resolver/src/lib.rs +++ b/crates/weaver_resolver/src/lib.rs @@ -13,7 +13,8 @@ use walkdir::DirEntry; use weaver_cache::RegistryRepo; use weaver_common::diagnostic::{DiagnosticMessage, DiagnosticMessages}; -use weaver_common::error::{format_errors, handle_errors, WeaverError}; +use weaver_common::error::{format_errors, WeaverError}; +use weaver_common::result::WResult; use weaver_common::Logger; use weaver_resolved_schema::catalog::Catalog; use weaver_resolved_schema::registry::Constraint; @@ -47,22 +48,6 @@ pub enum Error { error: String, }, - /// A semantic convention error. - #[error(transparent)] - #[diagnostic(transparent)] - SemConvError { - /// The semconv error that occurred. - #[from] - error: weaver_semconv::Error, - }, - - /// Failed to walk dir. - #[error("Failed walk dir: {error}")] - FailToWalkDir { - /// The error that occurred. - error: String, - }, - /// Failed to resolve a set of attributes. #[error("Failed to resolve a set of attributes {ids:?}: {error}")] FailToResolveAttributes { @@ -253,7 +238,7 @@ impl SchemaResolver { /// * `registry_repo` - The registry repository containing the semantic convention files. pub fn load_semconv_specs( registry_repo: &RegistryRepo, - ) -> Result, Error> { + ) -> WResult, weaver_semconv::Error> { Self::load_semconv_from_local_path( registry_repo.path().to_path_buf(), registry_repo.registry_path_repr(), @@ -270,7 +255,7 @@ impl SchemaResolver { fn load_semconv_from_local_path( local_path: PathBuf, registry_path_repr: &str, - ) -> Result, Error> { + ) -> WResult, weaver_semconv::Error> { fn is_hidden(entry: &DirEntry) -> bool { entry .file_name() @@ -301,8 +286,8 @@ impl SchemaResolver { return vec![].into_par_iter(); } - match SemConvRegistry::semconv_spec_from_file(entry.path()) { - Ok((path, spec)) => { + vec![SemConvRegistry::semconv_spec_from_file(entry.path()).map( + |(path, spec)| { // Replace the local path with the git URL combined with the relative path // of the semantic convention file. let prefix = local_path @@ -316,19 +301,12 @@ impl SchemaResolver { let relative_path = &path[prefix.len() + 1..]; format!("{}/{}", registry_path_repr, relative_path) }; - vec![Ok((path, spec))].into_par_iter() - } - Err(e) => match e { - weaver_semconv::Error::CompoundError(errors) => errors - .into_iter() - .map(|e| Err(Error::SemConvError { error: e })) - .collect::>() - .into_par_iter(), - _ => vec![Err(Error::SemConvError { error: e })].into_par_iter(), + (path, spec) }, - } + )] + .into_par_iter() } - Err(e) => vec![Err(Error::FailToWalkDir { + Err(e) => vec![WResult::FatalErr(weaver_semconv::Error::SemConvSpecError { error: e.to_string(), })] .into_par_iter(), @@ -336,20 +314,23 @@ impl SchemaResolver { }) .collect::>(); - let mut error = vec![]; - let result = result - .into_iter() - .filter_map(|r| match r { - Ok(r) => Some(r), - Err(e) => { - error.push(e); - None + // Process all the results of the previous parallel processing. + // The first fatal error will stop the processing and return the error. + // Otherwise, all non-fatal errors will be collected and returned along + // with the result. + let mut non_fatal_errors = vec![]; + let mut specs = vec![]; + for r in result { + match r { + WResult::Ok(t) => specs.push(t), + WResult::OkWithNFEs(t, nfes) => { + specs.push(t); + non_fatal_errors.extend(nfes); } - }) - .collect::>(); - - handle_errors(error)?; + WResult::FatalErr(e) => return WResult::FatalErr(e), + } + } - Ok(result) + WResult::OkWithNFEs(specs, non_fatal_errors) } } diff --git a/crates/weaver_resolver/src/registry.rs b/crates/weaver_resolver/src/registry.rs index 60709faf..db324169 100644 --- a/crates/weaver_resolver/src/registry.rs +++ b/crates/weaver_resolver/src/registry.rs @@ -814,6 +814,7 @@ mod tests { registry_id, &format!("{}/registry/*.yaml", test_dir), ) + .into_result_failing_non_fatal() .expect("Failed to load semconv specs"); let mut attr_catalog = AttributeCatalog::default(); @@ -874,6 +875,7 @@ mod tests { let mut sc_specs = SemConvRegistry::new("default"); sc_specs .add_semconv_spec_from_string("", registry_spec) + .into_result_failing_non_fatal() .expect("Failed to load semconv spec"); let mut attr_catalog = AttributeCatalog::default(); @@ -1017,7 +1019,8 @@ groups: let mut semconv_registry = SemConvRegistry::try_from_path_pattern( registry_id, "data/registry-test-7-spans/registry/*.yaml", - )?; + ) + .into_result_failing_non_fatal()?; // Resolve the semantic convention registry. let resolved_schema = diff --git a/crates/weaver_semconv/Cargo.toml b/crates/weaver_semconv/Cargo.toml index fb9b070e..f81d6645 100644 --- a/crates/weaver_semconv/Cargo.toml +++ b/crates/weaver_semconv/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_semconv" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_semconv/allowed-external-types.toml b/crates/weaver_semconv/allowed-external-types.toml index 1dbc3fa2..a87cab8c 100644 --- a/crates/weaver_semconv/allowed-external-types.toml +++ b/crates/weaver_semconv/allowed-external-types.toml @@ -5,7 +5,7 @@ allowed_external_types = [ "serde::ser::Serialize", "serde::de::Deserialize", - "weaver_common::error::WeaverError", + "weaver_common::*", "ordered_float::OrderedFloat", # ToDo: Remove this dependency before version 1.0 "miette::protocol::Diagnostic", "schemars::JsonSchema", diff --git a/crates/weaver_semconv/src/attribute.rs b/crates/weaver_semconv/src/attribute.rs index 444c9990..324cc8b3 100644 --- a/crates/weaver_semconv/src/attribute.rs +++ b/crates/weaver_semconv/src/attribute.rs @@ -11,6 +11,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::fmt::{Display, Formatter}; use std::ops::Not; +use weaver_common::result::WResult; use AttributeType::{Enum, PrimitiveOrArray, Template}; /// An attribute specification. @@ -438,7 +439,7 @@ impl Examples { group_id: &str, attr_id: &str, path_or_url: &str, - ) -> Result<(), Error> { + ) -> WResult<(), Error> { match (self, attr_type) { (Examples::Bool(_), PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Boolean)) | (Examples::Int(_), PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Int)) @@ -452,11 +453,11 @@ impl Examples { | (Examples::ListOfDoubles(_), PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Doubles)) | (Examples::ListOfBools(_), PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Booleans)) | (Examples::ListOfStrings(_), PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Strings)) => { - Ok(()) + WResult::Ok(()) } (_, Enum { .. }) => { // enum types are open so it's not possible to validate the examples - Ok(()) + WResult::Ok(()) } // Only if future mode is disabled, we allow to have examples following // the conventions used in semconv 1.27.0 and earlier. @@ -464,23 +465,29 @@ impl Examples { | (Examples::Doubles(_), PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Doubles)) | (Examples::Bools(_), PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Booleans)) | (Examples::Strings(_), PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Strings)) => { - Err(Error::InvalidExampleWarning { - path_or_url: path_or_url.to_owned(), - group_id: group_id.to_owned(), - attribute_id: attr_id.to_owned(), - error: format!("All examples SHOULD be of type `{}`", attr_type), - }) + WResult::OkWithNFEs( + (), + vec![Error::InvalidExampleWarning { + path_or_url: path_or_url.to_owned(), + group_id: group_id.to_owned(), + attribute_id: attr_id.to_owned(), + error: format!("All examples SHOULD be of type `{}`", attr_type), + }], + ) } (Examples::String(_), Template(TemplateTypeSpec::String)) | (Examples::Strings(_), Template(TemplateTypeSpec::String)) | (Examples::String(_), Template(TemplateTypeSpec::Strings)) - | (Examples::Strings(_), Template(TemplateTypeSpec::Strings)) => Ok(()), - _ => Err(Error::InvalidExampleError { - path_or_url: path_or_url.to_owned(), - group_id: group_id.to_owned(), - attribute_id: attr_id.to_owned(), - error: format!("All examples MUST be of type `{}`", attr_type), - }), + | (Examples::Strings(_), Template(TemplateTypeSpec::Strings)) => WResult::Ok(()), + _ => WResult::OkWithNFEs( + (), + vec![Error::InvalidExampleError { + path_or_url: path_or_url.to_owned(), + group_id: group_id.to_owned(), + attribute_id: attr_id.to_owned(), + error: format!("All examples MUST be of type `{}`", attr_type), + }], + ), } } } @@ -987,74 +994,148 @@ mod tests { // === Test int-like examples === let examples = Examples::Int(42); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_str, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_str, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::Ints(vec![42, 43]); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_str, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_str, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::ListOfInts(vec![vec![42, 43], vec![44, 45]]); - assert!(examples.validate(&attr_ints, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_ints, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::ListOfStrings(vec![ vec!["42".to_owned(), "43".to_owned()], vec!["44".to_owned(), "45".to_owned()], ]); - assert!(examples.validate(&attr_ints, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_ints, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); // === Test string-like examples === let examples = Examples::String("foo".to_owned()); - assert!(examples.validate(&attr_str, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_str, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::Strings(vec!["foo".to_owned(), "bar".to_owned()]); - assert!(examples.validate(&attr_str, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_str, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::ListOfStrings(vec![ vec!["foo".to_owned(), "bar".to_owned()], vec!["baz".to_owned(), "qux".to_owned()], ]); - assert!(examples.validate(&attr_strs, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_ints, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_strs, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_ints, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::ListOfInts(vec![vec![42, 43], vec![44, 45]]); - assert!(examples.validate(&attr_str, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_str, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); // Non-strict validation let examples = Examples::Strings(vec!["foo".to_owned(), "bar".to_owned()]); - assert!(examples.validate(&attr_str, "grp", "attr", "url").is_ok()); + assert!(examples + .validate(&attr_str, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); // === Test bool-like examples === let examples = Examples::Bool(true); - assert!(examples.validate(&attr_bool, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_bool, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::Bools(vec![true, false]); - assert!(examples.validate(&attr_bool, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_bool, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::ListOfBools(vec![vec![true, false], vec![false, true]]); - assert!(examples.validate(&attr_bools, "grp", "attr", "url").is_ok()); - assert!(examples.validate(&attr_bool, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_bools, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_ok()); + assert!(examples + .validate(&attr_bool, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::ListOfInts(vec![vec![42, 43], vec![44, 45]]); - assert!(examples.validate(&attr_bool, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_bool, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); // === Test double-like examples === let examples = Examples::Double(OrderedFloat(42.0)); assert!(examples .validate(&attr_double, "grp", "attr", "url") + .into_result_failing_non_fatal() .is_ok()); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::Doubles(vec![OrderedFloat(42.0), OrderedFloat(43.0)]); assert!(examples .validate(&attr_double, "grp", "attr", "url") + .into_result_failing_non_fatal() .is_ok()); - assert!(examples.validate(&attr_int, "grp", "attr", "url").is_err()); + assert!(examples + .validate(&attr_int, "grp", "attr", "url") + .into_result_failing_non_fatal() + .is_err()); let examples = Examples::ListOfDoubles(vec![ vec![OrderedFloat(42.0), OrderedFloat(43.0)], @@ -1062,14 +1143,17 @@ mod tests { ]); assert!(examples .validate(&attr_doubles, "grp", "attr", "url") + .into_result_failing_non_fatal() .is_ok()); assert!(examples .validate(&attr_double, "grp", "attr", "url") + .into_result_failing_non_fatal() .is_err()); let examples = Examples::ListOfInts(vec![vec![42, 43], vec![44, 45]]); assert!(examples .validate(&attr_double, "grp", "attr", "url") + .into_result_failing_non_fatal() .is_err()); } } diff --git a/crates/weaver_semconv/src/group.rs b/crates/weaver_semconv/src/group.rs index 3afab346..a0211432 100644 --- a/crates/weaver_semconv/src/group.rs +++ b/crates/weaver_semconv/src/group.rs @@ -13,7 +13,7 @@ use crate::attribute::{AttributeSpec, AttributeType, PrimitiveOrArrayTypeSpec}; use crate::group::InstrumentSpec::{Counter, Gauge, Histogram, UpDownCounter}; use crate::stability::Stability; use crate::Error; -use weaver_common::error::handle_errors; +use weaver_common::result::WResult; /// Group Spec contain the list of semantic conventions for attributes, /// metrics, events, spans, etc. @@ -90,7 +90,7 @@ pub struct GroupSpec { impl GroupSpec { /// Validation logic for the group. - pub(crate) fn validate(&self, path_or_url: &str) -> Result<(), Error> { + pub(crate) fn validate(&self, path_or_url: &str) -> WResult<(), Error> { let mut errors = vec![]; // Fields span_kind and events are only valid if type is span (the default). @@ -179,37 +179,44 @@ impl GroupSpec { } = attribute { if let Some(examples) = examples { - if let Err(err) = examples.validate(r#type, &self.id, id, path_or_url) { - errors.push(err); + match examples.validate(r#type, &self.id, id, path_or_url) { + WResult::Ok(_) => {} + WResult::OkWithNFEs(_, errs) => errors.extend(errs), + WResult::FatalErr(err) => return WResult::FatalErr(err), } - continue; - } + } else { + // No examples are set. - if *r#type == AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::String) { - errors.push(Error::InvalidAttribute { - path_or_url: path_or_url.to_owned(), - group_id: self.id.clone(), - attribute_id: attribute.id(), - error: "This attribute is a string but it does not contain any examples." - .to_owned(), - }); - } - if *r#type == AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Strings) { - errors.push(Error::InvalidAttribute { - path_or_url: path_or_url.to_owned(), - group_id: self.id.clone(), - attribute_id: attribute.id(), - error: + // string attributes must have examples. + if *r#type == AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::String) + { + errors.push(Error::InvalidExampleWarning { + path_or_url: path_or_url.to_owned(), + group_id: self.id.clone(), + attribute_id: attribute.id(), + error: + "This attribute is a string but it does not contain any examples." + .to_owned(), + }); + } + + // string array attributes must have examples. + if *r#type == AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::Strings) + { + errors.push(Error::InvalidExampleWarning { + path_or_url: path_or_url.to_owned(), + group_id: self.id.clone(), + attribute_id: attribute.id(), + error: "This attribute is a string array but it does not contain any examples." .to_owned(), - }); + }); + } } } } - handle_errors(errors)?; - - Ok(()) + WResult::with_non_fatal_errors((), errors) } } @@ -306,7 +313,7 @@ impl Display for InstrumentSpec { #[cfg(test)] mod tests { use crate::attribute::Examples; - use crate::Error::{CompoundError, InvalidAttribute, InvalidGroup, InvalidMetric}; + use crate::Error::{CompoundError, InvalidExampleWarning, InvalidGroup, InvalidMetric}; use super::*; @@ -342,11 +349,14 @@ mod tests { name: None, display_name: None, }; - assert!(group.validate("").is_ok()); + assert!(group + .validate("") + .into_result_failing_non_fatal() + .is_ok()); // Span kind is set but the type is not span. group.r#type = GroupType::Metric; - let result = group.validate(""); + let result = group.validate("").into_result_failing_non_fatal(); assert_eq!( Err(CompoundError(vec![ InvalidGroup { @@ -386,7 +396,7 @@ mod tests { group.r#type = GroupType::Event; "".clone_into(&mut group.prefix); group.name = None; - let result = group.validate(""); + let result = group.validate("").into_result_failing_non_fatal(); assert_eq!(Err( CompoundError( vec![ @@ -442,7 +452,10 @@ mod tests { name: None, display_name: None, }; - assert!(group.validate("").is_ok()); + assert!(group + .validate("") + .into_result_failing_non_fatal() + .is_ok()); // Examples are mandatory for string attributes. group.attributes = vec![AttributeSpec::Id { @@ -457,9 +470,9 @@ mod tests { sampling_relevant: None, note: "".to_owned(), }]; - let result = group.validate(""); + let result = group.validate("").into_result_failing_non_fatal(); assert_eq!( - Err(InvalidAttribute { + Err(InvalidExampleWarning { path_or_url: "".to_owned(), group_id: "test".to_owned(), attribute_id: "test".to_owned(), @@ -482,9 +495,9 @@ mod tests { sampling_relevant: None, note: "".to_owned(), }]; - let result = group.validate(""); + let result = group.validate("").into_result_failing_non_fatal(); assert_eq!( - Err(InvalidAttribute { + Err(InvalidExampleWarning { path_or_url: "".to_owned(), group_id: "test".to_owned(), attribute_id: "test".to_owned(), diff --git a/crates/weaver_semconv/src/lib.rs b/crates/weaver_semconv/src/lib.rs index bd1b797a..fc43344c 100644 --- a/crates/weaver_semconv/src/lib.rs +++ b/crates/weaver_semconv/src/lib.rs @@ -40,6 +40,15 @@ pub enum Error { error: String, }, + /// A generic error related to a semantic convention spec. + #[error( + "The following error occurred during the processing of semantic convention file: {error}" + )] + SemConvSpecError { + /// The error that occurred. + error: String, + }, + /// The semantic convention spec is invalid. #[error("The semantic convention spec is invalid (path_or_url: {path_or_url:?}). {error}")] InvalidSemConvSpec { @@ -193,7 +202,9 @@ mod tests { let mut registry = SemConvRegistry::default(); for yaml in yaml_files { - let result = registry.add_semconv_spec_from_file(yaml); + let result = registry + .add_semconv_spec_from_file(yaml) + .into_result_failing_non_fatal(); assert!(result.is_ok(), "{:#?}", result.err().unwrap()); } } @@ -204,7 +215,9 @@ mod tests { let mut registry = SemConvRegistry::default(); for yaml in yaml_files { - let result = registry.add_semconv_spec_from_file(yaml); + let result = registry + .add_semconv_spec_from_file(yaml) + .into_result_failing_non_fatal(); assert!(result.is_err(), "{:#?}", result.ok().unwrap()); if let Err(err) = result { let output = format!("{}", err); diff --git a/crates/weaver_semconv/src/registry.rs b/crates/weaver_semconv/src/registry.rs index 651aa4ae..5ee91a40 100644 --- a/crates/weaver_semconv/src/registry.rs +++ b/crates/weaver_semconv/src/registry.rs @@ -10,6 +10,7 @@ use crate::stats::Stats; use crate::Error; use std::collections::HashMap; use std::path::Path; +use weaver_common::result::WResult; /// A semantic convention registry is a collection of semantic convention /// specifications indexed by group id. @@ -64,20 +65,37 @@ impl SemConvRegistry { /// # Errors /// /// If the registry path pattern is invalid. - pub fn try_from_path_pattern(registry_id: &str, path_pattern: &str) -> Result { - let mut registry = SemConvRegistry::new(registry_id); - for sc_entry in glob::glob(path_pattern).map_err(|e| Error::InvalidRegistryPathPattern { - path_pattern: path_pattern.to_owned(), - error: e.to_string(), - })? { - let path_buf = sc_entry.map_err(|e| Error::InvalidRegistryPathPattern { - path_pattern: path_pattern.to_owned(), - error: e.to_string(), - })?; - let semconv_spec = SemConvSpecWithProvenance::from_file(path_buf.as_path())?; - registry.add_semconv_spec(semconv_spec); + pub fn try_from_path_pattern(registry_id: &str, path_pattern: &str) -> WResult { + fn create_registry_or_fatal( + registry_id: &str, + path_pattern: &str, + non_fatal_errors: &mut Vec, + ) -> Result { + let mut registry = SemConvRegistry::new(registry_id); + for sc_entry in + glob::glob(path_pattern).map_err(|e| Error::InvalidRegistryPathPattern { + path_pattern: path_pattern.to_owned(), + error: e.to_string(), + })? + { + let path_buf = sc_entry.map_err(|e| Error::InvalidRegistryPathPattern { + path_pattern: path_pattern.to_owned(), + error: e.to_string(), + })?; + let (semconv_spec, nfes) = SemConvSpecWithProvenance::from_file(path_buf.as_path()) + .into_result_with_non_fatal()?; + registry.add_semconv_spec(semconv_spec); + non_fatal_errors.extend(nfes); + } + Ok(registry) + } + + let mut non_fatal_errors = vec![]; + + match create_registry_or_fatal(registry_id, path_pattern, &mut non_fatal_errors) { + Ok(registry) => WResult::with_non_fatal_errors(registry, non_fatal_errors), + Err(e) => WResult::FatalErr(e), } - Ok(registry) } /// Creates a semantic convention registry from the given list of @@ -121,9 +139,8 @@ impl SemConvRegistry { pub fn add_semconv_spec_from_file + Clone>( &mut self, path: P, - ) -> Result<(), Error> { - self.add_semconv_spec(SemConvSpecWithProvenance::from_file(path.clone())?); - Ok(()) + ) -> WResult<(), Error> { + SemConvSpecWithProvenance::from_file(path.clone()).map(|spec| self.add_semconv_spec(spec)) } /// Load and add a semantic convention string to the semantic convention registry. @@ -131,24 +148,22 @@ impl SemConvRegistry { &mut self, provenance: &str, spec: &str, - ) -> Result<(), Error> { - self.add_semconv_spec(SemConvSpecWithProvenance::from_string(provenance, spec)?); - Ok(()) + ) -> WResult<(), Error> { + SemConvSpecWithProvenance::from_string(provenance, spec) + .map(|spec| self.add_semconv_spec(spec)) } /// Loads and returns the semantic convention spec from a file. pub fn semconv_spec_from_file>( semconv_path: P, - ) -> Result<(String, SemConvSpec), Error> { + ) -> WResult<(String, SemConvSpec), Error> { let provenance = semconv_path.as_ref().display().to_string(); - let spec = SemConvSpec::from_file(semconv_path)?; - Ok((provenance, spec)) + SemConvSpec::from_file(semconv_path).map(|spec| (provenance, spec)) } /// Downloads and returns the semantic convention spec from an URL. - pub fn semconv_spec_from_url(sem_conv_url: &str) -> Result<(String, SemConvSpec), Error> { - let spec = SemConvSpec::from_url(sem_conv_url)?; - Ok((sem_conv_url.to_owned(), spec)) + pub fn semconv_spec_from_url(sem_conv_url: &str) -> WResult<(String, SemConvSpec), Error> { + SemConvSpec::from_url(sem_conv_url).map(|spec| (sem_conv_url.to_owned(), spec)) } /// Returns the number of semantic convention specs added in the semantic @@ -204,12 +219,15 @@ mod tests { #[test] fn test_try_from_path_pattern() { // Test with a valid path pattern - let registry = SemConvRegistry::try_from_path_pattern("test", "data/c*.yaml").unwrap(); + let registry = SemConvRegistry::try_from_path_pattern("test", "data/c*.yaml") + .into_result_failing_non_fatal() + .unwrap(); assert_eq!(registry.id(), "test"); assert_eq!(registry.semconv_spec_count(), 3); // Test with an invalid path pattern - let registry = SemConvRegistry::try_from_path_pattern("test", "data/c***.yml"); + let registry = SemConvRegistry::try_from_path_pattern("test", "data/c***.yml") + .into_result_failing_non_fatal(); assert!(registry.is_err()); assert!(matches!( registry.unwrap_err(), @@ -220,7 +238,8 @@ mod tests { #[test] fn test_semconv_spec_from_url() { let semconv_url = "https://raw.githubusercontent.com/open-telemetry/semantic-conventions/main/model/url.yaml"; - let result = SemConvRegistry::semconv_spec_from_url(semconv_url); + let result = + SemConvRegistry::semconv_spec_from_url(semconv_url).into_result_failing_non_fatal(); assert!(result.is_ok()); } @@ -303,19 +322,24 @@ mod tests { #[test] fn test_semconv_from_path_pattern() { - let mut registry = SemConvRegistry::try_from_path_pattern("test", "data/c*.yaml").unwrap(); + let mut registry = SemConvRegistry::try_from_path_pattern("test", "data/c*.yaml") + .into_result_failing_non_fatal() + .unwrap(); assert_eq!(registry.id(), "test"); assert_eq!(registry.semconv_spec_count(), 3); registry .add_semconv_spec_from_file("data/database.yaml") + .into_result_failing_non_fatal() .unwrap(); assert_eq!(registry.semconv_spec_count(), 4); } #[test] fn test_stats() { - let registry = SemConvRegistry::try_from_path_pattern("test", "data/c*.yaml").unwrap(); + let registry = SemConvRegistry::try_from_path_pattern("test", "data/c*.yaml") + .into_result_failing_non_fatal() + .unwrap(); let stats = registry.stats(); assert_eq!(stats.file_count, 3); assert_eq!(stats.group_count, 3); @@ -333,7 +357,9 @@ mod tests { #[test] fn test_unresolved_group_with_provenance_iter() { - let registry = SemConvRegistry::try_from_path_pattern("test", "data/c*.yaml").unwrap(); + let registry = SemConvRegistry::try_from_path_pattern("test", "data/c*.yaml") + .into_result_failing_non_fatal() + .unwrap(); let groups = registry .unresolved_group_with_provenance_iter() diff --git a/crates/weaver_semconv/src/semconv.rs b/crates/weaver_semconv/src/semconv.rs index dab00320..ddc255fa 100644 --- a/crates/weaver_semconv/src/semconv.rs +++ b/crates/weaver_semconv/src/semconv.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use std::fs::File; use std::io::BufReader; use std::path::Path; -use weaver_common::error::handle_errors; +use weaver_common::result::WResult; /// 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`]). @@ -38,26 +38,33 @@ impl SemConvSpec { /// # Returns /// /// The [`SemConvSpec`] or an [`Error`] if the semantic convention spec is invalid. - pub fn from_file>(path: P) -> Result { - let provenance = path.as_ref().display().to_string(); - - // Load and deserialize the semantic convention registry - let semconv_file = File::open(path).map_err(|e| Error::RegistryNotFound { - path_or_url: provenance.clone(), - error: e.to_string(), - })?; - let semconv_spec: SemConvSpec = serde_yaml::from_reader(BufReader::new(semconv_file)) - .map_err(|e| Error::InvalidSemConvSpec { - path_or_url: provenance.clone(), - line: e.location().map(|loc| loc.line()), - column: e.location().map(|loc| loc.column()), + pub fn from_file>(path: P) -> WResult { + fn from_file_or_fatal(path: &Path, provenance: &str) -> Result { + // Load and deserialize the semantic convention registry + let semconv_file = File::open(path).map_err(|e| Error::RegistryNotFound { + path_or_url: provenance.to_owned(), error: e.to_string(), })?; + serde_yaml::from_reader(BufReader::new(semconv_file)).map_err(|e| { + Error::InvalidSemConvSpec { + path_or_url: provenance.to_owned(), + line: e.location().map(|loc| loc.line()), + column: e.location().map(|loc| loc.column()), + error: e.to_string(), + } + }) + } - // Important note: the resolution process expects this step of validation to be done for - // each semantic convention spec. - semconv_spec.validate(&provenance)?; - Ok(semconv_spec) + let provenance = path.as_ref().display().to_string(); + + match from_file_or_fatal(path.as_ref(), &provenance) { + Ok(semconv_spec) => { + // Important note: the resolution process expects this step of validation to be done for + // each semantic convention spec. + semconv_spec.validate(&provenance) + } + Err(e) => WResult::FatalErr(e), + } } /// Create a new semantic convention spec from a string. @@ -69,19 +76,20 @@ impl SemConvSpec { /// # Returns /// /// The [`SemConvSpec`] or an [`Error`] if the semantic convention spec is invalid. - pub fn from_string(spec: &str) -> Result { - let semconv_spec: SemConvSpec = - serde_yaml::from_str(spec).map_err(|e| Error::InvalidSemConvSpec { - path_or_url: "".to_owned(), - line: None, - column: None, - error: e.to_string(), - })?; - - // Important note: the resolution process expects this step of validation to be done for - // each semantic convention spec. - semconv_spec.validate("")?; - Ok(semconv_spec) + pub fn from_string(spec: &str) -> WResult { + match serde_yaml::from_str::(spec).map_err(|e| Error::InvalidSemConvSpec { + path_or_url: "".to_owned(), + line: None, + column: None, + error: e.to_string(), + }) { + Ok(semconv_spec) => { + // Important note: the resolution process expects this step of validation to be done for + // each semantic convention spec. + semconv_spec.validate("") + } + Err(e) => WResult::FatalErr(e), + } } /// Create a new semantic convention spec from a URL. @@ -93,40 +101,48 @@ impl SemConvSpec { /// # Returns /// /// The [`SemConvSpec`] or an [`Error`] if the semantic convention spec is invalid. - pub fn from_url(semconv_url: &str) -> Result { - // Create a content reader from the semantic convention URL - let reader = ureq::get(semconv_url) - .call() - .map_err(|e| Error::RegistryNotFound { - path_or_url: semconv_url.to_owned(), - error: e.to_string(), - })? - .into_reader(); + pub fn from_url(semconv_url: &str) -> WResult { + fn from_url_or_fatal(semconv_url: &str) -> Result { + // Create a content reader from the semantic convention URL + let reader = ureq::get(semconv_url) + .call() + .map_err(|e| Error::RegistryNotFound { + path_or_url: semconv_url.to_owned(), + error: e.to_string(), + })? + .into_reader(); - // Deserialize the telemetry schema from the content reader - let semconv_spec: SemConvSpec = + // Deserialize the telemetry schema from the content reader serde_yaml::from_reader(reader).map_err(|e| Error::InvalidSemConvSpec { path_or_url: semconv_url.to_owned(), line: e.location().map(|loc| loc.line()), column: e.location().map(|loc| loc.column()), error: e.to_string(), - })?; + }) + } - // Important note: the resolution process expects this step of validation to be done for - // each semantic convention spec. - semconv_spec.validate(semconv_url)?; - Ok(semconv_spec) + match from_url_or_fatal(semconv_url) { + Ok(semconv_spec) => { + // Important note: the resolution process expects this step of validation to be done for + // each semantic convention spec. + semconv_spec.validate(semconv_url) + } + Err(e) => WResult::FatalErr(e), + } } - fn validate(&self, provenance: &str) -> Result<(), Error> { - let errors: Vec = self - .groups - .iter() - .filter_map(|group| group.validate(provenance).err()) - .collect(); + fn validate(self, provenance: &str) -> WResult { + let mut errors: Vec = vec![]; - handle_errors(errors)?; - Ok(()) + for group in &self.groups { + match group.validate(provenance) { + WResult::Ok(_) => {} + WResult::OkWithNFEs(_, errs) => errors.extend(errs), + WResult::FatalErr(e) => return WResult::FatalErr(e), + } + } + + WResult::with_non_fatal_errors(self, errors) } } @@ -141,10 +157,9 @@ impl SemConvSpecWithProvenance { /// /// The semantic convention with provenance or an error if the semantic /// convention spec is invalid. - pub fn from_file>(path: P) -> Result { + pub fn from_file>(path: P) -> WResult { let provenance = path.as_ref().display().to_string(); - let spec = SemConvSpec::from_file(path)?; - Ok(SemConvSpecWithProvenance { spec, provenance }) + SemConvSpec::from_file(path).map(|spec| SemConvSpecWithProvenance { spec, provenance }) } /// Creates a semantic convention spec with provenance from a string. @@ -158,9 +173,8 @@ impl SemConvSpecWithProvenance { /// /// The semantic convention with provenance or an error if the semantic /// convention spec is invalid. - pub fn from_string(provenance: &str, spec: &str) -> Result { - let spec = SemConvSpec::from_string(spec)?; - Ok(SemConvSpecWithProvenance { + pub fn from_string(provenance: &str, spec: &str) -> WResult { + SemConvSpec::from_string(spec).map(|spec| SemConvSpecWithProvenance { spec, provenance: provenance.to_owned(), }) @@ -170,25 +184,29 @@ impl SemConvSpecWithProvenance { #[cfg(test)] mod tests { use super::*; - use crate::Error::{InvalidAttribute, InvalidSemConvSpec, RegistryNotFound}; + use crate::Error::{ + InvalidAttribute, InvalidExampleWarning, InvalidSemConvSpec, RegistryNotFound, + }; use std::path::PathBuf; #[test] fn test_semconv_spec_from_file() { // Existing file let path = PathBuf::from("data/database.yaml"); - let semconv_spec = SemConvSpec::from_file(path).unwrap(); + let semconv_spec = SemConvSpec::from_file(path) + .into_result_failing_non_fatal() + .unwrap(); assert_eq!(semconv_spec.groups.len(), 11); // Non-existing file let path = PathBuf::from("data/non-existing.yaml"); - let semconv_spec = SemConvSpec::from_file(path); + let semconv_spec = SemConvSpec::from_file(path).into_result_failing_non_fatal(); assert!(semconv_spec.is_err()); assert!(matches!(semconv_spec.unwrap_err(), RegistryNotFound { .. })); // Invalid file structure let path = PathBuf::from("data/invalid-semconv.yaml"); - let semconv_spec = SemConvSpec::from_file(path); + let semconv_spec = SemConvSpec::from_file(path).into_result_failing_non_fatal(); assert!(semconv_spec.is_err()); assert!(matches!( semconv_spec.unwrap_err(), @@ -215,7 +233,9 @@ mod tests { brief: "description2" type: "int" "#; - let semconv_spec = SemConvSpec::from_string(spec).unwrap(); + let semconv_spec = SemConvSpec::from_string(spec) + .into_result_failing_non_fatal() + .unwrap(); assert_eq!(semconv_spec.groups.len(), 2); // Invalid yaml @@ -224,7 +244,7 @@ mod tests { - - "#; - let semconv_spec = SemConvSpec::from_string(spec); + let semconv_spec = SemConvSpec::from_string(spec).into_result_failing_non_fatal(); assert!(semconv_spec.is_err()); assert!(matches!( semconv_spec.unwrap_err(), @@ -245,7 +265,7 @@ mod tests { - id: "attr2" type: "int" "#; - let semconv_spec = SemConvSpec::from_string(spec); + let semconv_spec = SemConvSpec::from_string(spec).into_result_failing_non_fatal(); if let Err(Error::CompoundError(errors)) = semconv_spec { assert_eq!(errors.len(), 3); assert_eq!( @@ -259,7 +279,7 @@ mod tests { "This attribute is not deprecated and does not contain a brief field." .to_owned(), }, - InvalidAttribute { + InvalidExampleWarning { path_or_url: "".to_owned(), group_id: "group1".to_owned(), attribute_id: "attr1".to_owned(), @@ -286,12 +306,14 @@ mod tests { // Existing URL. The URL is a raw file from the semantic conventions repository. // This file is expected to be available. let semconv_url = "https://raw.githubusercontent.com/open-telemetry/semantic-conventions/main/model/url.yaml"; - let semconv_spec = SemConvSpec::from_url(semconv_url).unwrap(); + let semconv_spec = SemConvSpec::from_url(semconv_url) + .into_result_failing_non_fatal() + .unwrap(); assert!(!semconv_spec.groups.is_empty()); // Invalid semconv file let semconv_url = "https://raw.githubusercontent.com/open-telemetry/semantic-conventions/main/model/README.md"; - let semconv_spec = SemConvSpec::from_url(semconv_url); + let semconv_spec = SemConvSpec::from_url(semconv_url).into_result_failing_non_fatal(); assert!(semconv_spec.is_err()); assert!(matches!( semconv_spec.unwrap_err(), @@ -300,7 +322,7 @@ mod tests { // Non-existing URL (including both a leading underscore (which is not a valid domain) and a non-existing domain) let semconv_url = "http://_unknown.com.invalid/unknown-semconv.yaml"; - let semconv_spec = SemConvSpec::from_url(semconv_url); + let semconv_spec = SemConvSpec::from_url(semconv_url).into_result_failing_non_fatal(); assert!(semconv_spec.is_err()); assert!(matches!(semconv_spec.unwrap_err(), RegistryNotFound { .. })); } @@ -308,7 +330,9 @@ mod tests { #[test] fn test_semconv_spec_with_provenance_from_file() { let path = PathBuf::from("data/database.yaml"); - let semconv_spec = SemConvSpecWithProvenance::from_file(&path).unwrap(); + let semconv_spec = SemConvSpecWithProvenance::from_file(&path) + .into_result_failing_non_fatal() + .unwrap(); assert_eq!(semconv_spec.spec.groups.len(), 11); assert_eq!(semconv_spec.provenance, path.display().to_string()); } @@ -333,7 +357,9 @@ mod tests { type: "int" "#; - let semconv_spec = SemConvSpecWithProvenance::from_string(provenance, spec).unwrap(); + let semconv_spec = SemConvSpecWithProvenance::from_string(provenance, spec) + .into_result_failing_non_fatal() + .unwrap(); assert_eq!(semconv_spec.spec.groups.len(), 2); assert_eq!(semconv_spec.provenance, provenance); } diff --git a/crates/weaver_semconv_gen/Cargo.toml b/crates/weaver_semconv_gen/Cargo.toml index bc8467f6..3abcb149 100644 --- a/crates/weaver_semconv_gen/Cargo.toml +++ b/crates/weaver_semconv_gen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_semconv_gen" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/crates/weaver_semconv_gen/src/lib.rs b/crates/weaver_semconv_gen/src/lib.rs index 0c76ec05..4bba2de6 100644 --- a/crates/weaver_semconv_gen/src/lib.rs +++ b/crates/weaver_semconv_gen/src/lib.rs @@ -327,7 +327,8 @@ impl ResolvedSemconvRegistry { registry_repo: &RegistryRepo, ) -> Result { let registry_id = "semantic_conventions"; - let semconv_specs = SchemaResolver::load_semconv_specs(registry_repo)?; + let semconv_specs = + SchemaResolver::load_semconv_specs(registry_repo).into_result_failing_non_fatal()?; let mut registry = SemConvRegistry::from_semconv_specs(registry_id, semconv_specs); let schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry)?; let lookup = ResolvedSemconvRegistry { diff --git a/crates/weaver_version/Cargo.toml b/crates/weaver_version/Cargo.toml index 08dcb789..cc14083a 100644 --- a/crates/weaver_version/Cargo.toml +++ b/crates/weaver_version/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "weaver_version" -version = "0.9.0" +version = "0.9.1" authors.workspace = true repository.workspace = true license.workspace = true diff --git a/justfile b/justfile index 73ef51ed..b0295b8a 100644 --- a/justfile +++ b/justfile @@ -8,6 +8,7 @@ install: cargo install cargo-check-external-types cargo install git-cliff cargo install cargo-tarpaulin + cargo install cargo-nextest --locked pre-push-check: rustup update @@ -20,7 +21,7 @@ pre-push-check: # cargo clippy --workspace --all-features --all-targets -- -D warnings --allow deprecated cargo clippy --workspace --all-targets -- -D warnings --allow deprecated rm -rf crates/weaver_forge/observed_output/* - cargo test --all + cargo nextest run --all # [workaround] removed --all-features due to an issue in one of the dependency in Tantity (zstd-safe) # [ToDo LQ] Re-enable --all-features once the issue is resolved # cargo doc --workspace --all-features --no-deps --document-private-items diff --git a/src/registry/check.rs b/src/registry/check.rs index 568fae3c..4d16af19 100644 --- a/src/registry/check.rs +++ b/src/registry/check.rs @@ -2,22 +2,20 @@ //! Check a semantic convention registry. -use std::path::PathBuf; - use clap::Args; +use miette::Diagnostic; +use std::path::PathBuf; use weaver_cache::registry_path::RegistryPath; use weaver_cache::RegistryRepo; use weaver_checker::PolicyStage; use weaver_common::diagnostic::{DiagnosticMessages, ResultExt}; -use weaver_common::error::handle_errors; use weaver_common::Logger; use weaver_forge::registry::ResolvedRegistry; use weaver_semconv::registry::SemConvRegistry; use crate::registry::RegistryArgs; use crate::util::{ - check_policies, check_policy_stage, init_policy_engine, load_semconv_specs, - resolve_semconv_specs, + check_policy, check_policy_stage, init_policy_engine, load_semconv_specs, resolve_semconv_specs, }; use crate::{DiagnosticArgs, ExitDirectives}; @@ -80,10 +78,18 @@ pub(crate) fn command( // Load the semantic convention registry into a local registry repo. // No parsing errors should be observed. - let main_semconv_specs = load_semconv_specs(&main_registry_repo, logger.clone())?; + let main_semconv_specs = load_semconv_specs(&main_registry_repo, logger.clone()) + .capture_non_fatal_errors(&mut diag_msgs)?; let baseline_semconv_specs = baseline_registry_repo .as_ref() - .map(|repo| load_semconv_specs(repo, logger.clone())) + .map(|repo| { + // Baseline registry resolution should allow non-future features + // and warnings against it should be suppressed when evaluating + // against it as a "baseline". + load_semconv_specs(repo, logger.clone()) + .ignore(|e| matches!(e.severity(), Some(miette::Severity::Warning))) + .capture_non_fatal_errors(&mut diag_msgs) + }) .transpose()?; let mut policy_engine = if !args.skip_policies { @@ -100,13 +106,18 @@ pub(crate) fn command( // Check the policies against the semantic convention specifications before resolution. // All violations should be captured into an ongoing list of diagnostic messages which // will be combined with the final result of future stages. - // `check_policies` either returns `()` or diagnostic messages, and `capture_diag_msgs_into` updates the - // provided parameters with any diagnostic messages produced by `check_policies`. - // In this specific case, `capture_diag_msgs_into` returns either `Some(())` or `None` - // if diagnostic messages have been captured. Therefore, it is acceptable to ignore the result in this - // particular case. - _ = check_policies(policy_engine, &main_semconv_specs, logger.clone()) - .capture_diag_msgs_into(&mut diag_msgs); + check_policy(policy_engine, &main_semconv_specs) + .inspect(|_, violations| { + if let Some(violations) = violations { + logger.success(&format!( + "All `before_resolution` policies checked ({} violations found)", + violations.len() + )); + } else { + logger.success("No `before_resolution` policy violation"); + } + }) + .capture_non_fatal_errors(&mut diag_msgs)?; } let mut main_registry = @@ -132,23 +143,24 @@ pub(crate) fn command( .combine_diag_msgs_with(&diag_msgs)?; // Check the policies against the resolved registry (`PolicyState::AfterResolution`). - let errs = check_policy_stage::( + check_policy_stage::( policy_engine, PolicyStage::AfterResolution, ®istry_path.to_string(), &main_resolved_registry, &[], - ); - logger.success(&format!( - "All `after_resolution` policies checked ({} violations found)", - errs.len() - )); - - // Append the policy errors to the ongoing list of diagnostic messages and if there are - // any errors, return them immediately. - if let Err(err) = handle_errors(errs) { - diag_msgs.extend(err.into()); - } + ) + .inspect(|_, violations| { + if let Some(violations) = violations { + logger.success(&format!( + "All `after_resolution` policies checked ({} violations found)", + violations.len() + )); + } else { + logger.success("No `after_resolution` policy violation"); + } + }) + .capture_non_fatal_errors(&mut diag_msgs)?; if let (Some(baseline_registry_repo), Some(baseline_semconv_specs)) = (baseline_registry_repo, baseline_semconv_specs) @@ -169,23 +181,24 @@ pub(crate) fn command( .combine_diag_msgs_with(&diag_msgs)?; // Check the policies against the resolved registry (`PolicyState::AfterResolution`). - let errs = check_policy_stage( + check_policy_stage( policy_engine, PolicyStage::ComparisonAfterResolution, ®istry_path.to_string(), &main_resolved_registry, &[baseline_resolved_registry], - ); - logger.success(&format!( - "All `comparison_after_resolution` policies checked ({} violations found)", - errs.len() - )); - - // Append the policy errors to the ongoing list of diagnostic messages and if there are - // any errors, return them immediately. - if let Err(err) = handle_errors(errs) { - diag_msgs.extend(err.into()); - } + ) + .inspect(|_, violations| { + if let Some(violations) = violations { + logger.success(&format!( + "All `comparison_after_resolution` policies checked ({} violations found)", + violations.len() + )); + } else { + logger.success("No `comparison_after_resolution` policy violation"); + } + }) + .capture_non_fatal_errors(&mut diag_msgs)?; } if !diag_msgs.is_empty() { diff --git a/src/registry/generate.rs b/src/registry/generate.rs index 84e0a6c3..edb696fe 100644 --- a/src/registry/generate.rs +++ b/src/registry/generate.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; use clap::Args; +use miette::Diagnostic; use serde_yaml::Value; use weaver_cache::RegistryRepo; @@ -17,7 +18,7 @@ use weaver_forge::{OutputDirective, TemplateEngine, SEMCONV_JQ}; use weaver_semconv::registry::SemConvRegistry; use crate::registry::{Error, RegistryArgs}; -use crate::util::{check_policies, init_policy_engine, load_semconv_specs, resolve_semconv_specs}; +use crate::util::{check_policy, init_policy_engine, load_semconv_specs, resolve_semconv_specs}; use crate::{registry, DiagnosticArgs, ExitDirectives}; /// Parameters for the `registry generate` sub-command @@ -100,6 +101,7 @@ pub(crate) fn command( args.registry.registry )); + let mut diag_msgs = DiagnosticMessages::empty(); let params = generate_params(args)?; let mut registry_path = args.registry.registry.clone(); // Support for --registry-git-sub-dir (should be removed in the future) @@ -112,11 +114,24 @@ pub(crate) fn command( let registry_repo = RegistryRepo::try_new("main", ®istry_path)?; // Load the semantic convention registry into a local cache. - let semconv_specs = load_semconv_specs(®istry_repo, logger.clone())?; + let semconv_specs = load_semconv_specs(®istry_repo, logger.clone()) + .ignore(|e| matches!(e.severity(), Some(miette::Severity::Warning))) + .into_result_failing_non_fatal()?; if !args.skip_policies { let policy_engine = init_policy_engine(®istry_repo, &args.policies, false)?; - check_policies(&policy_engine, &semconv_specs, logger.clone())?; + check_policy(&policy_engine, &semconv_specs) + .inspect(|_, violations| { + if let Some(violations) = violations { + logger.success(&format!( + "All `before_resolution` policies checked ({} violations found)", + violations.len() + )); + } else { + logger.success("No `before_resolution` policy violation"); + } + }) + .capture_non_fatal_errors(&mut diag_msgs)?; } let mut registry = SemConvRegistry::from_semconv_specs(registry_id, semconv_specs); @@ -144,6 +159,10 @@ pub(crate) fn command( &OutputDirective::File, )?; + if !diag_msgs.is_empty() { + return Err(diag_msgs); + } + logger.success("Artifacts generated successfully"); Ok(ExitDirectives { exit_code: 0, diff --git a/src/registry/resolve.rs b/src/registry/resolve.rs index e7eed7db..6c3cf54a 100644 --- a/src/registry/resolve.rs +++ b/src/registry/resolve.rs @@ -14,8 +14,9 @@ use weaver_semconv::registry::SemConvRegistry; use crate::format::{apply_format, Format}; use crate::registry::RegistryArgs; -use crate::util::{check_policies, init_policy_engine, load_semconv_specs, resolve_semconv_specs}; +use crate::util::{check_policy, init_policy_engine, load_semconv_specs, resolve_semconv_specs}; use crate::{registry, DiagnosticArgs, ExitDirectives}; +use miette::Diagnostic; /// Parameters for the `registry resolve` sub-command #[derive(Debug, Args)] @@ -68,6 +69,7 @@ pub(crate) fn command( } logger.loading(&format!("Resolving registry `{}`", args.registry.registry)); + let mut diag_msgs = DiagnosticMessages::empty(); let mut registry_path = args.registry.registry.clone(); // Support for --registry-git-sub-dir (should be removed in the future) if let registry::RegistryPath::GitRepo { sub_folder, .. } = &mut registry_path { @@ -80,11 +82,24 @@ pub(crate) fn command( let registry_repo = RegistryRepo::try_new("main", ®istry_path)?; // Load the semantic convention registry into a local cache. - let semconv_specs = load_semconv_specs(®istry_repo, logger.clone())?; + let semconv_specs = load_semconv_specs(®istry_repo, logger.clone()) + .ignore(|e| matches!(e.severity(), Some(miette::Severity::Warning))) + .into_result_failing_non_fatal()?; if !args.skip_policies { let policy_engine = init_policy_engine(®istry_repo, &args.policies, false)?; - check_policies(&policy_engine, &semconv_specs, logger.clone())?; + check_policy(&policy_engine, &semconv_specs) + .inspect(|_, violations| { + if let Some(violations) = violations { + logger.success(&format!( + "All `before_resolution` policies checked ({} violations found)", + violations.len() + )); + } else { + logger.success("No `before_resolution` policy violation"); + } + }) + .capture_non_fatal_errors(&mut diag_msgs)?; } let mut registry = SemConvRegistry::from_semconv_specs(registry_id, semconv_specs); @@ -118,6 +133,10 @@ pub(crate) fn command( panic!("{}", e); }); + if !diag_msgs.is_empty() { + return Err(diag_msgs); + } + Ok(ExitDirectives { exit_code: 0, quiet_mode: args.output.is_none(), diff --git a/src/registry/search.rs b/src/registry/search.rs index 066fd6e0..8ef29f81 100644 --- a/src/registry/search.rs +++ b/src/registry/search.rs @@ -388,7 +388,9 @@ pub(crate) fn command( let registry_repo = RegistryRepo::try_new("main", ®istry_path)?; // Load the semantic convention registry into a local cache. - let semconv_specs = load_semconv_specs(®istry_repo, logger.clone())?; + let semconv_specs = load_semconv_specs(®istry_repo, logger.clone()) + .ignore(|e| matches!(e.severity(), Some(miette::Severity::Warning))) + .into_result_failing_non_fatal()?; let mut registry = SemConvRegistry::from_semconv_specs(registry_id, semconv_specs); let schema = resolve_semconv_specs(&mut registry, logger.clone())?; diff --git a/src/registry/stats.rs b/src/registry/stats.rs index bc382331..0669ae7c 100644 --- a/src/registry/stats.rs +++ b/src/registry/stats.rs @@ -6,6 +6,7 @@ use crate::registry::RegistryArgs; use crate::util::{load_semconv_specs, resolve_semconv_specs}; use crate::{registry, DiagnosticArgs, ExitDirectives}; use clap::Args; +use miette::Diagnostic; use weaver_cache::RegistryRepo; use weaver_common::diagnostic::DiagnosticMessages; use weaver_common::Logger; @@ -47,7 +48,9 @@ pub(crate) fn command( let registry_repo = RegistryRepo::try_new("main", ®istry_path)?; // Load the semantic convention registry into a local cache. - let semconv_specs = load_semconv_specs(®istry_repo, logger.clone())?; + let semconv_specs = load_semconv_specs(®istry_repo, logger.clone()) + .ignore(|e| matches!(e.severity(), Some(miette::Severity::Warning))) + .into_result_failing_non_fatal()?; let mut registry = SemConvRegistry::from_semconv_specs(registry_id, semconv_specs); display_semconv_registry_stats(®istry); diff --git a/src/util.rs b/src/util.rs index 8a055aa9..db5ce962 100644 --- a/src/util.rs +++ b/src/util.rs @@ -10,7 +10,7 @@ use weaver_cache::RegistryRepo; use weaver_checker::Error::{InvalidPolicyFile, PolicyViolation}; use weaver_checker::{Engine, Error, PolicyStage, SEMCONV_REGO}; use weaver_common::diagnostic::DiagnosticMessages; -use weaver_common::error::handle_errors; +use weaver_common::result::WResult; use weaver_common::Logger; use weaver_resolved_schema::ResolvedTelemetrySchema; use weaver_resolver::SchemaResolver; @@ -31,15 +31,15 @@ use weaver_semconv::semconv::SemConvSpec; pub(crate) fn load_semconv_specs( registry_repo: &RegistryRepo, log: impl Logger + Sync + Clone, -) -> Result, weaver_resolver::Error> { - let semconv_specs = SchemaResolver::load_semconv_specs(registry_repo)?; - log.success(&format!( - "`{}` semconv registry `{}` loaded ({} files)", - registry_repo.id(), - registry_repo.registry_path_repr(), - semconv_specs.len() - )); - Ok(semconv_specs) +) -> WResult, weaver_semconv::Error> { + SchemaResolver::load_semconv_specs(registry_repo).inspect(|semconv_specs, _| { + log.success(&format!( + "`{}` semconv registry `{}` loaded ({} files)", + registry_repo.id(), + registry_repo.registry_path_repr(), + semconv_specs.len() + )); + }) } /// Initializes the policy engine with policies from the registry and command line. @@ -91,17 +91,13 @@ pub(crate) fn init_policy_engine( /// * `policy_stage` - The policy stage to check. /// * `policy_file` - The policy file to check. /// * `input` - The input to check. -/// -/// # Returns -/// -/// A list of policy violations represented as errors. pub(crate) fn check_policy_stage( policy_engine: &mut Engine, policy_stage: PolicyStage, policy_file: &str, input: &T, data: &[U], -) -> Vec { +) -> WResult<(), Error> { let mut errors = vec![]; for d in data { @@ -133,7 +129,7 @@ pub(crate) fn check_policy_stage( error: e.to_string(), }), } - errors + WResult::with_non_fatal_errors((), errors) } /// Checks the policies of a semantic convention registry. @@ -150,11 +146,11 @@ pub(crate) fn check_policy_stage( pub(crate) fn check_policy( policy_engine: &Engine, semconv_specs: &[(String, SemConvSpec)], -) -> Result<(), Error> { +) -> WResult<(), Error> { // Check policies in parallel - let policy_errors = semconv_specs + let results = semconv_specs .par_iter() - .flat_map(|(path, semconv)| { + .map(|(path, semconv)| { // Create a local policy engine inheriting the policies // from the global policy engine let mut policy_engine = policy_engine.clone(); @@ -166,42 +162,19 @@ pub(crate) fn check_policy( &[], ) }) - .collect::>(); - - handle_errors(policy_errors)?; - Ok(()) -} - -/// Checks the policies of a semantic convention registry. -/// -/// # Arguments -/// -/// * `policy_engine` - The policy engine. -/// * `semconv_specs` - The semantic convention specifications to check. -/// * `logger` - The logger for logging messages. -/// -/// # Returns -/// -/// A `Result` which is `Ok` if all policies are checked successfully, -/// or `DiagnosticMessages` if any policy violations occur. -pub(crate) fn check_policies( - policy_engine: &Engine, - semconv_specs: &[(String, SemConvSpec)], - logger: impl Logger + Sync + Clone, -) -> Result<(), DiagnosticMessages> { - if policy_engine.policy_package_count() > 0 { - check_policy(policy_engine, semconv_specs).map_err(|e| { - if let Error::CompoundError(errors) = e { - DiagnosticMessages::from_errors(errors) - } else { - DiagnosticMessages::from_error(e) + .collect::>>(); + + let mut nfes = vec![]; + for result in results { + match result { + WResult::Ok(_) => {} + WResult::OkWithNFEs(_, errors) => { + nfes.extend(errors); } - })?; - logger.success("All `before_resolution` policies checked"); - } else { - logger.success("No `before_resolution` policy found"); + WResult::FatalErr(e) => return WResult::FatalErr(e), + } } - Ok(()) + WResult::with_non_fatal_errors((), nfes) } /// Resolves the semantic convention specifications and returns the resolved schema. diff --git a/test_data/compatibility_check.rego b/test_data/compatibility_check.rego index 8bcf9ea3..62f60f15 100644 --- a/test_data/compatibility_check.rego +++ b/test_data/compatibility_check.rego @@ -46,4 +46,4 @@ back_comp_violation(description, group_id, attr_id) := violation if { "group": group_id, "attr": attr_id, } -} \ No newline at end of file +} diff --git a/tests/resolution_process.rs b/tests/resolution_process.rs index 447efdb7..e29404d1 100644 --- a/tests/resolution_process.rs +++ b/tests/resolution_process.rs @@ -2,6 +2,8 @@ //! Integration tests for the resolution process. +use miette::Diagnostic; + use weaver_cache::registry_path::RegistryPath; use weaver_cache::RegistryRepo; use weaver_common::TestLogger; @@ -39,9 +41,12 @@ fn test_cli_interface() { let registry_repo = RegistryRepo::try_new("main", ®istry_path).unwrap_or_else(|e| { panic!("Failed to create the registry repo, error: {e}"); }); - let semconv_specs = SchemaResolver::load_semconv_specs(®istry_repo).unwrap_or_else(|e| { - panic!("Failed to load the semantic convention specs, error: {e}"); - }); + let semconv_specs = SchemaResolver::load_semconv_specs(®istry_repo) + .ignore(|e| matches!(e.severity(), Some(miette::Severity::Warning))) + .into_result_failing_non_fatal() + .unwrap_or_else(|e| { + panic!("Failed to load the semantic convention specs, error: {e}"); + }); let semconv_specs = SemConvRegistry::from_semconv_specs(registry_id, semconv_specs); // Check if the logger has reported any warnings or errors.