-
Notifications
You must be signed in to change notification settings - Fork 3k
[Core] Add method for setting span error status #40703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a6f0e1a
to
e63f6f6
Compare
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a utility method (set_span_status) to the OpenTelemetryTracer class so that users can set span statuses without having to import opentelemetry types.
- Introduces a static method set_span_status in opentelemetry.py that maps string status values ("OK", "ERROR", "UNSET") to corresponding Opentelemetry StatusCode enums.
- Adds tests in test_tracer_otel.py to verify correct behavior for valid statuses and proper error handling for invalid statuses.
- Updates the CHANGELOG to document the new feature.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
sdk/core/azure-core/tests/test_tracer_otel.py | Added unit tests covering valid, case-insensitive, and invalid span status cases. |
sdk/core/azure-core/azure/core/tracing/opentelemetry.py | Defined the new set_span_status method to simplify span status setting. |
sdk/core/azure-core/CHANGELOG.md | Updated changelog to include the set_span_status feature. |
Comments suppressed due to low confidence (1)
sdk/core/azure-core/tests/test_tracer_otel.py:378
- [nitpick] Consider asserting the exception message to confirm that the raised ValueError contains the expected error message. This can help ensure that the error is being raised for the correct reason.
with pytest.raises(ValueError):
API change check APIView has identified API level changes in this PR and created following API reviews. |
Spans started by `OpenTelemetryTracer` in core return an OpenTelemetry span. The set_status on this method requires that a user imports `opentelemetry` types for `StatusCode` enums. We want to avoid SDK developers needing to import `opentelemetry` in order to set a span status, however it all cases so far where a user is setting a status, it's to set it to ERROR. Here, we are adding a utility method on the `OpenTelemetryTracer` class to enable setting an error status on a provided span. Signed-off-by: Paul Van Eck <[email protected]>
e5bb939
to
6b81910
Compare
will there be other methods that do need an import however, even if right now its largely the error scenario? |
/check-enforcer override |
Spans started by `OpenTelemetryTracer` in core return an OpenTelemetry span. The set_status on this method requires that a user imports `opentelemetry` types for `StatusCode` enums. We want to avoid SDK developers needing to import `opentelemetry` in order to set a span status, however in all cases so far where a user is setting a status, it's to set it to ERROR. Here, we are adding a utility method on the `OpenTelemetryTracer` class to enable setting an error status on a provided span. Signed-off-by: Paul Van Eck <[email protected]>
Spans started by
OpenTelemetryTracer
in core return an OpenTelemetry span. Theset_status
on this method requires that a user importsopentelemetry
types forStatusCode
enums.We want to avoid SDK developers needing to import
opentelemetry
in order to set a span status. However, in all cases so far, where a status on a span is being set, it's to set it toERROR
. Here, we are adding a utility method on theOpenTelemetryTracer
class to enable setting an error status on a provided span.