Skip to content

Commit

Permalink
Span Status: only set description for ERROR status code.
Browse files Browse the repository at this point in the history
According to the spec:

> Description MUST only be used with the Error StatusCode value.
> An empty Description is equivalent with a not present one.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status

This commit updates the `Status` implementation to comply with the spec.
  • Loading branch information
owais committed Mar 5, 2021
1 parent 1af9f87 commit 9c03467
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v0.18b0...HEAD)
- Status now only sets `description` when `status_code` is set to `StatusCode.ERROR`
([#1673])(https://github.com/open-telemetry/opentelemetry-python/pull/1673)

### Added
- Add `max_attr_value_length` support to Jaeger exporter
Expand Down
10 changes: 8 additions & 2 deletions opentelemetry-api/src/opentelemetry/trace/status.py
Expand Up @@ -48,10 +48,16 @@ def __init__(
):
self._status_code = status_code
self._description = None

if description is not None and not isinstance(description, str):
logger.warning("Invalid status description type, expected str")
else:
self._description = description
return

if status_code != StatusCode.ERROR:
logger.warning("description should only be set when status_code is set to StatusCode.ERROR")
return

self._description = description

@property
def status_code(self) -> StatusCode:
Expand Down
20 changes: 19 additions & 1 deletion opentelemetry-api/tests/trace/test_status.py
Expand Up @@ -29,7 +29,25 @@ def test_constructor(self):
self.assertEqual(status.description, "unavailable")

def test_invalid_description(self):
with self.assertLogs(level=WARNING):
with self.assertLogs(level=WARNING) as warning:
status = Status(description={"test": "val"}) # type: ignore
self.assertIs(status.status_code, StatusCode.UNSET)
self.assertEqual(status.description, None)
self.assertIn("Invalid status description type, expected str", warning.output[0])

def test_description_and_non_error_status(self):
with self.assertLogs(level=WARNING) as warning:
status = Status(status_code=StatusCode.OK, description="status description")
self.assertIs(status.status_code, StatusCode.OK)
self.assertEqual(status.description, None)
self.assertIn("description should only be set when status_code is set to StatusCode.ERROR", warning.output[0])

with self.assertLogs(level=WARNING) as warning:
status = Status(status_code=StatusCode.UNSET, description="status description")
self.assertIs(status.status_code, StatusCode.UNSET)
self.assertEqual(status.description, None)
self.assertIn("description should only be set when status_code is set to StatusCode.ERROR", warning.output[0])

status = Status(status_code=StatusCode.ERROR, description="status description")
self.assertIs(status.status_code, StatusCode.ERROR)
self.assertEqual(status.description, "status description")

0 comments on commit 9c03467

Please sign in to comment.