Skip to content

Commit f123d84

Browse files
bartlomiejuclaude
andauthored
fix(ext/otel): remove panicking unwraps in telemetry code (#32557)
## Summary - Replace `.unwrap()` calls on `OTEL_GLOBALS.get()` and isolate slot accesses with graceful error handling throughout `ext/telemetry/lib.rs` - GC prologue/epilogue callbacks now return early if the isolate slot is missing (can happen during isolate teardown in watch mode restarts) - `op_otel_enable_isolate_metrics` / `op_otel_collect_isolate_metrics` return early if globals or slots are not available - `OtelTracer::builtin()`, `OtelTracer::start_span()`, `OtelTracer::start_span_foreign()`, and `OtelMeter::new()` return JS errors instead of panicking ## Context These unwraps could panic in watch mode when: 1. V8 GC callbacks fire during isolate teardown after slots have been cleaned up 2. OTEL globals are accessed before initialization completes Refs: #29478 Refs: #30567 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0d1c07e commit f123d84

1 file changed

Lines changed: 28 additions & 12 deletions

File tree

ext/telemetry/lib.rs

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,12 +1433,14 @@ impl OtelTracer {
14331433

14341434
#[static_method]
14351435
#[cppgc]
1436-
fn builtin() -> OtelTracer {
1436+
fn builtin() -> Result<OtelTracer, JsErrorBox> {
14371437
let OtelGlobals {
14381438
builtin_instrumentation_scope,
14391439
..
1440-
} = OTEL_GLOBALS.get().unwrap();
1441-
OtelTracer(builtin_instrumentation_scope.clone())
1440+
} = OTEL_GLOBALS
1441+
.get()
1442+
.ok_or_else(|| JsErrorBox::generic("otel not initialized"))?;
1443+
Ok(OtelTracer(builtin_instrumentation_scope.clone()))
14421444
}
14431445

14441446
#[cppgc]
@@ -1451,7 +1453,9 @@ impl OtelTracer {
14511453
start_time: Option<f64>,
14521454
#[smi] attribute_count: usize,
14531455
) -> Result<OtelSpan, JsErrorBox> {
1454-
let OtelGlobals { id_generator, .. } = OTEL_GLOBALS.get().unwrap();
1456+
let OtelGlobals { id_generator, .. } = OTEL_GLOBALS
1457+
.get()
1458+
.ok_or_else(|| JsErrorBox::generic("otel not initialized"))?;
14551459
let span_context;
14561460
let parent_span_id;
14571461
match parent {
@@ -1540,7 +1544,9 @@ impl OtelTracer {
15401544
if parent_span_id == SpanId::INVALID {
15411545
return Err(JsErrorBox::generic("invalid span id"));
15421546
};
1543-
let OtelGlobals { id_generator, .. } = OTEL_GLOBALS.get().unwrap();
1547+
let OtelGlobals { id_generator, .. } = OTEL_GLOBALS
1548+
.get()
1549+
.ok_or_else(|| JsErrorBox::generic("otel not initialized"))?;
15441550
let span_context = SpanContext::new(
15451551
parent_trace_id,
15461552
id_generator.new_span_id(),
@@ -1915,7 +1921,7 @@ impl OtelMeter {
19151921
#[string] name: String,
19161922
#[string] version: Option<String>,
19171923
#[string] schema_url: Option<String>,
1918-
) -> OtelMeter {
1924+
) -> Result<OtelMeter, JsErrorBox> {
19191925
let mut builder = opentelemetry::InstrumentationScope::builder(name);
19201926
if let Some(version) = version {
19211927
builder = builder.with_version(version);
@@ -1926,10 +1932,10 @@ impl OtelMeter {
19261932
let scope = builder.build();
19271933
let meter = OTEL_GLOBALS
19281934
.get()
1929-
.unwrap()
1935+
.ok_or_else(|| JsErrorBox::generic("otel not initialized"))?
19301936
.meter_provider
19311937
.meter_with_scope(scope);
1932-
OtelMeter(meter)
1938+
Ok(OtelMeter(meter))
19331939
}
19341940

19351941
#[cppgc]
@@ -2556,7 +2562,9 @@ impl GcMetricData {
25562562
// SAFETY: Isolate is valid during callback
25572563
let isolate =
25582564
unsafe { v8::Isolate::from_raw_isolate_ptr_unchecked(isolate) };
2559-
let this = isolate.get_slot::<Self>().unwrap();
2565+
let Some(this) = isolate.get_slot::<Self>() else {
2566+
return;
2567+
};
25602568
this.0.borrow_mut().start = Instant::now();
25612569
}
25622570

@@ -2569,7 +2577,9 @@ impl GcMetricData {
25692577
// SAFETY: Isolate is valid during callback
25702578
let isolate =
25712579
unsafe { v8::Isolate::from_raw_isolate_ptr_unchecked(isolate) };
2572-
let this = isolate.get_slot::<Self>().unwrap();
2580+
let Some(this) = isolate.get_slot::<Self>() else {
2581+
return;
2582+
};
25732583
let this = this.0.borrow_mut();
25742584

25752585
let elapsed = this.start.elapsed();
@@ -2605,7 +2615,10 @@ fn op_otel_enable_isolate_metrics(scope: &mut v8::PinScope<'_, '_>) {
26052615
return;
26062616
}
26072617

2608-
let meter = OTEL_GLOBALS.get().unwrap().meter_provider.meter("v8js");
2618+
let Some(globals) = OTEL_GLOBALS.get() else {
2619+
return;
2620+
};
2621+
let meter = globals.meter_provider.meter("v8js");
26092622

26102623
// https://opentelemetry.io/docs/specs/semconv/runtime/v8js-metrics/#metric-v8jsgcduration
26112624
let duration = meter
@@ -2663,7 +2676,10 @@ fn op_otel_enable_isolate_metrics(scope: &mut v8::PinScope<'_, '_>) {
26632676

26642677
#[op2(fast)]
26652678
fn op_otel_collect_isolate_metrics(scope: &mut v8::PinScope<'_, '_>) {
2666-
let data = scope.get_slot::<HeapMetricData>().unwrap().clone();
2679+
let Some(data) = scope.get_slot::<HeapMetricData>() else {
2680+
return;
2681+
};
2682+
let data = data.clone();
26672683
for i in 0..scope.get_number_of_data_slots() {
26682684
let Some(space) = scope.get_heap_space_statistics(i as _) else {
26692685
continue;

0 commit comments

Comments
 (0)