fix(ext/telemetry): add exception.* attributes to OTEL log records#32726
Conversation
When a single Error object is passed to console.log/error/warn/etc, the OTEL log record now includes exception.type, exception.message, and exception.stacktrace attributes per the OpenTelemetry semantic conventions for exception logs. Closes denoland#27915 Closes denoland#29104 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of adding a 3rd parameter to Console's printFunc calls (which broke existing consumers expecting only 2 args), wrap the otel console methods to capture Error objects in a module-level variable before the Console stringifies them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kajukitli
left a comment
There was a problem hiding this comment.
The PR introduces exception attribute recording for OpenTelemetry console logging, but has a significant race condition vulnerability due to the use of global mutable state (pendingException) to pass exception context between wrapper methods and the logging function. This pattern is unsafe in concurrent JavaScript execution contexts.
[HIGH] ext/telemetry/telemetry.ts:1189: Race condition due to global mutable state pendingException. When multiple console methods are called concurrently (from different async contexts, timers, or callbacks), the exception set by one call could be incorrectly associated with a different log message, or cleared before being processed.
Suggestion: Pass the exception directly through the console wrapping mechanism rather than using global state, or use a synchronous stack-based approach that ties the exception to the specific log call.
[MEDIUM] ext/telemetry/telemetry.ts:1199: Exception is consumed even when not used. pendingException is cleared at the start of otelLog regardless of whether the function completes successfully or returns early, causing exception information to be silently lost.
Suggestion: Only clear
pendingExceptionafter it has been successfully used to emit a log record.
[MEDIUM] ext/telemetry/telemetry.ts:1193: Exception capture is too restrictive. The wrapper only captures the exception when args.length === 1, so calls like console.error(err, 'additional context') won't record exception attributes even though an error is being logged.
Suggestion: Check if the first argument is a native error regardless of args.length to support logging errors with additional context.
kajukitli
left a comment
There was a problem hiding this comment.
Reviewed with full repo context. Initial findings didn't hold up on closer inspection. Looks good.
kajukitli
left a comment
There was a problem hiding this comment.
This PR adds OpenTelemetry exception attribute recording for console error/warn methods. The implementation has a race condition concern with the global pendingException state that could cause exceptions to be misattributed in concurrent scenarios, and the exception capture only works for single-argument Error calls which may be an intentional limitation but could surprise users.
[MEDIUM] ext/telemetry/telemetry.ts:1189: Race condition with global pendingException state. The variable is set in wrapped console methods and consumed in otelLog. In concurrent async scenarios, an exception from one call could be incorrectly associated with a different log call, or lost if another console call happens between setting and consuming.
Suggestion: Consider passing the exception through the Console callback mechanism or using a more context-aware approach instead of relying on module-level global state.
[MEDIUM] ext/telemetry/telemetry.ts:1196: Exception is only captured for single-argument Error calls. console.error(new Error('test'), 'extra info') will NOT capture exception attributes. The test confirms this limitation, but it may surprise users who expect exception metadata when any Error is logged.
Suggestion: Consider capturing exception info when the first argument is a native Error regardless of args.length, if the intent is to capture exception metadata whenever an Error object is logged.
Previously exception attributes were only captured when console.error was called with a single Error argument. Now they are captured whenever the first argument is a native Error, regardless of additional args. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ream Consolidate op_otel_log_exception and op_otel_log_foreign_exception into op_otel_log and op_otel_log_foreign by adding optional exception string parameters. This fixes a bug where exception log records were missing the log.iostream attribute, and extracts a severity_from_level helper to eliminate the 4x duplicated severity match. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on test The expected output was missing log.iostream attributes (present on all OTEL log records) and exception attributes for the multi-arg error case (console.error(error, "extra") also sets pendingException). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…butes Avoids redundant .to_owned() allocations by consuming the String directly via .into(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
console.log/console.error/etc., the OTEL log record now includesexception.type,exception.message, andexception.stacktraceattributes per the OpenTelemetry semantic conventions for exception logs[ErrorData]internal slot (checked viaisNativeError)Closes #27915
Closes #29104
Test plan
log_exceptionverifies:console.error(new Error(...))→ exception attributes presentconsole.log("plain")→ no exception attributesconsole.error("a", "b")→ no exception attributes (multiple args)otel_basictests pass🤖 Generated with Claude Code