Skip to content

Commit c2bc9c6

Browse files
committed
Revert "chore: relax str to [u8] for thread ctx attrs"
This reverts commit 7dd922e. After discussion, the spec actually requires UTF8 bytes, so let's enforce that at the type system level.
1 parent 7dd922e commit c2bc9c6

1 file changed

Lines changed: 21 additions & 20 deletions

File tree

libdd-profiling/src/otel_thread_ctx.rs

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
//!
4141
//! let trace_id = [0u8; 16];
4242
//! let span_id = [1u8; 8];
43-
//! let attrs: &[(u8, &[u8])] = &[(0, "GET"), (1, "/api/v1")];
43+
//! let attrs: &[(u8, &str)] = &[(0, "GET"), (1, "/api/v1")];
4444
//!
4545
//! // Publish a new context and save the previously attached one (if any).
4646
//! let ctx = ThreadContext::new(trace_id, span_id, attrs);
@@ -176,7 +176,7 @@ pub mod linux {
176176
trace_id: [u8; 16],
177177
span_id: [u8; 8],
178178
local_root_span_id: [u8; 8],
179-
attrs: &[(u8, &[u8])],
179+
attrs: &[(u8, &str)],
180180
) -> Self {
181181
const { assert!(size_of::<ThreadContextRecord>() == 640) }
182182

@@ -210,15 +210,16 @@ pub mod linux {
210210
/// recovery would require us to be able to rollback to the previous attributes which would
211211
/// hurt the happy path, or leave the record in a inconsistent state. Another possibility
212212
/// would be to error out and reset the record in that situation.
213-
fn set_attrs(&mut self, local_root_span_id: [u8; 8], attributes: &[(u8, &[u8])]) -> bool {
213+
fn set_attrs(&mut self, local_root_span_id: [u8; 8], attributes: &[(u8, &str)]) -> bool {
214214
let mut fully_encoded = true;
215215

216216
self.attrs_data[0] = ROOT_SPAN_KEY_INDEX;
217217
self.attrs_data[1] = 8;
218218
self.attrs_data[2..10].copy_from_slice(local_root_span_id.as_slice());
219219
let mut offset = 10;
220220

221-
for &(key_index, val_bytes) in attributes {
221+
for &(key_index, val) in attributes {
222+
let val_bytes = val.as_bytes();
222223
let val_len = val_bytes.len();
223224
let val_len = if val_len > 255 {
224225
fully_encoded = false;
@@ -234,7 +235,7 @@ pub mod linux {
234235
}
235236

236237
self.attrs_data[offset] = key_index;
237-
// `val_len <= 255` from the check above
238+
// `val_len <= 255` thanks to the `min()`
238239
self.attrs_data[offset + 1] = val_len as u8;
239240
self.attrs_data[offset + 2..offset + 2 + val_len]
240241
.copy_from_slice(&val_bytes[..val_len]);
@@ -279,7 +280,7 @@ pub mod linux {
279280
trace_id: [u8; 16],
280281
span_id: [u8; 8],
281282
local_root_span_id: [u8; 8],
282-
attrs: &[(u8, &[u8])],
283+
attrs: &[(u8, &str)],
283284
) -> Self {
284285
Self::from(ThreadContextRecord::new(
285286
trace_id,
@@ -363,7 +364,7 @@ pub mod linux {
363364
trace_id: [u8; 16],
364365
span_id: [u8; 8],
365366
local_root_span_id: [u8; 8],
366-
attrs: &[(u8, &[u8])],
367+
attrs: &[(u8, &str)],
367368
) {
368369
let slot = get_tls_slot();
369370

@@ -481,7 +482,7 @@ pub mod linux {
481482
#[cfg_attr(miri, ignore)]
482483
fn attribute_encoding_basic() {
483484
let root_span_id = [0u8; 8];
484-
let attrs: &[(u8, &[u8])] = &[(1, b"GET"), (2, b"/api/v1")];
485+
let attrs: &[(u8, &str)] = &[(1, "GET"), (2, "/api/v1")];
485486
ThreadContext::new([0u8; 16], [0u8; 8], root_span_id, attrs).attach();
486487

487488
let ptr = read_tls_context_ptr();
@@ -512,14 +513,14 @@ pub mod linux {
512513
// Two such entries: 514 bytes, plus root_span_id: 524.
513514
// A third entry of 100 chars would need 102 bytes, bringing the total to 626 > 612, so
514515
// the third entry must be dropped.
515-
let val_a = b"a".repeat(255); // 257 bytes encoded
516-
let val_b = b"b".repeat(255); // 257 bytes encoded → 514 total
517-
let val_c = b"c".repeat(100); // 102 bytes encoded → 626 total: must be dropped
518-
519-
let attrs: &[(u8, &[u8])] = &[
520-
(1, val_a.as_slice()),
521-
(2, val_b.as_slice()),
522-
(3, val_c.as_slice()),
516+
let val_a = "a".repeat(255); // 257 bytes encoded
517+
let val_b = "b".repeat(255); // 257 bytes encoded → 514 total
518+
let val_c = "c".repeat(100); // 102 bytes encoded → 626 total: must be dropped
519+
520+
let attrs: &[(u8, &str)] = &[
521+
(1, val_a.as_str()),
522+
(2, val_b.as_str()),
523+
(3, val_c.as_str()),
523524
];
524525

525526
ThreadContext::new([0u8; 16], [0u8; 8], [0u8; 8], attrs).attach();
@@ -548,7 +549,7 @@ pub mod linux {
548549
let root_span_id2 = [0x79u8; 8];
549550

550551
// Updating before any context is attached should be equivalent to `attach()`
551-
ThreadContext::update(trace_id1, span_id1, root_span_id1, &[(0, b"v1")]);
552+
ThreadContext::update(trace_id1, span_id1, root_span_id1, &[(0, "v1")]);
552553

553554
let ptr_before = read_tls_context_ptr();
554555
assert!(!ptr_before.is_null());
@@ -563,7 +564,7 @@ pub mod linux {
563564
assert_eq!(record.attrs_data[11], 2);
564565
assert_eq!(&record.attrs_data[12..14], b"v1");
565566

566-
ThreadContext::update(trace_id2, span_id2, root_span_id2, &[(0, b"v2")]);
567+
ThreadContext::update(trace_id2, span_id2, root_span_id2, &[(0, "v2")]);
567568

568569
let ptr_after = read_tls_context_ptr();
569570
assert_eq!(
@@ -603,8 +604,8 @@ pub mod linux {
603604
#[test]
604605
#[cfg_attr(miri, ignore)]
605606
fn long_value_capped_at_255_bytes() {
606-
let long_val = b"a".repeat(300);
607-
ThreadContext::new([0u8; 16], [0u8; 8], [0u8; 8], &[(0, long_val.as_slice())]).attach();
607+
let long_val = "a".repeat(300);
608+
ThreadContext::new([0u8; 16], [0u8; 8], [0u8; 8], &[(0, long_val.as_str())]).attach();
608609

609610
let ptr = read_tls_context_ptr();
610611
assert!(!ptr.is_null());

0 commit comments

Comments
 (0)