diff --git a/CHANGELOG.md b/CHANGELOG.md index e6057e67b..88bd5e4bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. - Add support for metrics in Live Check. ([#728](https://github.com/open-telemetry/weaver/pull/728) by @jerbly) - Fix #750 - Dual registry resolves incorrectly. ([#753](https://github.com/open-telemetry/weaver/pull/753) by @lquerel) +- Fail on unstructured `deprecated` note (behind `--future` flag) ([#737](https://github.com/open-telemetry/weaver/pull/737) by @lmolkova) # [0.15.0] - 2025-05-01 diff --git a/crates/weaver_codegen_test/semconv_registry/registry/deprecated/network.yaml b/crates/weaver_codegen_test/semconv_registry/registry/deprecated/network.yaml index bc1f2ab4d..b602651aa 100644 --- a/crates/weaver_codegen_test/semconv_registry/registry/deprecated/network.yaml +++ b/crates/weaver_codegen_test/semconv_registry/registry/deprecated/network.yaml @@ -6,55 +6,74 @@ groups: attributes: - id: net.sock.peer.name type: string - deprecated: "Removed." + deprecated: + reason: obsoleted + note: > + Removed, no replacement at this time. stability: experimental brief: Deprecated, no replacement at this time. examples: ['/var/my.sock'] - id: net.sock.peer.addr type: string - deprecated: "Replaced by `network.peer.address`." + deprecated: + reason: renamed + renamed_to: network.peer.address stability: experimental brief: Deprecated, use `network.peer.address`. examples: ['192.168.0.1'] - id: net.sock.peer.port type: int - deprecated: "Replaced by `network.peer.port`." + deprecated: + reason: renamed + renamed_to: network.peer.port stability: experimental examples: [65531] brief: Deprecated, use `network.peer.port`. - id: net.peer.name type: string - deprecated: "Replaced by `server.address` on client spans and `client.address` on server spans." + deprecated: + reason: uncategorized + note: "Replaced by `server.address` on client spans and `client.address` on server spans." stability: experimental brief: Deprecated, use `server.address` on client spans and `client.address` on server spans. examples: ['example.com'] - id: net.peer.port type: int - deprecated: "Replaced by `server.port` on client spans and `client.port` on server spans." + deprecated: + reason: uncategorized + note: "Replaced by `server.port` on client spans and `client.port` on server spans." stability: experimental brief: Deprecated, use `server.port` on client spans and `client.port` on server spans. examples: [8080] - id: net.host.name type: string - deprecated: "Replaced by `server.address`." + deprecated: + reason: renamed + renamed_to: server.address stability: experimental brief: Deprecated, use `server.address`. examples: ['example.com'] - id: net.host.port type: int - deprecated: "Replaced by `server.port`." + deprecated: + reason: renamed + renamed_to: server.port stability: experimental brief: Deprecated, use `server.port`. examples: [8080] - id: net.sock.host.addr type: string - deprecated: "Replaced by `network.local.address`." + deprecated: + reason: renamed + renamed_to: network.local.address stability: experimental brief: Deprecated, use `network.local.address`. examples: ['/var/my.sock'] - id: net.sock.host.port type: int - deprecated: "Replaced by `network.local.port`." + deprecated: + reason: renamed + renamed_to: network.local.port stability: experimental brief: Deprecated, use `network.local.port`. examples: [8080] @@ -84,18 +103,24 @@ groups: value: "other" stability: experimental brief: 'Something else (non IP-based).' - deprecated: "Replaced by `network.transport`." + deprecated: + reason: renamed + renamed_to: network.transport stability: experimental brief: Deprecated, use `network.transport`. - id: net.protocol.name type: string - deprecated: "Replaced by `network.protocol.name`." + deprecated: + reason: renamed + renamed_to: network.protocol.name stability: experimental brief: Deprecated, use `network.protocol.name`. examples: ['amqp', 'http', 'mqtt'] - id: net.protocol.version type: string - deprecated: "Replaced by `network.protocol.version`." + deprecated: + reason: renamed + renamed_to: network.protocol.version stability: experimental brief: Deprecated, use `network.protocol.version`. examples: '3.1.1' diff --git a/crates/weaver_forge/data/exporter.yaml b/crates/weaver_forge/data/exporter.yaml index cd5da11cb..2790cdea3 100644 --- a/crates/weaver_forge/data/exporter.yaml +++ b/crates/weaver_forge/data/exporter.yaml @@ -23,12 +23,16 @@ groups: - id: otel.library.name stability: stable type: string - deprecated: use the `otel.scope.name` attribute. + deprecated: + reason: renamed + renamed_to: otel.scope.name brief: examples: ['io.opentelemetry.contrib.mongodb'] - id: otel.library.version stability: stable type: string - deprecated: use the `otel.scope.version` attribute. + deprecated: + reason: renamed + renamed_to: otel.scope.version brief: examples: ['1.0.0'] \ No newline at end of file diff --git a/crates/weaver_forge/expected_output/test/resource/library.md b/crates/weaver_forge/expected_output/test/resource/library.md index 5bba80113..e84f39af6 100644 --- a/crates/weaver_forge/expected_output/test/resource/library.md +++ b/crates/weaver_forge/expected_output/test/resource/library.md @@ -22,7 +22,7 @@ Brief: Span attributes used by non-OTLP exporters to represent OpenTelemetry Sco - Examples: [ "io.opentelemetry.contrib.mongodb", ] -- Deprecated: use the `otel.scope.name` attribute. +- Deprecated: Replaced by `otel.scope.name`. - Stability: Stable @@ -38,7 +38,7 @@ Brief: Span attributes used by non-OTLP exporters to represent OpenTelemetry Sco - Examples: [ "1.0.0", ] -- Deprecated: use the `otel.scope.version` attribute. +- Deprecated: Replaced by `otel.scope.version`. - Stability: Stable diff --git a/crates/weaver_forge/src/extensions/otel.rs b/crates/weaver_forge/src/extensions/otel.rs index 70d5ed5a6..a33c3e246 100644 --- a/crates/weaver_forge/src/extensions/otel.rs +++ b/crates/weaver_forge/src/extensions/otel.rs @@ -851,7 +851,7 @@ mod tests { Ctx { attr }, ) .unwrap(), - "uncategorized: Replaced by new_name." + "unspecified: Replaced by new_name." ); // --------------------------------------------------------------------- diff --git a/crates/weaver_live_check/src/advice.rs b/crates/weaver_live_check/src/advice.rs index 30d51879a..4a23c2b20 100644 --- a/crates/weaver_live_check/src/advice.rs +++ b/crates/weaver_live_check/src/advice.rs @@ -53,7 +53,9 @@ fn deprecated_to_value(deprecated: &Deprecated) -> Value { match deprecated { Deprecated::Renamed { .. } => Value::String("renamed".to_owned()), Deprecated::Obsoleted { .. } => Value::String("obsoleted".to_owned()), - Deprecated::Uncategorized { .. } => Value::String("uncategorized".to_owned()), + Deprecated::Uncategorized { .. } | Deprecated::Unspecified { .. } => { + Value::String("uncategorized".to_owned()) + } } } diff --git a/crates/weaver_resolved_schema/src/lib.rs b/crates/weaver_resolved_schema/src/lib.rs index 6a7536f52..b7b7fa325 100644 --- a/crates/weaver_resolved_schema/src/lib.rs +++ b/crates/weaver_resolved_schema/src/lib.rs @@ -329,7 +329,7 @@ impl ResolvedTelemetrySchema { }, ); } - Deprecated::Uncategorized { note } => { + Deprecated::Unspecified { note } | Deprecated::Uncategorized { note } => { changes.add_change( SchemaItemType::RegistryAttributes, SchemaItemChange::Uncategorized { @@ -410,7 +410,7 @@ impl ResolvedTelemetrySchema { }, ); } - Deprecated::Uncategorized { note } => { + Deprecated::Unspecified { note } | Deprecated::Uncategorized { note } => { changes.add_change( schema_item_type, SchemaItemChange::Uncategorized { diff --git a/crates/weaver_semconv/data/exporter.yaml b/crates/weaver_semconv/data/exporter.yaml index 2248a4591..592255ce3 100644 --- a/crates/weaver_semconv/data/exporter.yaml +++ b/crates/weaver_semconv/data/exporter.yaml @@ -25,12 +25,16 @@ groups: - id: name stability: stable type: string - deprecated: use the `otel.scope.name` attribute. + deprecated: + reason: renamed + renamed_to: otel.scope.name brief: examples: ['io.opentelemetry.contrib.mongodb'] - id: version stability: stable type: string - deprecated: use the `otel.scope.version` attribute. + deprecated: + reason: renamed + renamed_to: otel.scope.version brief: examples: ['1.0.0'] \ No newline at end of file diff --git a/crates/weaver_semconv/data/invalid-stability.yaml b/crates/weaver_semconv/data/invalid-stability.yaml index 8ef1926a2..ef6edc488 100644 --- a/crates/weaver_semconv/data/invalid-stability.yaml +++ b/crates/weaver_semconv/data/invalid-stability.yaml @@ -17,5 +17,7 @@ groups: When observed from the server side, and when communicating through an intermediary, `client.address` SHOULD represent the client address behind any intermediaries (e.g. proxies) if it's available. examples: ['/tmp/my.sock', '10.1.2.80'] - deprecated: use xyz instead + deprecated: + reason: renamed + renamed_to: xyz stability: stable \ No newline at end of file diff --git a/crates/weaver_semconv/src/deprecated.rs b/crates/weaver_semconv/src/deprecated.rs index 90f5c7f27..c8a038578 100644 --- a/crates/weaver_semconv/src/deprecated.rs +++ b/crates/weaver_semconv/src/deprecated.rs @@ -43,6 +43,13 @@ pub enum Deprecated { /// The note to provide more context about the deprecation. note: String, }, + + /// This variant is used to capture old, unstructured deprecated "string". + /// Used for backward-compatibility only. + Unspecified { + /// The note to provide more context about the deprecation. + note: String, + }, } /// Custom deserialization function to handle both old and new formats. @@ -72,7 +79,7 @@ where where E: de::Error, { - Ok(Deprecated::Uncategorized { + Ok(Deprecated::Unspecified { note: value.to_owned(), }) } @@ -172,7 +179,8 @@ impl Display for Deprecated { let text = match self { Deprecated::Renamed { note, .. } | Deprecated::Obsoleted { note } - | Deprecated::Uncategorized { note } => note, + | Deprecated::Uncategorized { note } + | Deprecated::Unspecified { note } => note, }; write!(f, "{}", text) } @@ -192,14 +200,14 @@ mod tests { fn test_deser_and_to_string() { let yaml_data = r#" - deprecated: 'Replaced by `jvm.buffer.memory.used`.' -- deprecated: +- deprecated: reason: obsoleted - deprecated: reason: renamed renamed_to: foo.unique_id - deprecated: reason: uncategorized - note: This field is deprecated for some complex reasons. + note: This field is deprecated for some complex reasons. - deprecated: reason: renamed renamed_to: foo.unique_id @@ -210,7 +218,7 @@ mod tests { assert_eq!(items.len(), 5); assert_eq!( items[0].deprecated, - Some(Deprecated::Uncategorized { + Some(Deprecated::Unspecified { note: "Replaced by `jvm.buffer.memory.used`.".to_owned() }) ); diff --git a/crates/weaver_semconv/src/group.rs b/crates/weaver_semconv/src/group.rs index d7955c284..129a3b34a 100644 --- a/crates/weaver_semconv/src/group.rs +++ b/crates/weaver_semconv/src/group.rs @@ -256,6 +256,14 @@ impl GroupSpec { } } + if matches!(self.deprecated, Some(Deprecated::Unspecified { .. })) { + errors.push(Error::UnstructuredDeprecatedProperty { + path_or_url: path_or_url.to_owned(), + id: self.id.clone(), + error: "Unstructured deprecated note is not supported on groups.".to_owned(), + }); + } + // Validates the attributes. for attribute in &self.attributes { match attribute { @@ -316,6 +324,15 @@ impl GroupSpec { } } } + + if matches!(deprecated, Some(Deprecated::Unspecified { .. })) { + errors.push(Error::UnstructuredDeprecatedProperty { + path_or_url: path_or_url.to_owned(), + id: attribute.id(), + error: "Unstructured deprecated note is not supported on attributes." + .to_owned(), + }); + } } AttributeSpec::Ref { .. } => {} } @@ -602,7 +619,7 @@ mod tests { CompoundError, InvalidAttributeAllowCustomValues, InvalidAttributeWarning, InvalidExampleWarning, InvalidGroup, InvalidGroupMissingExtendsOrAttributes, InvalidGroupMissingType, InvalidGroupStability, InvalidGroupUsesPrefix, InvalidMetric, - InvalidSpanMissingSpanKind, + InvalidSpanMissingSpanKind, UnstructuredDeprecatedProperty, }; use super::*; @@ -753,6 +770,23 @@ mod tests { ], ), ), result); + + // Group deprecated is set to unspecified + group.name = Some("test".to_owned()); + group.span_kind = None; + group.events = vec![]; + group.deprecated = Some(Deprecated::Unspecified { + note: "note".to_owned(), + }); + let result = group.validate("").into_result_failing_non_fatal(); + assert_eq!( + Err(UnstructuredDeprecatedProperty { + path_or_url: "".to_owned(), + id: "test".to_owned(), + error: "Unstructured deprecated note is not supported on groups.".to_owned(), + },), + result + ); } #[test] @@ -990,6 +1024,33 @@ mod tests { },), result ); + + // Deprecated is set to unspecified. + group.attributes = vec![AttributeSpec::Id { + id: "test".to_owned(), + r#type: AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::String), + brief: Some("brief".to_owned()), + stability: Some(Stability::Stable), + deprecated: Some(Deprecated::Unspecified { + note: "note".to_owned(), + }), + examples: Some(Examples::String("test".to_owned())), + tag: None, + requirement_level: Default::default(), + sampling_relevant: None, + note: "".to_owned(), + annotations: None, + role: Default::default(), + }]; + let result = group.validate("").into_result_failing_non_fatal(); + assert_eq!( + Err(UnstructuredDeprecatedProperty { + path_or_url: "".to_owned(), + id: "test".to_owned(), + error: "Unstructured deprecated note is not supported on attributes.".to_owned(), + },), + result + ); } #[test] diff --git a/crates/weaver_semconv/src/lib.rs b/crates/weaver_semconv/src/lib.rs index 3c5dd00e4..efa7c4b52 100644 --- a/crates/weaver_semconv/src/lib.rs +++ b/crates/weaver_semconv/src/lib.rs @@ -295,6 +295,20 @@ pub enum Error { error: String, }, + /// This indicates that deprecated property is invalid + #[error( + "The `deprecated` property in `{id}` is invalid. {error}\nProvenance: {path_or_url:?}" + )] + #[diagnostic(severity(Warning))] + UnstructuredDeprecatedProperty { + /// The path or URL of the semantic convention asset. + path_or_url: String, + /// The group id of the attribute. + id: String, + /// The reason of the error. + error: String, + }, + /// A container for multiple errors. #[error("{:?}", format_errors(.0))] CompoundError(#[related] Vec), diff --git a/src/registry/check.rs b/src/registry/check.rs index 10bed333e..9ef445be3 100644 --- a/src/registry/check.rs +++ b/src/registry/check.rs @@ -220,8 +220,9 @@ mod tests { + 12 /* allow_custom_values */ + 3 /* missing stability on enum members */ + 13 /* before resolution */ - + 3 /* metric after resolution */ - + 9 /* http after resolution */ + + 3 /* metric after resolution */ + + 9 /* http after resolution */ + + 1 /* deprecated string note */ ); } } diff --git a/tests/registry_check.rs b/tests/registry_check.rs index 43e600a89..ff0aa1876 100644 --- a/tests/registry_check.rs +++ b/tests/registry_check.rs @@ -50,5 +50,6 @@ fn test_cli_interface() { // - 13 violations before resolution // - 3 violations for metrics after resolution // - 9 violations for http after resolution - assert_eq!(json_value.len(), 42); + // - 1 deprecated string note + assert_eq!(json_value.len(), 43); } diff --git a/tests/registry_generate.rs b/tests/registry_generate.rs index ce650c239..1fdec1cfb 100644 --- a/tests/registry_generate.rs +++ b/tests/registry_generate.rs @@ -46,5 +46,5 @@ fn test_cli_interface() { let stdout = String::from_utf8(output.stdout).expect("Invalid UTF-8"); let json_value: Vec = serde_json::from_str(&stdout).expect("Invalid JSON"); // We expect 42 policy violations. - assert_eq!(json_value.len(), 42); + assert_eq!(json_value.len(), 43); }