From ef13b4627cc8074df3d15a58b980ad5cb30f4945 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Wed, 1 Jul 2026 13:27:15 +0200 Subject: [PATCH 1/2] Expose ThreadLocalMetadata via process_discovery Replaces the flat `threadlocal_attribute_keys` field on the napi TracerMetadata with a `threadlocal_metadata` substruct mirroring the libdatadog Rust API. Lets callers set the schema-version string and extra process-context attributes (strings and 64-bit ints for now) alongside the attribute key map. libdd-library-config and libdd-trace-protobuf are pinned to the merge commit that introduced the new API (7cdeb7896e92d1ba38bde495934e112dac2eda25); swap back to a tagged release once one that includes it is published. This is a breaking change to the process_discovery.TracerMetadata constructor. Downstream (dd-trace-js) will need to migrate from passing threadlocalAttributeKeys directly to passing a threadlocalMetadata object. --- Cargo.lock | 5 +- crates/process_discovery/Cargo.toml | 6 ++- crates/process_discovery/src/lib.rs | 78 +++++++++++++++++++++++++---- test/process-discovery.js | 27 ++++++++-- 4 files changed, 97 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a8d1c7e..9649a7e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -812,7 +812,7 @@ dependencies = [ [[package]] name = "libdd-library-config" version = "2.0.0" -source = "git+https://github.com/DataDog/libdatadog.git?tag=v35.0.0#aa78483fba211c72ca3759c76318895f45e0184b" +source = "git+https://github.com/DataDog/libdatadog.git?rev=7cdeb7896e92d1ba38bde495934e112dac2eda25#7cdeb7896e92d1ba38bde495934e112dac2eda25" dependencies = [ "anyhow", "libc", @@ -884,7 +884,7 @@ dependencies = [ [[package]] name = "libdd-trace-protobuf" version = "3.0.2" -source = "git+https://github.com/DataDog/libdatadog.git?tag=v35.0.0#aa78483fba211c72ca3759c76318895f45e0184b" +source = "git+https://github.com/DataDog/libdatadog.git?rev=7cdeb7896e92d1ba38bde495934e112dac2eda25#7cdeb7896e92d1ba38bde495934e112dac2eda25" dependencies = [ "prost", "serde", @@ -1402,6 +1402,7 @@ version = "0.1.0" dependencies = [ "anyhow", "libdd-library-config", + "libdd-trace-protobuf", "napi", "napi-derive", ] diff --git a/crates/process_discovery/Cargo.toml b/crates/process_discovery/Cargo.toml index 4aa80ce..4404228 100644 --- a/crates/process_discovery/Cargo.toml +++ b/crates/process_discovery/Cargo.toml @@ -8,7 +8,11 @@ crate-type = ["cdylib", "rlib"] [dependencies] anyhow = "1" -libdd-library-config = { git = "https://github.com/DataDog/libdatadog.git", tag = "v35.0.0", features = ["otel-thread-ctx"] } +# Pointed at the merge commit that introduced ThreadLocalMetadata (caller-supplied +# schema version + extra process-context attributes). Swap back to a tagged release +# once one that includes 7cdeb7896e92d1ba38bde495934e112dac2eda25 is published. +libdd-library-config = { git = "https://github.com/DataDog/libdatadog.git", rev = "7cdeb7896e92d1ba38bde495934e112dac2eda25", features = ["otel-thread-ctx"] } +libdd-trace-protobuf = { git = "https://github.com/DataDog/libdatadog.git", rev = "7cdeb7896e92d1ba38bde495934e112dac2eda25" } napi = { version = "2" } napi-derive = { version = "2", default-features = false } diff --git a/crates/process_discovery/src/lib.rs b/crates/process_discovery/src/lib.rs index 3bf3516..cb77db9 100644 --- a/crates/process_discovery/src/lib.rs +++ b/crates/process_discovery/src/lib.rs @@ -2,6 +2,7 @@ use napi::{Error, Status}; use napi_derive::napi; use libdd_library_config::tracer_metadata; +use libdd_trace_protobuf::opentelemetry::proto::common::v1::any_value; #[napi] pub struct NapiAnonymousFileHandle { @@ -11,6 +12,44 @@ pub struct NapiAnonymousFileHandle { #[napi] impl NapiAnonymousFileHandle {} +/// Additional OTel process-context attribute the threadlocal writer wants to +/// publish alongside the key map (e.g. language-runtime layout constants). Set +/// exactly one of `string_value` / `int_value` — the other variants of OTel's +/// `AnyValue` (bool, double, bytes, array, kvlist) are not yet exposed. An +/// entry with neither field set is silently dropped. +#[derive(Clone)] +#[napi(object)] +pub struct ExtraAttribute { + pub key: String, + pub string_value: Option, + pub int_value: Option, +} + +/// Thread-level context metadata the tracer wants to publish as part of the +/// OTel process context. When present on a [`TracerMetadata`], drives the +/// `threadlocal.*` block in the emitted process context; when absent, no such +/// block is emitted. +#[derive(Clone)] +#[napi(object)] +pub struct ThreadLocalMetadata { + /// Ordered list of attribute key names for thread-level OTEP-4947 context + /// records. Wire key indices index into this list. libdatadog implicitly + /// prepends `datadog.local_root_span_id` at wire index 0, so entry 0 here + /// is wire key index 1. + pub attribute_keys: Vec, + + /// Value of the `threadlocal.schema_version` attribute. Identifies the + /// on-the-wire record schema (e.g. `"tlsdesc_v1_dev"` for libdatadog's own + /// TLSDESC writer, `"nodejs_v1_dev"` for a Node.js writer). Defaults to + /// `"tlsdesc_v1_dev"` when omitted. + pub schema_version: Option, + + /// Extra `threadlocal.*` attributes to publish alongside the key map (e.g. + /// V8 layout constants a Node.js reader needs to walk from the discovery + /// TLS symbol into the record). + pub extra_attributes: Vec, +} + #[napi(constructor)] pub struct TracerMetadata { pub runtime_id: Option, @@ -21,16 +60,33 @@ pub struct TracerMetadata { pub service_version: Option, pub process_tags: Option, pub container_id: Option, - /// Ordered list of attribute key names for thread-level OTEP-4947 - /// context records. Key indices on the wire index into this list. - /// libdatadog's OTel process-context conversion prepends the - /// implicit `datadog.local_root_span_id` entry at wire index 0, so - /// callers should only set their additional keys here — entry 0 in - /// this list corresponds to wire key index 1. - /// - /// `null`/omitted (the default) disables the thread-context-related - /// attributes in the OTel process context entirely. - pub threadlocal_attribute_keys: Option>, + /// Optional thread-level context metadata; see [`ThreadLocalMetadata`]. + /// `null`/omitted (the default) disables the `threadlocal.*` block in the + /// emitted OTel process context entirely. + pub threadlocal_metadata: Option, +} + +fn convert_extra_attribute(ea: &ExtraAttribute) -> Option<(String, any_value::Value)> { + let value = if let Some(s) = &ea.string_value { + any_value::Value::StringValue(s.clone()) + } else if let Some(i) = ea.int_value { + any_value::Value::IntValue(i) + } else { + return None; + }; + Some((ea.key.clone(), value)) +} + +fn convert_threadlocal_metadata(tlm: &ThreadLocalMetadata) -> tracer_metadata::ThreadLocalMetadata { + tracer_metadata::ThreadLocalMetadata { + attribute_keys: tlm.attribute_keys.clone(), + schema_version: tlm.schema_version.clone(), + extra_attributes: tlm + .extra_attributes + .iter() + .filter_map(convert_extra_attribute) + .collect(), + } } #[napi] @@ -46,7 +102,7 @@ pub fn store_metadata(data: &TracerMetadata) -> napi::Result Date: Wed, 1 Jul 2026 17:55:30 +0200 Subject: [PATCH 2/2] Reject ExtraAttribute with none or both value fields set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review feedback: the previous behavior silently preferred string_value when both were set and silently dropped the entry when neither was — both are shapes a caller likely didn't mean. Return an InvalidArg napi::Error instead, matching the 'exactly one of' docstring. Test both paths. --- crates/process_discovery/src/lib.rs | 53 ++++++++++++++++++++--------- test/process-discovery.js | 36 ++++++++++++++++++++ 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/crates/process_discovery/src/lib.rs b/crates/process_discovery/src/lib.rs index cb77db9..6e042e8 100644 --- a/crates/process_discovery/src/lib.rs +++ b/crates/process_discovery/src/lib.rs @@ -15,8 +15,8 @@ impl NapiAnonymousFileHandle {} /// Additional OTel process-context attribute the threadlocal writer wants to /// publish alongside the key map (e.g. language-runtime layout constants). Set /// exactly one of `string_value` / `int_value` — the other variants of OTel's -/// `AnyValue` (bool, double, bytes, array, kvlist) are not yet exposed. An -/// entry with neither field set is silently dropped. +/// `AnyValue` (bool, double, bytes, array, kvlist) are not yet exposed. +/// Passing both set or neither set is rejected as invalid input. #[derive(Clone)] #[napi(object)] pub struct ExtraAttribute { @@ -66,27 +66,44 @@ pub struct TracerMetadata { pub threadlocal_metadata: Option, } -fn convert_extra_attribute(ea: &ExtraAttribute) -> Option<(String, any_value::Value)> { - let value = if let Some(s) = &ea.string_value { - any_value::Value::StringValue(s.clone()) - } else if let Some(i) = ea.int_value { - any_value::Value::IntValue(i) - } else { - return None; +fn convert_extra_attribute(ea: &ExtraAttribute) -> napi::Result<(String, any_value::Value)> { + let value = match (&ea.string_value, ea.int_value) { + (Some(s), None) => any_value::Value::StringValue(s.clone()), + (None, Some(i)) => any_value::Value::IntValue(i), + (Some(_), Some(_)) => { + return Err(Error::new( + Status::InvalidArg, + format!( + "ExtraAttribute {:?}: exactly one of stringValue / intValue must be set, both are", + ea.key, + ), + )); + } + (None, None) => { + return Err(Error::new( + Status::InvalidArg, + format!( + "ExtraAttribute {:?}: exactly one of stringValue / intValue must be set, neither is", + ea.key, + ), + )); + } }; - Some((ea.key.clone(), value)) + Ok((ea.key.clone(), value)) } -fn convert_threadlocal_metadata(tlm: &ThreadLocalMetadata) -> tracer_metadata::ThreadLocalMetadata { - tracer_metadata::ThreadLocalMetadata { +fn convert_threadlocal_metadata( + tlm: &ThreadLocalMetadata, +) -> napi::Result { + Ok(tracer_metadata::ThreadLocalMetadata { attribute_keys: tlm.attribute_keys.clone(), schema_version: tlm.schema_version.clone(), extra_attributes: tlm .extra_attributes .iter() - .filter_map(convert_extra_attribute) - .collect(), - } + .map(convert_extra_attribute) + .collect::>()?, + }) } #[napi] @@ -102,7 +119,11 @@ pub fn store_metadata(data: &TracerMetadata) -> napi::Result process_discovery.storeMetadata(bad_metadata_neither), + /neither is/, +) + +// Setting both stringValue and intValue is also a caller error — the intent +// is ambiguous, so reject. +const bad_metadata_both = new process_discovery.TracerMetadata( + '7938685c-19dd-490f-b9b3-8aae4c22f89a', + '1.0.0', + 'my_hostname', + undefined, undefined, undefined, undefined, undefined, + { + attributeKeys: [], + schemaVersion: undefined, + extraAttributes: [{ key: 'threadlocal.bogus', stringValue: 's', intValue: 1 }], + }, +) +assert.throws( + () => process_discovery.storeMetadata(bad_metadata_both), + /both are/, +) + if (process.platform === 'linux') { const contains_datadog_memfd = (fds) => { for (const fd in fds) {