diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index b164e44c15..654149d655 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4393,11 +4393,13 @@ dependencies = [ "opentelemetry_sdk", "os_info", "pretty_assertions", + "rcgen", "reqwest 0.12.28", "seccompiler", "serde", "serde_json", "strum_macros 0.28.0", + "tempfile", "thiserror 2.0.18", "tokio", "tokio-tungstenite", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index c944e5db9c..18308d0703 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -16678,6 +16678,15 @@ } ] }, + "span_attributes": { + "additionalProperties": { + "type": "string" + }, + "type": [ + "object", + "null" + ] + }, "tool_result": { "anyOf": [ { @@ -16697,6 +16706,18 @@ "type": "null" } ] + }, + "tracestate": { + "additionalProperties": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + }, + "type": [ + "object", + "null" + ] } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 79920c59cb..546c07a354 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -12670,6 +12670,15 @@ } ] }, + "span_attributes": { + "additionalProperties": { + "type": "string" + }, + "type": [ + "object", + "null" + ] + }, "tool_result": { "anyOf": [ { @@ -12689,6 +12698,18 @@ "type": "null" } ] + }, + "tracestate": { + "additionalProperties": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + }, + "type": [ + "object", + "null" + ] } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json index 6f0b285cde..caa30bcab5 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json @@ -1294,6 +1294,15 @@ } ] }, + "span_attributes": { + "additionalProperties": { + "type": "string" + }, + "type": [ + "object", + "null" + ] + }, "tool_result": { "anyOf": [ { @@ -1313,6 +1322,18 @@ "type": "null" } ] + }, + "tracestate": { + "additionalProperties": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + }, + "type": [ + "object", + "null" + ] } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index 21124b8714..8100a9a56d 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index 133b65b61a..8a6ba438d1 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/OtelRequirements.ts b/codex-rs/app-server-protocol/schema/typescript/v2/OtelRequirements.ts index 6b1fa9d6bc..7279fe8746 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/OtelRequirements.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/OtelRequirements.ts @@ -4,4 +4,4 @@ import type { OtelExporter } from "./OtelExporter"; import type { OtelToolResultLogConfig } from "./OtelToolResultLogConfig"; -export type OtelRequirements = { tool_result: OtelToolResultLogConfig | null, log_user_prompt: boolean | null, log_agent_responses: boolean | null, log_guardian_assessments: boolean | null, environment: string | null, exporter: OtelExporter | null, trace_exporter: OtelExporter | null, metrics_exporter: OtelExporter | null, }; +export type OtelRequirements = { tool_result: OtelToolResultLogConfig | null, log_user_prompt: boolean | null, log_agent_responses: boolean | null, log_guardian_assessments: boolean | null, environment: string | null, exporter: OtelExporter | null, trace_exporter: OtelExporter | null, metrics_exporter: OtelExporter | null, span_attributes: { [key in string]?: string } | null, tracestate: { [key in string]?: { [key in string]?: string } } | null, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index 444fffde86..177d000775 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -513,6 +513,8 @@ pub struct OtelRequirements { pub exporter: Option, pub trace_exporter: Option, pub metrics_exporter: Option, + pub span_attributes: Option>, + pub tracestate: Option>>, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index ad2ea0028d..abb47d0bb3 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -711,3 +711,11 @@ managed fields. Configuration writes that overlap managed settings are rejected. Exporter header values are redacted in effective config, source layers, managed requirements, and write-override metadata. Runtime exporters retain the original values. + +Managed span attributes and tracestate metadata are included in effective config +and requirements reads. Unmanaged metadata remains writable. + +Tracestate requirements merge individual fields within each member. For example, +requiring `otel.tracestate.vendor.tenant` preserves an ordinary `vendor.region`. +Only required fields and edits to their ancestors are protected; sibling fields +retain their origins and remain writable. diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index fa443cb4ed..a0738698fc 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -1722,7 +1722,9 @@ log_agent_responses = true log_guardian_assessments = false exporter = "none" trace_exporter = "none" -metrics_exporter = "none""#, +metrics_exporter = "none" +span_attributes = { "service.name" = "managed-service" } +tracestate = { managed = { tenant = "enterprise" }, empty = {} }"#, ), ); for (key_path, managed) in [ @@ -1733,8 +1735,14 @@ metrics_exporter = "none""#, ("otel.exporter.otlp-http.endpoint", true), ("otel.trace_exporter.otlp-grpc.endpoint", true), ("otel.metrics_exporter.otlp-http.headers", true), + ("otel.span_attributes.\"service.name\"", true), + ("otel.tracestate.managed.tenant", true), ("otel.environment", false), ("otel.tool_result.max_bytes", true), + ("otel.span_attributes.\"service.version\"", false), + ("otel.tracestate.local.tenant", false), + ("otel.tracestate.managed.region", false), + ("otel.tracestate.empty.tenant", false), ] { let result = service .write_value(ConfigValueWriteParams { @@ -1829,7 +1837,9 @@ async fn read_projects_managed_otel_values_and_preserves_unmanaged_settings() { tool_result = { max_bytes = 1234 } log_user_prompt = true environment = "user" -exporter = "statsig""#, +exporter = "statsig" +span_attributes = { "service.name" = "user-service", "service.version" = "user-version" } +tracestate = { managed = { tenant = "user-tenant", region = "us" }, local = { tenant = "preserved" }, empty = { tenant = "also-preserved" } }"#, ) .expect("write user config"); let service = ConfigManager::new_for_tests( @@ -1839,7 +1849,9 @@ exporter = "statsig""#, CloudConfigBundleFixture::loader_with_enterprise_requirement( r#"[otel] log_user_prompt = false -exporter = "none""#, +exporter = "none" +span_attributes = { "service.name" = "managed-service" } +tracestate = { managed = { tenant = "enterprise" }, empty = {} }"#, ), ); let response = service @@ -1865,15 +1877,28 @@ exporter = "none""#, "exporter": "none", "trace_exporter": null, "metrics_exporter": null, - "span_attributes": null, - "tracestate": null, + "span_attributes": { + "service.name": "managed-service", + "service.version": "user-version", + }, + "tracestate": { + "managed": { "tenant": "enterprise", "region": "us" }, + "local": { "tenant": "preserved" }, + "empty": { "tenant": "also-preserved" }, + }, }), ); for (path, user_owned) in [ ("otel.log_user_prompt", false), ("otel.exporter", false), + ("otel.span_attributes.service.name", false), + ("otel.tracestate.managed.tenant", false), + ("otel.tracestate.managed.region", true), ("otel.environment", true), ("otel.tool_result.max_bytes", true), + ("otel.span_attributes.service.version", true), + ("otel.tracestate.local.tenant", true), + ("otel.tracestate.empty.tenant", true), ] { assert_eq!(response.origins.contains_key(path), user_owned, "{path}"); } diff --git a/codex-rs/app-server/src/otel_config.rs b/codex-rs/app-server/src/otel_config.rs index 12b79b01e2..c299989908 100644 --- a/codex-rs/app-server/src/otel_config.rs +++ b/codex-rs/app-server/src/otel_config.rs @@ -25,6 +25,8 @@ pub(crate) fn map_otel_requirements_to_api(otel: OtelRequirementsToml) -> OtelRe exporter: otel.exporter.map(map_otel_exporter_to_api), trace_exporter: otel.trace_exporter.map(map_otel_exporter_to_api), metrics_exporter: otel.metrics_exporter.map(map_otel_exporter_to_api), + span_attributes: otel.span_attributes, + tracestate: otel.tracestate, } } diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index fcaa7759d4..8f21fc0edf 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -599,6 +599,105 @@ async fn config_read_disables_guardian_v2_when_managed_config_requires_guardian_ Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn managed_otel_tracestate_merges_fields_and_protects_only_required_fields() -> Result<()> { + let codex_home = TempDir::new()?; + write_config( + &codex_home, + r#"[otel.tracestate.vendor] +tenant = "user" +region = "us" +"#, + )?; + std::fs::write( + codex_home.path().join("requirements.toml"), + "[otel.tracestate.vendor]\ntenant = \"enterprise\"\n", + )?; + let mut app_server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build_initialized_with_timeout(DEFAULT_READ_TIMEOUT) + .await?; + + let read_id = app_server + .send_config_read_request(ConfigReadParams { + include_layers: false, + cwd: None, + }) + .await?; + let response: ConfigReadResponse = + timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(read_id)).await??; + assert_eq!( + response.config.additional["otel"]["tracestate"], + json!({ "vendor": { "tenant": "enterprise", "region": "us" } }), + ); + assert!( + !response + .origins + .contains_key("otel.tracestate.vendor.tenant") + ); + assert!( + response + .origins + .contains_key("otel.tracestate.vendor.region") + ); + + let write_id = app_server + .send_config_value_write_request(ConfigValueWriteParams { + file_path: None, + key_path: "otel.tracestate.vendor.region".to_string(), + value: json!("eu"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await?; + let _: ConfigWriteResponse = + timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(write_id)).await??; + + for key_path in [ + "otel.tracestate.vendor.tenant", + "otel.tracestate.vendor", + "otel.tracestate", + ] { + let write_id = app_server + .send_config_value_write_request(ConfigValueWriteParams { + file_path: None, + key_path: key_path.to_string(), + value: json!("user"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await?; + let error: JSONRPCError = timeout( + DEFAULT_READ_TIMEOUT, + app_server.read_stream_until_error_message(RequestId::Integer(write_id)), + ) + .await??; + assert_eq!( + error + .error + .data + .as_ref() + .and_then(|data| data.get("config_write_error_code")), + Some(&json!("configRequirementReadonly")), + "{key_path}", + ); + } + let read_id = app_server + .send_config_read_request(ConfigReadParams { + include_layers: false, + cwd: None, + }) + .await?; + let response: ConfigReadResponse = + timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(read_id)).await??; + assert_eq!( + response.config.additional["otel"]["tracestate"], + json!({ "vendor": { "tenant": "enterprise", "region": "eu" } }), + ); + Ok(()) +} + #[test_case::test_case("json", "none"; "json_and_disabled_metrics")] #[test_case::test_case("binary", "statsig"; "binary_and_statsig_metrics")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -623,7 +722,9 @@ log_guardian_assessments = false environment = "managed" exporter = { otlp-http = { endpoint = "http://127.0.0.1:1/v1/logs", protocol = "$PROTOCOL", headers = { Authorization = "Bearer managed-token" }, tls = { ca-certificate = "ca.pem", client-certificate = "client.pem", client-private-key = "client-key.pem" } } } trace_exporter = { otlp-grpc = { endpoint = "http://127.0.0.1:4317", headers = { "x-api-key" = "managed-grpc-secret" }, tls = { ca-certificate = "ca.pem" } } } -metrics_exporter = "$METRICS_EXPORTER""# +metrics_exporter = "$METRICS_EXPORTER" +span_attributes = { "service.name" = "managed-service" } +tracestate = { managed = { tenant = "enterprise" } }"# .replace("$PROTOCOL", protocol) .replace("$METRICS_EXPORTER", metrics_exporter), )?; @@ -662,6 +763,8 @@ metrics_exporter = "$METRICS_EXPORTER""# }, }, "metrics_exporter": metrics_exporter, + "span_attributes": { "service.name": "managed-service" }, + "tracestate": { "managed": { "tenant": "enterprise" } }, }) ); // API clients must preserve certificate paths from servers on either OS. diff --git a/codex-rs/code-mode-host/src/main.rs b/codex-rs/code-mode-host/src/main.rs index fc7cedb848..8f823c6624 100644 --- a/codex-rs/code-mode-host/src/main.rs +++ b/codex-rs/code-mode-host/src/main.rs @@ -120,7 +120,7 @@ fn build_trace_provider(endpoint: &str) -> anyhow::Result { }, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), }) .map_err(|error| anyhow::anyhow!("failed to build code-mode host OTEL provider: {error}"))? diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index b487c4a1b7..0d1fcfdf98 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -1122,6 +1122,8 @@ pub struct OtelRequirementsToml { pub exporter: Option, pub trace_exporter: Option, pub metrics_exporter: Option, + pub span_attributes: Option>, + pub tracestate: Option>>, } impl OtelRequirementsToml { @@ -1134,6 +1136,17 @@ impl OtelRequirementsToml { && self.exporter.is_none() && self.trace_exporter.is_none() && self.metrics_exporter.is_none() + && !self.has_metadata() + } + + fn has_metadata(&self) -> bool { + self.span_attributes + .as_ref() + .is_some_and(|fields| !fields.is_empty()) + || self + .tracestate + .as_ref() + .is_some_and(|members| members.values().any(|fields| !fields.is_empty())) } } @@ -1847,6 +1860,11 @@ impl ConfigRequirementsToml { .find_map(|(is_managed, managed_path, field)| { (is_managed && config_paths_overlap(segments, managed_path)).then_some(field) }) + .or_else(|| { + otel.and_then(|otel| { + exact_otel_metadata_requirement_for_config_path(otel, segments) + }) + }) } } @@ -1862,6 +1880,15 @@ macro_rules! otel_merge_functions { requirements.get_or_insert_default().$field = Some(value.clone()); contributed = true; })+ + if higher_priority.has_metadata() { + let target = requirements.get_or_insert_default(); + merge_otel_metadata( + &mut target.span_attributes, + &mut target.tracestate, + higher_priority, + ); + contributed = true; + } contributed } @@ -1875,6 +1902,11 @@ macro_rules! otel_merge_functions { conflict |= config.$field.as_ref().is_some_and(|current| current != value); config.$field = Some(value.clone()); })+ + conflict |= merge_otel_metadata( + &mut config.span_attributes, + &mut config.tracestate, + requirements, + ); conflict } }; @@ -1891,6 +1923,70 @@ otel_merge_functions!( metrics_exporter, ); +fn merge_otel_metadata( + span_attributes: &mut Option>, + tracestate: &mut Option>>, + requirements: &OtelRequirementsToml, +) -> bool { + let mut conflict = false; + if let Some(attributes) = &requirements.span_attributes + && !attributes.is_empty() + { + let configured = span_attributes.get_or_insert_default(); + for (key, value) in attributes { + conflict |= configured + .insert(key.clone(), value.clone()) + .is_some_and(|configured| configured != *value); + } + } + if let Some(members) = &requirements.tracestate { + for (member, fields) in members.iter().filter(|(_, fields)| !fields.is_empty()) { + let configured = tracestate + .get_or_insert_default() + .entry(member.clone()) + .or_default(); + for (key, value) in fields { + conflict |= configured + .insert(key.clone(), value.clone()) + .is_some_and(|configured| configured != *value); + } + } + } + conflict +} + +fn exact_otel_metadata_requirement_for_config_path( + requirements: &OtelRequirementsToml, + segments: &[String], +) -> Option<&'static str> { + if requirements + .span_attributes + .as_ref() + .is_some_and(|attributes| { + attributes.keys().any(|key| { + config_paths_overlap(segments, &["otel", "span_attributes", key.as_str()]) + }) + }) + { + return Some("otel.span_attributes"); + } + + requirements + .tracestate + .as_ref() + .is_some_and(|members| { + members.iter().any(|(member, fields)| { + fields.keys().any(|field| { + config_paths_overlap( + segments, + &["otel", "tracestate", member.as_str(), field.as_str()], + ) + }) + }) + }) + .then_some("otel.tracestate") +} + fn config_paths_overlap(segments: &[String], managed_path: &[&str]) -> bool { segments .iter() @@ -2471,9 +2567,16 @@ mod tests { #[test_case::test_case(&["otel", "exporter", "otlp-http", "endpoint"], Some("otel.exporter"); "exporter_endpoint")] #[test_case::test_case(&["otel", "trace_exporter"], Some("otel.trace_exporter"); "trace_exporter")] #[test_case::test_case(&["otel", "metrics_exporter"], Some("otel.metrics_exporter"); "metrics_exporter")] - #[test_case::test_case(&["otel", "span_attributes", "service.name"], None; "unmanaged_span_attribute")] - #[test_case::test_case(&["otel", "tracestate", "user"], None; "unmanaged_tracestate")] - fn otel_requirements_project_and_lock_exact_scalar_and_exporter_values( + #[test_case::test_case(&["otel", "span_attributes"], Some("otel.span_attributes"); "span_attributes")] + #[test_case::test_case(&["otel", "span_attributes", "service.name"], Some("otel.span_attributes"); "span_attributes_service_name")] + #[test_case::test_case(&["otel", "span_attributes", "other"], None; "span_attributes_other")] + #[test_case::test_case(&["otel", "tracestate"], Some("otel.tracestate"); "tracestate")] + #[test_case::test_case(&["otel", "tracestate", "managed", "tenant"], Some("otel.tracestate"); "tracestate_managed_tenant")] + #[test_case::test_case(&["otel", "tracestate", "managed"], Some("otel.tracestate"); "tracestate_managed")] + #[test_case::test_case(&["otel", "tracestate", "managed", "region"], None; "tracestate_managed_region")] + #[test_case::test_case(&["otel", "tracestate", "other"], None; "tracestate_other")] + #[test_case::test_case(&["otel", "tracestate", "empty"], None; "tracestate_empty")] + fn otel_requirements_project_exact_values_and_lock_only_managed_map_entries( segments: &[&str], expected: Option<&str>, ) -> Result<()> { @@ -2486,6 +2589,8 @@ environment = "managed" exporter = "none" trace_exporter = "none" metrics_exporter = "none" +span_attributes = { "service.name" = "managed" } +tracestate = { managed = { tenant = "enterprise" }, empty = {} } "#, )?; let mut configured: ConfigToml = from_str( @@ -2497,8 +2602,8 @@ environment = "user" exporter = "statsig" trace_exporter = { otlp-grpc = { endpoint = "https://user.example" } } metrics_exporter = "statsig" -span_attributes = { "service.name" = "user" } -tracestate = { user = { tenant = "preserved" } } +span_attributes = { "service.name" = "user", other = "preserved" } +tracestate = { managed = { tenant = "user", region = "us" }, other = { keep = "yes" } } "#, )?; @@ -2514,8 +2619,8 @@ environment = "managed" exporter = "none" trace_exporter = "none" metrics_exporter = "none" -span_attributes = { "service.name" = "user" } -tracestate = { user = { tenant = "preserved" } } +span_attributes = { "service.name" = "managed", other = "preserved" } +tracestate = { managed = { tenant = "enterprise", region = "us" }, other = { keep = "yes" } } "# )? ); @@ -2527,11 +2632,17 @@ tracestate = { user = { tenant = "preserved" } } expected, ); - let empty: ConfigRequirementsToml = from_str("[otel]")?; - assert!(empty.is_empty()); - let mut configured = ConfigToml::default(); - empty.apply_exact_to_config(&mut configured); - assert_eq!(configured.otel, None); + for empty in [ + "[otel]", + "[otel.span_attributes]", + "[otel.tracestate.empty]", + ] { + let requirements: ConfigRequirementsToml = from_str(empty)?; + assert!(requirements.is_empty(), "{empty}"); + let mut configured = ConfigToml::default(); + requirements.apply_exact_to_config(&mut configured); + assert_eq!(configured.otel, None, "{empty}"); + } Ok(()) } diff --git a/codex-rs/config/src/requirements_layers/stack.rs b/codex-rs/config/src/requirements_layers/stack.rs index e37dce2dc8..10f31ae569 100644 --- a/codex-rs/config/src/requirements_layers/stack.rs +++ b/codex-rs/config/src/requirements_layers/stack.rs @@ -10,7 +10,8 @@ //! - `rules.prefix_rules` append high-priority rules first. //! - `hooks` append high-priority event groups first while failing closed on //! active managed-dir conflicts. -//! - `otel` replaces exact managed values, treating exporters atomically. +//! - `otel` replaces exact managed values, treats exporters atomically, and +//! merges span attributes and fields within each tracestate member by key. //! - `permissions.filesystem.deny_read` is a high-priority-first union across //! layers. //! - `auto_review.required_on_models` is a high-priority-first union across layers. diff --git a/codex-rs/config/src/requirements_layers/stack_tests.rs b/codex-rs/config/src/requirements_layers/stack_tests.rs index 64fe0dc958..e402ccafab 100644 --- a/codex-rs/config/src/requirements_layers/stack_tests.rs +++ b/codex-rs/config/src/requirements_layers/stack_tests.rs @@ -341,7 +341,7 @@ refresh_interval_ms = 12345 } #[test] -fn otel_requirements_compose_exact_values_and_atomic_exporters_with_relative_tls_paths() { +fn otel_requirements_compose_exact_values_atomic_exporters_and_metadata() { let low_dir = tempdir().expect("low-priority requirements directory"); let high_dir = tempdir().expect("high-priority requirements directory"); let low_base = AbsolutePathBuf::from_absolute_path(low_dir.path()).expect("absolute low base"); @@ -359,6 +359,8 @@ environment = "low" exporter = { otlp-http = { endpoint = "https://low.example", protocol = "json", headers = { stale = "low" }, tls = { ca-certificate = "low-ca.pem" } } } trace_exporter = { otlp-grpc = { endpoint = "https://low.example", tls = { client-certificate = "low-client.pem" } } } metrics_exporter = "statsig" +span_attributes = { shared = "low", lower = "preserved" } +tracestate = { shared = { tenant = "low", region = "us" }, lower = { tenant = "preserved" }, empty = {} } "#, ) .with_base_dir(low_base); @@ -372,12 +374,22 @@ log_user_prompt = false environment = "high" exporter = { otlp-http = { endpoint = "https://high.example", protocol = "binary", tls = { ca-certificate = "high-ca.pem" } } } metrics_exporter = "none" +span_attributes = { shared = "high", higher = "added" } +tracestate = { shared = { tenant = "replacement" }, lower = {}, higher = { tenant = "added" } } "#, ) .with_base_dir(high_base); let expected_source = RequirementSource::composite([high.source.clone(), low.source.clone()]); let composed = compose_requirements_for_hostname( - vec![low, high, layer("empty", "Empty", "[otel]")], + vec![ + low, + high, + layer( + "empty", + "Empty", + "[otel]\nspan_attributes = {}\ntracestate = { lower = {} }", + ), + ], /*hostname*/ None, ) .expect("compose OTEL requirements") @@ -394,6 +406,8 @@ environment = "high" exporter = { otlp-http = { endpoint = "https://high.example", protocol = "binary", tls = { ca-certificate = '$HIGH_CA' } } } trace_exporter = { otlp-grpc = { endpoint = "https://low.example", tls = { client-certificate = '$LOW_CLIENT' } } } metrics_exporter = "none" +span_attributes = { shared = "high", lower = "preserved", higher = "added" } +tracestate = { shared = { tenant = "replacement", region = "us" }, lower = { tenant = "preserved" }, higher = { tenant = "added" } } "# .replace( "$HIGH_CA", @@ -407,34 +421,25 @@ metrics_exporter = "none" composed.value, *expected_requirements(expected).otel.unwrap() ); + assert_eq!( + compose(vec![layer( + "mixed", + "Mixed", + "[otel]\nexporter = 'none'\nspan_attributes = {}\ntracestate = { empty = {} }", + )]), + Ok(Some(expected_requirements("[otel]\nexporter = 'none'"))), + ); - assert_eq!(compose(vec![layer("empty", "Empty", "[otel]")]), Ok(None)); -} - -#[test] -fn managed_otel_trace_metadata_is_rejected_with_its_requirements_source() { - for field in ["span_attributes", "tracestate"] { - let error = compose(vec![layer( - "req_metadata", - "Managed policy", - &format!("[otel]\n{field} = {{}}"), - )]) - .expect_err("metadata is unsupported"); - let RequirementsCompositionError::Parse { - layer_source, - message, - } = error - else { - panic!("expected a layer parse error: {error:?}"); - }; + for empty in [ + "[otel]", + "[otel.span_attributes]", + "[otel.tracestate.empty]", + ] { assert_eq!( - layer_source, - RequirementSource::EnterpriseManaged { - id: "req_metadata".to_string(), - name: "Managed policy".to_string(), - }, + compose(vec![layer("empty", "Empty", empty)]), + Ok(None), + "{empty}", ); - assert!(message.contains(&format!("unknown field `{field}`"))); } } diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 3edd406c5b..6ec41a99aa 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -662,7 +662,7 @@ pub struct OtelConfigToml { pub metrics_exporter: Option, /// Attributes to add to every exported trace span. - pub span_attributes: Option>, + pub span_attributes: Option>, /// Semicolon-separated `key:value` fields to upsert into W3C tracestate members. pub tracestate: Option>>, @@ -679,7 +679,7 @@ pub struct OtelConfig { pub exporter: OtelExporterKind, pub trace_exporter: OtelExporterKind, pub metrics_exporter: OtelExporterKind, - pub span_attributes: BTreeMap, + pub span_attributes: HashMap, pub tracestate: BTreeMap>, } @@ -694,7 +694,7 @@ impl Default for OtelConfig { exporter: OtelExporterKind::None, trace_exporter: OtelExporterKind::None, metrics_exporter: OtelExporterKind::Statsig, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), } } diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 1f5f0250a9..309e2d3d54 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -10400,7 +10400,7 @@ beta = "two" assert_eq!( config.otel.span_attributes, - BTreeMap::from([("example.trace_attr".to_string(), "enabled".to_string())]) + HashMap::from([("example.trace_attr".to_string(), "enabled".to_string())]) ); assert_eq!( config.otel.tracestate, @@ -10450,7 +10450,7 @@ alpha = "one\ntwo" assert_eq!(config.otel.environment, "test"); assert_eq!( config.otel.span_attributes, - BTreeMap::from([("example.trace_attr".to_string(), "enabled".to_string())]) + HashMap::from([("example.trace_attr".to_string(), "enabled".to_string())]) ); assert_eq!( config.otel.tracestate, @@ -13940,6 +13940,8 @@ environment = "production" exporter = { otlp-http = { endpoint = "https://managed.example/logs", protocol = "binary", headers = { Authorization = "managed-secret" } } } trace_exporter = "none" metrics_exporter = { otlp-grpc = { endpoint = "https://managed.example:4317" } } +span_attributes = { "deployment.environment" = "production", team = "platform" } +tracestate = { vendor = { tenant = "managed" }, managed = { required = "yes" } } "#, r#" [otel] @@ -13949,10 +13951,10 @@ environment = "production" exporter = { otlp-http = { endpoint = "https://managed.example/logs", protocol = "binary", headers = { Authorization = "managed-secret" } } } trace_exporter = "none" metrics_exporter = { otlp-grpc = { endpoint = "https://managed.example:4317" } } -span_attributes = { "deployment.environment" = "user", machine = "laptop" } -tracestate = { vendor = { tenant = "user", region = "us" }, other = { keep = "yes" } } +span_attributes = { "deployment.environment" = "production", machine = "laptop", team = "platform" } +tracestate = { vendor = { tenant = "managed", region = "us" }, other = { keep = "yes" }, managed = { required = "yes" } } "#, - true; "managed_exact_values_and_user_metadata" + true; "managed_exact_values_and_unmanaged_metadata" )] #[test_case::test_case( "[otel]\nenvironment = 'user'", "[otel]\ntool_result = { max_bytes = 1024 }", diff --git a/codex-rs/core/src/config/otel.rs b/codex-rs/core/src/config/otel.rs index 2710ebf9d5..dd75994063 100644 --- a/codex-rs/core/src/config/otel.rs +++ b/codex-rs/core/src/config/otel.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::collections::HashMap; use std::fmt::Display; use codex_config::types::DEFAULT_OTEL_ENVIRONMENT; @@ -40,16 +41,16 @@ pub(crate) fn resolve_config( } fn resolve_span_attributes( - span_attributes: Option>, + span_attributes: Option>, startup_warnings: &mut Vec, -) -> BTreeMap { +) -> HashMap { let Some(span_attributes) = span_attributes else { - return BTreeMap::new(); + return HashMap::new(); }; - let mut valid_attributes = BTreeMap::new(); + let mut valid_attributes = HashMap::new(); for (key, value) in span_attributes { - let attribute = BTreeMap::from([(key.clone(), value.clone())]); + let attribute = HashMap::from([(key.clone(), value.clone())]); if let Err(err) = codex_otel::validate_span_attributes(&attribute) { push_invalid_config_warning("otel.span_attributes", err, startup_warnings); continue; diff --git a/codex-rs/exec-server/src/server/process_otel_tests.rs b/codex-rs/exec-server/src/server/process_otel_tests.rs index e7142c3dcb..69588b7996 100644 --- a/codex-rs/exec-server/src/server/process_otel_tests.rs +++ b/codex-rs/exec-server/src/server/process_otel_tests.rs @@ -643,7 +643,7 @@ fn exported_logs( }, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), }) .expect("OTEL settings") diff --git a/codex-rs/otel/Cargo.toml b/codex-rs/otel/Cargo.toml index 3197eaa846..f1124348bd 100644 --- a/codex-rs/otel/Cargo.toml +++ b/codex-rs/otel/Cargo.toml @@ -67,6 +67,8 @@ opentelemetry_sdk = { workspace = true, features = [ "testing", ] } pretty_assertions = { workspace = true } +rcgen = { workspace = true } +tempfile = { workspace = true } [target.'cfg(unix)'.dev-dependencies] libc = { workspace = true } diff --git a/codex-rs/otel/README.md b/codex-rs/otel/README.md index b277fe3642..43287ecb45 100644 --- a/codex-rs/otel/README.md +++ b/codex-rs/otel/README.md @@ -39,7 +39,7 @@ let settings = OtelSettings { tls: None, }, metrics_exporter: OtelExporter::None, - span_attributes: std::collections::BTreeMap::new(), + span_attributes: std::collections::HashMap::new(), tracestate: std::collections::BTreeMap::new(), }; diff --git a/codex-rs/otel/src/config.rs b/codex-rs/otel/src/config.rs index 69ad3b62d1..761b913286 100644 --- a/codex-rs/otel/src/config.rs +++ b/codex-rs/otel/src/config.rs @@ -36,7 +36,7 @@ pub(crate) fn resolve_exporter(exporter: &OtelExporter) -> OtelExporter { } /// Validates configured span attributes before they are attached to exported spans. -pub fn validate_span_attributes(attributes: &BTreeMap) -> std::io::Result<()> { +pub fn validate_span_attributes(attributes: &HashMap) -> std::io::Result<()> { if attributes.keys().any(String::is_empty) { return Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, @@ -58,7 +58,7 @@ pub struct OtelSettings { pub trace_exporter: OtelExporter, pub metrics_exporter: OtelExporter, pub runtime_metrics: bool, - pub span_attributes: BTreeMap, + pub span_attributes: HashMap, pub tracestate: BTreeMap>, } diff --git a/codex-rs/otel/src/otlp.rs b/codex-rs/otel/src/otlp.rs index 09b9b01a5a..a20ad5d65b 100644 --- a/codex-rs/otel/src/otlp.rs +++ b/codex-rs/otel/src/otlp.rs @@ -130,7 +130,7 @@ fn build_http_client_inner( key_location.display() )) })?; - builder = builder.identity(identity).https_only(true); + builder = builder.use_rustls_tls().identity(identity).https_only(true); } (Some(_), None) | (None, Some(_)) => { return Err(config_error( @@ -234,7 +234,13 @@ fn config_error(message: impl Into) -> Box { #[cfg(test)] mod tests { use super::*; + use codex_http_client::HttpClientFactory; + use codex_http_client::OutboundProxyPolicy; use pretty_assertions::assert_eq; + use rcgen::CertifiedKey; + use rcgen::generate_simple_self_signed; + use std::fs; + use tempfile::TempDir; use tokio::runtime::Builder; #[test] @@ -274,4 +280,51 @@ mod tests { assert!(client.is_ok()); } + + #[test] + fn build_http_client_accepts_pem_client_identity() { + let (_temp_dir, tls) = write_test_mtls_config(); + + build_http_client(&tls, OTEL_EXPORTER_OTLP_TIMEOUT) + .expect("blocking HTTP client should accept PEM client identity"); + } + + #[tokio::test] + async fn build_async_http_client_accepts_pem_client_identity() { + let (_temp_dir, tls) = write_test_mtls_config(); + + let factory = HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault); + build_async_http_client(&factory, Some(&tls), OTEL_EXPORTER_OTLP_TIMEOUT) + .expect("async HTTP client should accept PEM client identity"); + } + + fn write_test_mtls_config() -> (TempDir, OtelTlsConfig) { + let temp_dir = tempfile::tempdir().expect("temporary TLS fixture directory"); + let CertifiedKey { cert, signing_key } = + generate_simple_self_signed(vec!["localhost".to_string()]) + .expect("test certificate should generate"); + let certificate_path = temp_dir.path().join("client.pem"); + let private_key_path = temp_dir.path().join("client-key.pem"); + fs::write(&certificate_path, cert.pem()).expect("client certificate should write"); + fs::write(&private_key_path, signing_key.serialize_pem()) + .expect("client private key should write"); + + ( + temp_dir, + OtelTlsConfig { + ca_certificate: Some( + AbsolutePathBuf::from_absolute_path(&certificate_path) + .expect("CA certificate path should be absolute"), + ), + client_certificate: Some( + AbsolutePathBuf::from_absolute_path(&certificate_path) + .expect("client certificate path should be absolute"), + ), + client_private_key: Some( + AbsolutePathBuf::from_absolute_path(&private_key_path) + .expect("client private key path should be absolute"), + ), + }, + ) + } } diff --git a/codex-rs/otel/src/provider.rs b/codex-rs/otel/src/provider.rs index d012b49d91..d04d107a0e 100644 --- a/codex-rs/otel/src/provider.rs +++ b/codex-rs/otel/src/provider.rs @@ -40,6 +40,7 @@ use opentelemetry_sdk::trace::TracerProviderBuilder; use opentelemetry_sdk::trace::span_processor_with_async_runtime::BatchSpanProcessor as TokioBatchSpanProcessor; use opentelemetry_semantic_conventions as semconv; use std::collections::BTreeMap; +use std::collections::HashMap; use std::error::Error; use std::io; use std::mem::ManuallyDrop; @@ -411,7 +412,7 @@ fn normalize_host_name(host_name: &str) -> Option { fn tracer_provider_builder( resource: &Resource, - span_attributes: BTreeMap, + span_attributes: HashMap, ) -> TracerProviderBuilder { let builder = SdkTracerProvider::builder().with_resource(resource.clone()); if span_attributes.is_empty() { @@ -429,7 +430,7 @@ fn tracer_provider_builder( /// per-span metadata, so they need to be attached before each span is exported. #[derive(Debug)] struct SpanAttributesProcessor { - attributes: BTreeMap, + attributes: HashMap, } impl SpanProcessor for SpanAttributesProcessor { @@ -536,7 +537,7 @@ fn build_logger( fn build_tracer_provider( resource: &Resource, exporter: &OtelExporter, - span_attributes: BTreeMap, + span_attributes: HashMap, factory: &codex_http_client::HttpClientFactory, ) -> Result> { let span_exporter = match crate::config::resolve_exporter(exporter) { @@ -876,7 +877,7 @@ mod tests { trace_exporter: OtelExporter::None, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), } } diff --git a/codex-rs/otel/tests/suite/buffered_operations_tests.rs b/codex-rs/otel/tests/suite/buffered_operations_tests.rs index 50789799d1..63b3dd8eda 100644 --- a/codex-rs/otel/tests/suite/buffered_operations_tests.rs +++ b/codex-rs/otel/tests/suite/buffered_operations_tests.rs @@ -14,6 +14,7 @@ use opentelemetry_sdk::metrics::data::AggregatedMetrics; use opentelemetry_sdk::metrics::data::MetricData; use pretty_assertions::assert_eq; use std::collections::BTreeMap; +use std::collections::HashMap; use std::time::Duration; fn install() -> MetricsClient { @@ -94,7 +95,7 @@ fn rejected_opt_out_preserves_recording_and_accepted_opt_out_survives_shutdown() }, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::from([(String::new(), "invalid".to_string())]), + span_attributes: HashMap::from([(String::new(), "invalid".to_string())]), tracestate: BTreeMap::new(), }; assert_eq!( diff --git a/codex-rs/otel/tests/suite/otlp_http_loopback.rs b/codex-rs/otel/tests/suite/otlp_http_loopback.rs index 3fd9249603..1548b88c87 100644 --- a/codex-rs/otel/tests/suite/otlp_http_loopback.rs +++ b/codex-rs/otel/tests/suite/otlp_http_loopback.rs @@ -411,7 +411,7 @@ fn otlp_http_exporter_sends_logs_to_collector() trace_exporter: OtelExporter::None, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), })? .expect("otel provider"); @@ -471,7 +471,7 @@ fn otel_provider_rejects_header_unsafe_configured_tracestate() { }, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::from([( "example".to_string(), BTreeMap::from([("alpha".to_string(), "one\ntwo".to_string())]), @@ -537,7 +537,7 @@ fn otlp_http_exporter_sends_traces_to_collector() }, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::from([( + span_attributes: HashMap::from([( "test.configured_attribute".to_string(), "configured-value".to_string(), )]), @@ -683,7 +683,7 @@ async fn otlp_http_exporter_sends_traces_to_collector_with_bounded_shutdown_in_t }, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), })? .expect("otel provider"); @@ -777,7 +777,7 @@ fn otlp_http_exporter_times_out_when_collector_stalls_during_bounded_shutdown() }, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), }) .expect("build otel provider") @@ -877,7 +877,7 @@ fn otlp_http_exporter_sends_traces_to_collector_in_current_thread_tokio_runtime( }, metrics_exporter: OtelExporter::None, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), }) .map_err(|err| err.to_string())? diff --git a/codex-rs/windows-sandbox-rs/src/wfp_setup.rs b/codex-rs/windows-sandbox-rs/src/wfp_setup.rs index 45e8dc2382..3cdc7d75d1 100644 --- a/codex-rs/windows-sandbox-rs/src/wfp_setup.rs +++ b/codex-rs/windows-sandbox-rs/src/wfp_setup.rs @@ -6,6 +6,7 @@ use codex_otel::OtelProvider; use codex_otel::OtelSettings; use codex_otel::StatsigMetricsSettings; use std::collections::BTreeMap; +use std::collections::HashMap; use std::path::Path; const WFP_SETUP_SERVICE_NAME: &str = "codex-windows-sandbox-setup"; @@ -58,7 +59,7 @@ fn build_wfp_metrics_provider( trace_exporter: OtelExporter::None, metrics_exporter: OtelExporter::Statsig, runtime_metrics: false, - span_attributes: BTreeMap::new(), + span_attributes: HashMap::new(), tracestate: BTreeMap::new(), }) .map_err(|err| anyhow::anyhow!("failed to initialize WFP setup metrics provider: {err}")) diff --git a/sdk/python/src/openai_codex/generated/v2_all.py b/sdk/python/src/openai_codex/generated/v2_all.py index 8234815ec7..1fb7406fbb 100644 --- a/sdk/python/src/openai_codex/generated/v2_all.py +++ b/sdk/python/src/openai_codex/generated/v2_all.py @@ -9207,8 +9207,10 @@ class OtelRequirements(BaseModel): log_guardian_assessments: bool | None = None log_user_prompt: bool | None = None metrics_exporter: OtelExporter | None = None + span_attributes: dict[str, Any] | None = None tool_result: OtelToolResultLogConfig | None = None trace_exporter: OtelExporter | None = None + tracestate: dict[str, Any] | None = None class OverriddenMetadata(BaseModel):