Rust first variant: for review only by scottgerring · Pull Request #2 · scottgerring/custom-labels

scottgerring · 2026-04-08T14:39:51Z

Update polarsignals custom-labels repo to the new TLS format.

Decisions of relevance:

Let's just use prost for protobuf because that is how regular consumers will use it (and we can lift it straight from libdd)
Both context mechanisms provide an API generic of platform, but no-op for non-Linux-64; this involved rejigging libdd code a little
Let's focus on reducing diff to libdd rather than custom-labels, which means we drop the complete C bindings (it becomes just an impl detail to provide C-style TLS)

Open Qs:

What do we do with the license headers?
Do we need to do anything special to donate from libdatadog to OTel?

To see the diff against the libdd code, try this link

ivoanjo

This is looking good! I think the main question to me is supporting the old api as well/keeping spin.rs as-it-was.

I kinda like the idea of showing off how the old api can be layered on the new format + providing a nice migration path for users of custom-labels (we can even ask claude to give it a stab) but let me know if you're not convinced, as I'm happy to approve as-is.

What do we do with the license headers?
Do we need to do anything special to donate from libdatadog to OTel?

custom-labels, libdatadog AND opentelemetry all use Apache v2 so they're license-compatible.

My read on 1 and 2 is that it's fine to keep the current copyrights (Datadog for files that came from libdatadog, and implicitly Polar Signals for the other files) and once this moves out of WIP we can check with all parties if we should just assign the copyright to OTel directly.

ivoanjo · 2026-04-09T08:49:02Z

This part I think it would be very nice to keep if possible -- e.g. retrofit the existing custom-labels code to use the new APIs, rather than forcing a full API rewrite.

This would provide a really nice way of testing against existing users of custom-labels -- just switch the dep and recompile; vs requiring a full API change; spin.rs would remain exactly the same, which would be a nice demo for how the new format is able to provide the same feature-set (or very close).

ACK ! addressed in subsequent commits as discussed offline

ivoanjo · 2026-04-09T08:54:46Z

+        // On x86-64, force TLSDESC dialect as required by the OTel spec.
+        // On aarch64, TLSDESC is already the default.
+        #[cfg(target_arch = "x86_64")]
+        build.flag("-mtls-dialect=gnu2");


Minor: Could we use mtls-dialect=gnu2 everywhere? E.g. does arm mind if we ask for the default? I think that would simplify a bit (and is one less sharp edge if someone tries to e.g. compile this on another arch)

nah, it fails on arm:

unrecognized argument in option '-mtls-dialect=gnu2', valid values are desc and trad.

we can do the opposite ("add this if you're not arm") but i'm not confident that it's going to work on other arches either. Seeing as we only promise to support arm64 / amd64 this is probably fine?

unrecognized argument in option '-mtls-dialect=gnu2', valid values are desc and trad.

Grumble grumble why are compiler people so bad at good/sane behaviors grumble grumble.

I guess let's keep it as-is, we can figure out later what to do with other architectures...

…nux-64 targets

scottgerring · 2026-04-09T11:57:10Z

    String::from_utf8(
-        rand::thread_rng()
-            .sample_iter(&Alphanumeric)
+        rand::rng()


because of dep update and rust's passion for prerelease crate versions

scottgerring · 2026-04-09T11:57:31Z

    loop {
-        custom_labels::with_label("l1", rand_str(), || {
-            custom_labels::with_label("l2", rand_str(), || loop {
+        otel_thread_ctx::custom_labels_adapter::with_label("l1", rand_str(), || {


can't do much about the namespace change; intentionally leaving the adapter "off to the side" as discussed rather than presenting a weird API

scottgerring · 2026-04-09T14:55:36Z

+        /// Mutate the currently attached record in-place with proper synchronization.
+        /// Sets `valid = 0` before calling `f`, and `valid = 1` after, with compiler
+        /// fences to prevent reordering. Returns `false` if no record is attached.
+        fn mutate_in_place(f: impl FnOnce(&mut ThreadContextRecord)) -> bool {


factored this out from update so we have the guards in one place and can use it for pop attr too

scottgerring · 2026-04-09T14:56:01Z


        pub fn update(_trace_id: [u8; 16], _span_id: [u8; 8], _attrs: &[(u8, &str)]) {}

+        pub fn pop_last_attr() {}


no-op for non-linux

scottgerring · 2026-04-09T14:57:01Z

+        resource: Some(Resource::default()),
+        extra_attributes: vec![
+            KeyValue {
+                key: "threadlocal.schema_version".into(),


add the core attributes.
fwiw i've also tested both variants are readable with our test reader for procCtx and TLS !

Remove unused files

5146a12

scottgerring force-pushed the sgg/rust-first branch 3 times, most recently from 688ced9 to 41ca8d2 Compare April 8, 2026 15:19

scottgerring commented Apr 9, 2026

View reviewed changes

Comment thread proto/opentelemetry/proto/common/v1/process_context.proto Outdated

scottgerring force-pushed the sgg/rust-first branch from 41ca8d2 to 532fb07 Compare April 9, 2026 06:45

ivoanjo reviewed Apr 9, 2026

View reviewed changes

scottgerring force-pushed the sgg/rust-first branch from 532fb07 to b087314 Compare April 9, 2026 10:42

scottgerring added 4 commits April 9, 2026 12:49

Update cargo.toml

21605f9

Update README

cfb67fb

bring process_context in from libdatadog

7b2cf58

bring thread context code from libdatadog

fd1ca5f

scottgerring force-pushed the sgg/rust-first branch 2 times, most recently from 46a66f8 to 148c923 Compare April 9, 2026 11:54

scottgerring added 5 commits April 9, 2026 13:55

integrate the thread context code from libdatadog

7f60994

generic-ify things out so we have a workable but no-op impl on non-li…

687f505

…nux-64 targets

update deps and cleanup

d558222

drop custom-labels API back in

0e52387

implement custom-labels API on top of new API

f9f2c08

scottgerring force-pushed the sgg/rust-first branch from 148c923 to 0c8d9df Compare April 9, 2026 11:55

scottgerring commented Apr 9, 2026

View reviewed changes

scottgerring force-pushed the sgg/rust-first branch from 0c8d9df to 85cca8a Compare April 9, 2026 14:48

scottgerring added 2 commits April 9, 2026 16:53

address ivo review comments

d014661

fmt

683634d

scottgerring force-pushed the sgg/rust-first branch from 85cca8a to 683634d Compare April 9, 2026 14:54

scottgerring commented Apr 9, 2026

View reviewed changes


		pub fn update(_trace_id: [u8; 16], _span_id: [u8; 8], _attrs: &[(u8, &str)]) {}

		pub fn pop_last_attr() {}

Conversation

scottgerring commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ivoanjo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivoanjo Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottgerring Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

scottgerring commented Apr 8, 2026 •

edited

Loading

ivoanjo Apr 9, 2026 •

edited

Loading

scottgerring Apr 9, 2026 •

edited

Loading