Skip to content

Conversation

xavierroma
Copy link
Contributor

What was changed

Enhanced error span recording in the OpenTelemetry instrumentation. The wrapWithSpan function now sets message alongside status and capturing the exception..

Why?

This improvement brings better observability by providing a clear error message when a span ends in error status. This helps observability tools display a concise error message without having to look at the span's exception events.

Checklist

  1. Closes [Feature Request] Add error message to OpenTelemetry span setStatus #1631

  2. How was this tested:
    Added a new test "instrumentation: Error status includes message and records exception" in test-otel.ts that verifies:

  • Error messages are properly included in span status
  • Exceptions are properly recorded as span events
  • The span has the correct ERROR status code
  1. Any docs updates needed?
    None required as this is an internal enhancement.

@xavierroma xavierroma requested a review from a team as a code owner February 24, 2025 23:47
@CLAassistant
Copy link

CLAassistant commented Feb 24, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mjameswh
Copy link
Contributor

Thanks, @xavierroma, that certainly seems like a pertinent addition.

@xavierroma xavierroma force-pushed the add-error-message-to-span-status branch from 678618f to c27f73f Compare February 25, 2025 17:49
@xavierroma
Copy link
Contributor Author

Fixed failed lint CI step.
I forgot to add @opentelemetry/sdk-trace-base as a test dependency and verified lintting works locally

@xavierroma
Copy link
Contributor Author

Hi @mjameswh, do you mind running the ci jobs. Should there be anything else to be done before merging, please let me know.

@xavierroma
Copy link
Contributor Author

@mjameswh good to go?

@mjameswh mjameswh merged commit b0316da into temporalio:main Feb 27, 2025
22 checks passed
@mjameswh
Copy link
Contributor

@mjameswh good to go?

Yes, that's merged. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Add error message to OpenTelemetry span setStatus
3 participants