Skip to content
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

Change status codes from grpc status codes, remove setting status in instrumentations except on ERROR #1282

Merged
merged 25 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/examples/auto-instrumentation/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ similar to:
"start_time": "2020-04-30T17:28:57.886397Z",
"end_time": "2020-04-30T17:28:57.886490Z",
"status": {
"canonical_code": "OK"
"status_code": "OK"
},
"attributes": {
"component": "http",
Expand Down Expand Up @@ -164,7 +164,7 @@ similar to:
"start_time": "2020-04-30T17:10:02.400604Z",
"end_time": "2020-04-30T17:10:02.401858Z",
"status": {
"canonical_code": "OK"
"status_code": "OK"
},
"attributes": {
"component": "http",
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/django/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ output similar to this one:
"start_time": "2020-04-26T01:49:57.205833Z",
"end_time": "2020-04-26T01:49:57.206214Z",
"status": {
"canonical_code": "OK"
"status_code": "OK"
},
"attributes": {
"component": "http",
Expand Down
6 changes: 3 additions & 3 deletions docs/getting-started.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ We can run it, and see the traces print to your console:
"start_time": "2020-05-07T14:39:52.906272Z",
"end_time": "2020-05-07T14:39:52.906343Z",
"status": {
"canonical_code": "OK"
"status_code": "OK"
},
"attributes": {},
"events": [],
Expand All @@ -67,7 +67,7 @@ We can run it, and see the traces print to your console:
"start_time": "2020-05-07T14:39:52.906230Z",
"end_time": "2020-05-07T14:39:52.906601Z",
"status": {
"canonical_code": "OK"
"status_code": "OK"
},
"attributes": {},
"events": [],
Expand All @@ -85,7 +85,7 @@ We can run it, and see the traces print to your console:
"start_time": "2020-05-07T14:39:52.906157Z",
"end_time": "2020-05-07T14:39:52.906743Z",
"status": {
"canonical_code": "OK"
"status_code": "OK"
},
"attributes": {},
"events": [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import opentelemetry.trace as trace_api
from opentelemetry.sdk.trace import sampling
from opentelemetry.sdk.trace.export import SpanExporter, SpanExportResult
from opentelemetry.trace.status import StatusCanonicalCode

# pylint:disable=relative-beyond-top-level
from .constants import (
Expand Down Expand Up @@ -145,7 +144,7 @@ def _translate_to_datadog(self, spans):
datadog_span.start_ns = span.start_time
datadog_span.duration_ns = span.end_time - span.start_time

if span.status.canonical_code is not StatusCanonicalCode.OK:
if not span.status.is_ok:
datadog_span.error = 1
if span.status.description:
exc_type, exc_val = _get_exc_info(span)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
from opentelemetry.exporter.jaeger.gen.agent import Agent as agent
from opentelemetry.exporter.jaeger.gen.jaeger import Collector as jaeger
from opentelemetry.sdk.trace.export import Span, SpanExporter, SpanExportResult
from opentelemetry.trace.status import StatusCanonicalCode
from opentelemetry.trace.status import StatusCode

DEFAULT_AGENT_HOST_NAME = "localhost"
DEFAULT_AGENT_PORT = 6831
Expand Down Expand Up @@ -224,7 +224,7 @@ def _translate_to_jaeger(spans: Span):

tags.extend(
[
_get_long_tag("status.code", status.canonical_code.value),
_get_long_tag("status.code", status.status_code.value),
_get_string_tag("status.message", status.description),
_get_string_tag("span.kind", span.kind.name),
]
Expand All @@ -245,7 +245,7 @@ def _translate_to_jaeger(spans: Span):
)

# Ensure that if Status.Code is not OK, that we set the "error" tag on the Jaeger span.
if status.canonical_code is not StatusCanonicalCode.OK:
if not status.is_ok:
tags.append(_get_bool_tag("error", True))

refs = _extract_refs_from_span(span)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from opentelemetry.sdk import trace
from opentelemetry.sdk.trace import Resource
from opentelemetry.sdk.util.instrumentation import InstrumentationInfo
from opentelemetry.trace.status import Status, StatusCanonicalCode
from opentelemetry.trace.status import Status, StatusCode


class TestJaegerSpanExporter(unittest.TestCase):
Expand Down Expand Up @@ -210,7 +210,7 @@ def test_translate_to_jaeger(self):
jaeger.Tag(
key="status.code",
vType=jaeger.TagType.LONG,
vLong=StatusCanonicalCode.OK.value,
vLong=StatusCode.UNSET.value,
),
jaeger.Tag(
key="status.message", vType=jaeger.TagType.STRING, vStr=None
Expand Down Expand Up @@ -249,7 +249,7 @@ def test_translate_to_jaeger(self):
attributes={"key_resource": "some_resource"}
)
otel_spans[0].set_status(
Status(StatusCanonicalCode.UNKNOWN, "Example description")
Status(StatusCode.ERROR, "Example description")
)
otel_spans[0].end(end_time=end_times[0])

Expand All @@ -259,6 +259,7 @@ def test_translate_to_jaeger(self):

otel_spans[2].start(start_time=start_times[2])
otel_spans[2].resource = Resource({})
otel_spans[2].set_status(Status(StatusCode.OK, "Example description"))
otel_spans[2].end(end_time=end_times[2])
otel_spans[2].instrumentation_info = InstrumentationInfo(
name="name", version="version"
Expand Down Expand Up @@ -304,7 +305,7 @@ def test_translate_to_jaeger(self):
jaeger.Tag(
key="status.code",
vType=jaeger.TagType.LONG,
vLong=StatusCanonicalCode.UNKNOWN.value,
vLong=StatusCode.ERROR.value,
),
jaeger.Tag(
key="status.message",
Expand Down Expand Up @@ -380,12 +381,12 @@ def test_translate_to_jaeger(self):
jaeger.Tag(
key="status.code",
vType=jaeger.TagType.LONG,
vLong=StatusCanonicalCode.OK.value,
vLong=StatusCode.OK.value,
),
jaeger.Tag(
key="status.message",
vType=jaeger.TagType.STRING,
vStr=None,
vStr="Example description",
),
jaeger.Tag(
key="span.kind",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def translate_to_collector(spans: Sequence[Span]):
status = None
if span.status is not None:
status = trace_pb2.Status(
code=span.status.canonical_code.value,
code=span.status.status_code.value,
message=span.status.description,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,14 @@ def test_translate_to_collector(self):
otel_spans[0].set_attribute("key_int", 333)
otel_spans[0].set_status(
trace_api.Status(
trace_api.status.StatusCanonicalCode.INTERNAL,
"test description",
trace_api.status.StatusCode.OK, "test description",
)
)
otel_spans[0].end(end_time=end_times[0])
otel_spans[1].start(start_time=start_times[1])
otel_spans[1].set_status(
trace_api.Status(
trace_api.status.StatusCanonicalCode.INTERNAL, {"test", "val"},
trace_api.status.StatusCode.ERROR, {"test", "val"},
)
)
otel_spans[1].end(end_time=end_times[1])
Expand Down Expand Up @@ -197,8 +196,7 @@ def test_translate_to_collector(self):
output_spans[2].parent_span_id, b"\x11\x11\x11\x11\x11\x11\x11\x11"
)
self.assertEqual(
output_spans[0].status.code,
trace_api.status.StatusCanonicalCode.INTERNAL.value,
output_spans[0].status.code, trace_api.status.StatusCode.OK.value,
)
self.assertEqual(output_spans[0].status.message, "test description")
self.assertEqual(len(output_spans[0].tracestate.entries), 1)
Expand Down Expand Up @@ -270,7 +268,7 @@ def test_translate_to_collector(self):
)
self.assertEqual(
output_spans[1].status.code,
trace_api.status.StatusCanonicalCode.INTERNAL.value,
trace_api.status.StatusCode.ERROR.value,
)
self.assertEqual(
output_spans[2].links.link[0].type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from opentelemetry.proto.trace.v1.trace_pb2 import Status
from opentelemetry.sdk.trace import Span as SDKSpan
from opentelemetry.sdk.trace.export import SpanExporter, SpanExportResult
from opentelemetry.trace.status import StatusCode

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -198,9 +199,12 @@ def _translate_links(self, sdk_span: SDKSpan) -> None:

def _translate_status(self, sdk_span: SDKSpan) -> None:
if sdk_span.status is not None:
# TODO: Update this when the proto definitions are updated to include UNSET and ERROR
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this TODO required for GA? If so, may be create an issue and link it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proto_status_code = Status.STATUS_CODE_OK
if sdk_span.status.status_code is StatusCode.ERROR:
proto_status_code = Status.STATUS_CODE_UNKNOWN_ERROR
self._collector_span_kwargs["status"] = Status(
code=sdk_span.status.canonical_code.value,
message=sdk_span.status.description,
code=proto_status_code, message=sdk_span.status.description,
)

def _translate_data(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def _translate_to_zipkin(self, spans: Sequence[Span]):

if span.status is not None:
zipkin_span["tags"]["otel.status_code"] = str(
span.status.canonical_code.value
span.status.status_code.value
)
if span.status.description is not None:
zipkin_span["tags"][
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from opentelemetry.sdk.trace.export import SpanExportResult
from opentelemetry.sdk.util.instrumentation import InstrumentationInfo
from opentelemetry.trace import TraceFlags
from opentelemetry.trace.status import Status, StatusCanonicalCode
from opentelemetry.trace.status import Status, StatusCode


class MockResponse:
Expand Down Expand Up @@ -179,7 +179,7 @@ def test_export(self):
otel_spans[0].set_attribute("key_string", "hello_world")
otel_spans[0].set_attribute("key_float", 111.22)
otel_spans[0].set_status(
Status(StatusCanonicalCode.UNKNOWN, "Example description")
Status(StatusCode.ERROR, "Example description")
)
otel_spans[0].end(end_time=end_times[0])

Expand Down Expand Up @@ -248,7 +248,7 @@ def test_export(self):
"kind": None,
"tags": {
"key_resource": "some_resource",
"otel.status_code": "0",
"otel.status_code": "1",
},
"annotations": None,
},
Expand All @@ -263,7 +263,7 @@ def test_export(self):
"tags": {
"key_string": "hello_world",
"key_resource": "some_resource",
"otel.status_code": "0",
"otel.status_code": "1",
},
"annotations": None,
},
Expand All @@ -278,7 +278,7 @@ def test_export(self):
"tags": {
"otel.instrumentation_library.name": "name",
"otel.instrumentation_library.version": "version",
"otel.status_code": "0",
"otel.status_code": "1",
},
"annotations": None,
},
Expand Down Expand Up @@ -356,7 +356,7 @@ def test_zero_padding(self):
"duration": duration // 10 ** 3,
"localEndpoint": local_endpoint,
"kind": None,
"tags": {"otel.status_code": "0"},
"tags": {"otel.status_code": "1"},
"annotations": None,
"debug": True,
"parentId": "0aaaaaaaaaaaaaaa",
Expand Down Expand Up @@ -400,9 +400,7 @@ def test_max_tag_length(self):
# added here to preserve order
span.set_attribute("k1", "v" * 500)
span.set_attribute("k2", "v" * 50)
span.set_status(
Status(StatusCanonicalCode.UNKNOWN, "Example description")
)
span.set_status(Status(StatusCode.ERROR, "Example description"))
span.end()

exporter = ZipkinSpanExporter(service_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ def strip_query_params(url: yarl.URL) -> str:
from opentelemetry.instrumentation.aiohttp_client.version import __version__
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.instrumentation.utils import (
http_status_to_canonical_code,
http_status_to_status_code,
unwrap,
)
from opentelemetry.trace import SpanKind, TracerProvider, get_tracer
from opentelemetry.trace.status import Status, StatusCanonicalCode
from opentelemetry.trace.status import Status, StatusCode

_UrlFilterT = typing.Optional[typing.Callable[[str], str]]
_SpanNameT = typing.Optional[
Expand Down Expand Up @@ -194,9 +194,7 @@ async def on_request_end(

if trace_config_ctx.span.is_recording():
trace_config_ctx.span.set_status(
Status(
http_status_to_canonical_code(int(params.response.status))
)
Status(http_status_to_status_code(int(params.response.status)))
)
trace_config_ctx.span.set_attribute(
"http.status_code", params.response.status
Expand All @@ -214,22 +212,8 @@ async def on_request_exception(
if trace_config_ctx.span is None:
return

if trace_config_ctx.span.is_recording():
if isinstance(
params.exception,
(aiohttp.ServerTimeoutError, aiohttp.TooManyRedirects),
):
status = StatusCanonicalCode.DEADLINE_EXCEEDED
# Assume any getaddrinfo error is a DNS failure.
elif isinstance(
params.exception, aiohttp.ClientConnectorError
) and isinstance(params.exception.os_error, socket.gaierror):
# DNS resolution failed
status = StatusCanonicalCode.UNKNOWN
else:
status = StatusCanonicalCode.UNAVAILABLE

trace_config_ctx.span.set_status(Status(status))
if trace_config_ctx.span.is_recording() and params.exception:
trace_config_ctx.span.set_status(Status(StatusCode.ERROR))
trace_config_ctx.span.record_exception(params.exception)
_end_trace(trace_config_ctx)

Expand Down
Loading