feat: replace v1 custom labels with OTEP 4947 TLS record format by scottgerring · Pull Request #1 · scottgerring/custom-labels

scottgerring · 2026-03-31T13:39:45Z

Summary

Replaces v1 string-based labelset format with OTEP 4947 compact TLS record format (trace context + inline attributes)
Adds process context module (OTEP 4719) for cross-process context sharing via memfd/mmap
Adds OTel TLS Rust module with record builder, reader, and key table configuration

scottgerring · 2026-04-01T07:36:08Z

@@ -1,165 +1,83 @@
+/**


When we forked originally, we grabbed this when it was still C. I've switched back to C because we don't need anything from C++ and it keeps things simpler.

The C++-related incantations might come in handy if someone wants to pull this file from a C++ project (and they might not want to have to pull in a C compiler just for these bits), so I can see some usefulness in them eventually (the process context reference impl does this).

Having said that, we can re-add it any time so I think we're good with the simplification for now. (And in particular, if we want to keep it clean in both C and C++ we probably want a setup like the process context where we test-build with both, as I found that it's very easy to do a change that makes building with a C++ compiler unhappy)

scottgerring · 2026-04-01T07:36:24Z

@@ -1,281 +0,0 @@
-// addon.c


Removed all the js stuff as its out of scope for us

scottgerring · 2026-04-01T07:36:43Z

@@ -0,0 +1,13 @@
+[package]


I've had to make this a standalone crate so we can emit the build instructions

yannham

I've reviewed by commit, so apologies of some of them are outdated/irrelevant.

yannham · 2026-04-01T13:40:47Z

+) {
+    custom_labels_tl_record_t *old_record = otel_thread_ctx_v1;
+    BARRIER;
+    otel_thread_ctx_v1 = new_record;


Does this store need to be either volatile or atomic (or rather, should the otel_thread_ctx_v1 be a volatile sig_atomic_t ?) Otherwise I believe the store might be optimized away, or the signal handler might come in the middle of a non-atomic store to the variable (all of that is quite theoretical: we do read back the value in our case, so I don't think it'll be optimized away, and usize stores are atomic on all reasonable platforms, but I've grown very afraid of what advanced optimizing compilers can do 😅 )

so I don't think it'll be optimized away, and usize stores are atomic on all reasonable platforms
Famous last words and no harm in being overly cautious here I think!

@yannham : It looks like sig_atomic_t is typically a 32 bit integer? I've tried __thread _Atomic(custom_labels_tl_record_t *) otel_thread_ctx_v1 . I think this captures the spirit of your comment. Unfortunately, bindgen chokes on this, but we don't use this from the Rust side of things, so I can use the preprocessor to hide it when bindgen runs.

This was a bit more hectic than I expected; lmk what you think!

Answered on another channel, but short answer, this works as well 🙂

I don't think we need any barrier here? Either the reader observes the write before or after; this is a pointer after all, and we document the semantics as "equivalent to a signal handler" (e.g. a thread observes its own things).

TL;DR: I think some barrier is required (or some ordering), at least it would be in Rust. But I agree that the second barrier is not needed.

The semantics of signal handlers is a bit different than code running of the same program running in the same thread, basically because the compiler is oblivious to it. So what could happen is something like:

custom_labels_tl_record_t *record = malloc(...); record->xxx = yyy; // other stores... custom_labels_set_current_record(record);

the function (say without the barrier) then gets inlined, so the compiler sees:

custom_labels_tl_record_t *record = malloc(...); record->xxx = yyy; // other stores... custom_labels_tl_record_t *old_record = otel_thread_ctx_v1; otel_thread_ctx_v1 = record;

Those are just a bunch of stores with no atomics around (there's actually one read for old_record, but it doesn't change the reasoning), so nothing prevents them from being re-ordered, say as:

custom_labels_tl_record_t *record = malloc(...); custom_labels_tl_record_t *old_record = otel_thread_ctx_v1; // moved up otel_thread_ctx_v1 = record; // moved up record->xxx = yyy; // other stores...

Now if your signal handler comes in at L3 it will see an uninitialized record.

Using an atomic with relaxed order isn't sufficient because it doesn't create any synchronization. The compiler is still free to re-order stuff past or before it. However a release barrier just before setting the pointer (as the first BARRIER here), or setting otel_thread_ctx_v1 atomically with release order should prevent that. With the caveat that I don't know what's the exact semantics of C's BARRIER here, but if it is at least as strong as a C++ compiler-only release fence, this should be good.

Yup, good call, I missed that.

In theory the reader would be able to adapt (since it's expected to use safe memory reading APIs), and when it tried to interpret the data it would probably realize it doesn't make sense. In practice it's much better if we avoid that happening altogether, and the cost of this barrier should be really low so it's definitely worth it.

ivoanjo

I gave it a pass but didn't finish. Need to switch gears for a bit!

ivoanjo · 2026-04-07T11:02:47Z

@@ -1,4 +1,5 @@
 /target
+.idea


THE FORBIDDEN DATADOG FRUIT 👀 (Sorry couldn't resist)

True story: I won a jetbrains license at a conference last week, and immediately used it to restore my access to rust rover

ivoanjo · 2026-04-07T11:05:18Z

+    // dynamic-list is Linux-only
+    #[cfg(target_os = "linux")]
+    println!("cargo:rustc-link-arg=-Wl,--dynamic-list=./dlist");


This change is slightly weird -- isn't all of custom-labels linux-only?

You're right, it is. I thought I needed this at some point to have some modicum of LSP/build access on mac but it doesn't seem necessary anymore so it can definitely go.

ivoanjo · 2026-04-07T11:07:09Z

 [package]
 name = "custom-labels"
-version = "0.4.5"
+version = "0.5.0"
 edition = "2021"
 license = "Apache-2.0"
 keywords = ["profiling", "pprof", "polarsignals", "parca"]
 description = "Custom labels for profilers"
 homepage = "https://polarsignals.com"
 repository = "https://github.com/polarsignals/custom-labels"
 readme = "README.md"


Should we maybe update these? E.g. maybe call it custom-labels-otep-wip or something like that? (and then change the metadata)

I don't think custom-labels really works now that it's "labels and some fixed metadata"; similar to the TLS var rename.

How about otel-thread-ctx

Sounds good! We can always adjust later if needed (e.g. before this becomes "stable")

ivoanjo · 2026-04-07T11:13:17Z


 ## Acknowledgements

+* This is a fork of [polarsignals/custom-labels](https://github.com/polarsignals/custom-labels), extended to support the formalisation of the thread-local context sharing mechanism in [OTEP 4947](https://github.com/open-telemetry/opentelemetry-specification/pull/4947).


I was thinking we could call this out in a bit of a louder form at the beginning of the README. Something along the lines of

"This is a temporary fork of https://github.com/polarsignals/custom-labels to match up with the open-telemetry/opentelemetry-specification#4947 . It is experimental. This work will be incorporated/moved back upstream once the OTEP is accepted. Thanks to Polar signals for providing a great shoulder for us to stand on -- this work is (C) Polar Signals ..."

...or something like that?

ivoanjo · 2026-04-07T11:14:14Z

I spotted a few more files that we should probably remove: GOVERNANCE.md, tsconfig.json. The Makefile is also outdated, although I'd also suggest removing and we can always re-add later.

ivoanjo · 2026-04-07T13:12:09Z

+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_roundtrip_string_values() {
+        let ctx = ProcessContext::new()
+            .with_resource("service.name", "my-service")
+            .with_resource("service.version", "1.2.3")
+            .with_resource("deployment.environment", "production");
+
+        let encoded = encode(&ctx).unwrap();


So I'll be honest and say I didn't look at the encoder very closely -- I think if we do change to a full decoder, the tests below kinda have us covered for "output of encoder is valid protobuf".

ivoanjo · 2026-04-07T13:14:01Z

+/// Maximum varint value (14 bits)
+pub const UINT14_MAX: u16 = 16383;


Minor: I think this could be local to the encoder (this limitation is actually in the encoder itself, not protobuf)

ivoanjo · 2026-04-07T13:15:00Z

+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum Value {
+    /// String value (AnyValue.string_value)
+    String(String),
+    /// Integer value (AnyValue.int_value)
+    Int(i64),
+    /// Key-value list (AnyValue.kvlist_value)
+    KvList(Vec<KeyValue>),
+    /// Array value (AnyValue.array_value)
+    Array(Vec<Value>),
+}


Minor: Since this implementation exists only for the thread context, I'd probably just simplify it even more -- support only strings and arrays which is actually the only thing we're making use of right now.

ivoanjo · 2026-04-07T13:16:40Z

+#[cfg(target_os = "linux")]
+use std::ptr;
+#[cfg(target_os = "linux")]
+use std::sync::atomic::{AtomicU64, fence, Ordering};
+#[cfg(target_os = "linux")]
+use libc::timespec;
+
+use tracing::info;
+
+#[cfg(target_os = "linux")]
+use std::sync::Mutex;
+
+#[cfg(target_os = "linux")]
+use libc::{
+    c_void, close, ftruncate, madvise, mmap, munmap, prctl, sysconf, MADV_DONTFORK, MAP_ANONYMOUS,
+    MAP_FAILED, MAP_PRIVATE, MAP_SHARED, PROT_READ, PROT_WRITE, _SC_PAGESIZE,
+};
+
+// prctl constants for naming anonymous mappings (Linux 5.17+)
+#[cfg(target_os = "linux")]
+const PR_SET_VMA: i32 = 0x53564d41;
+#[cfg(target_os = "linux")]
+const PR_SET_VMA_ANON_NAME: u64 = 0;
+
+// memfd_create flags
+#[cfg(target_os = "linux")]
+const MFD_CLOEXEC: libc::c_uint = 0x0001;
+#[cfg(target_os = "linux")]
+const MFD_ALLOW_SEALING: libc::c_uint = 0x0002;
+#[cfg(target_os = "linux")]
+const MFD_NOEXEC_SEAL: libc::c_uint = 0x0008; // Linux 6.3+
+
+/// Wrapper for memfd_create syscall
+#[cfg(target_os = "linux")]


All this #[cfg(target_os = "linux")] is kinda redundant; is it maybe possible to do it at the file or crate level?

ivoanjo · 2026-04-07T13:19:19Z

+/// Get the mapping size (1 page, as per PR #34)
+#[cfg(target_os = "linux")]
+fn mapping_size() -> Result<usize> {
+    let page_size = unsafe { sysconf(_SC_PAGESIZE) };
+    if page_size < 4096 {
+        return Err(Error::MappingFailed("failed to get page size".to_string()));
+    }
+    Ok(page_size as usize)
+}


This... is weird -- is this 34 a reference to open-telemetry/sig-profiling#34? We actually don't need page size anymore, that PR was where we removed it?

refactor: convert customlabels.cpp back to pure C

01bc18e

scottgerring force-pushed the feat/otel-ctx-v1 branch from 083d07d to e2405a2 Compare March 31, 2026 13:46

scottgerring changed the title ~~feat: replace v1 custom labels with v2 TLS record format~~ feat: replace v1 custom labels with OTEP 4947 TLS record format Mar 31, 2026

scottgerring force-pushed the feat/otel-ctx-v1 branch from e2405a2 to 7895f14 Compare March 31, 2026 13:51

scottgerring added 5 commits March 31, 2026 16:46

feat: replace v1 labelset format with OTEP 4947 TLS record format

41deb75

feat: replace v1 Rust API with OTEP 4947 TLS record module

5b91031

feat: add process context (OTEP 4719) and TLS config extension

60e0b77

docs: update spin example for OTEP 4947 API

dea0679

Remove reader code

cd16c6e

scottgerring force-pushed the feat/otel-ctx-v1 branch from 1d13984 to cd16c6e Compare March 31, 2026 14:46

feat: add update_current_record for in-place TLS updates

dce34aa

scottgerring force-pushed the feat/otel-ctx-v1 branch from 1267ca1 to bffd3e3 Compare April 1, 2026 06:50

scottgerring added 3 commits April 1, 2026 09:04

docs: update README with OTEP references and remove stale content

d61a5db

remove js bindings

f45b9d0

update example

0e191ff

scottgerring force-pushed the feat/otel-ctx-v1 branch from bffd3e3 to 0e191ff Compare April 1, 2026 07:35

scottgerring commented Apr 1, 2026

View reviewed changes

Comment thread js/addon.cpp

@@ -1,281 +0,0 @@

// addon.c

Copy link
Copy Markdown

Owner Author

scottgerring Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed all the js stuff as its out of scope for us

scottgerring commented Apr 1, 2026

View reviewed changes

yannham reviewed Apr 1, 2026

View reviewed changes

scottgerring force-pushed the feat/otel-ctx-v1 branch from 8420631 to eacc787 Compare April 2, 2026 07:28

ivoanjo reviewed Apr 7, 2026

View reviewed changes

Address Yann review comments

04e1ba3

scottgerring force-pushed the feat/otel-ctx-v1 branch from eacc787 to ac68cd5 Compare April 7, 2026 14:27

Address Ivo review comments

2618d24

scottgerring force-pushed the feat/otel-ctx-v1 branch from ac68cd5 to 2618d24 Compare April 7, 2026 14:51


		## Acknowledgements

		* This is a fork of [polarsignals/custom-labels](https://github.com/polarsignals/custom-labels), extended to support the formalisation of the thread-local context sharing mechanism in [OTEP 4947](https://github.com/open-telemetry/opentelemetry-specification/pull/4947).

		/// Maximum varint value (14 bits)
		pub const UINT14_MAX: u16 = 16383;

Conversation

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

Uh oh!

Summary

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!

yannham left a comment

Choose a reason for hiding this comment

Uh oh!

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!

yannham Apr 8, 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivoanjo left a comment

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!

scottgerring Apr 7, 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!

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!

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!

3 participants

scottgerring commented Mar 31, 2026 •

edited

Loading

yannham Apr 8, 2026 •

edited

Loading

scottgerring Apr 7, 2026 •

edited

Loading