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

This commit is contained in:
Leighton Chen
2020-10-28 17:28:58 -04:00
committed by GitHub
parent b630d65294
commit 032c1ff730
8 changed files with 22 additions and 48 deletions

View File

@ -5,7 +5,7 @@ import asyncpg
from opentelemetry.instrumentation.asyncpg import AsyncPGInstrumentor from opentelemetry.instrumentation.asyncpg import AsyncPGInstrumentor
from opentelemetry.test.test_base import TestBase from opentelemetry.test.test_base import TestBase
from opentelemetry.trace.status import StatusCanonicalCode from opentelemetry.trace.status import StatusCode
POSTGRES_HOST = os.getenv("POSTGRESQL_HOST ", "localhost") POSTGRES_HOST = os.getenv("POSTGRESQL_HOST ", "localhost")
POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT ", "5432")) POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT ", "5432"))
@ -45,9 +45,7 @@ class TestFunctionalAsyncPG(TestBase):
async_call(self._connection.execute("SELECT 42;")) async_call(self._connection.execute("SELECT 42;"))
spans = self.memory_exporter.get_finished_spans() spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1) self.assertEqual(len(spans), 1)
self.assertEqual( self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
StatusCanonicalCode.OK, spans[0].status.canonical_code
)
self.assertEqual( self.assertEqual(
spans[0].attributes, spans[0].attributes,
{ {
@ -90,9 +88,7 @@ class TestFunctionalAsyncPG(TestBase):
}, },
spans[0].attributes, spans[0].attributes,
) )
self.assertEqual( self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
StatusCanonicalCode.OK, spans[0].status.canonical_code
)
self.assertEqual( self.assertEqual(
{ {
"db.instance": POSTGRES_DB_NAME, "db.instance": POSTGRES_DB_NAME,
@ -102,9 +98,7 @@ class TestFunctionalAsyncPG(TestBase):
}, },
spans[1].attributes, spans[1].attributes,
) )
self.assertEqual( self.assertIs(StatusCode.UNSET, spans[1].status.status_code)
StatusCanonicalCode.OK, spans[1].status.canonical_code
)
self.assertEqual( self.assertEqual(
{ {
"db.instance": POSTGRES_DB_NAME, "db.instance": POSTGRES_DB_NAME,
@ -114,9 +108,7 @@ class TestFunctionalAsyncPG(TestBase):
}, },
spans[2].attributes, spans[2].attributes,
) )
self.assertEqual( self.assertIs(StatusCode.UNSET, spans[2].status.status_code)
StatusCanonicalCode.OK, spans[2].status.canonical_code
)
def test_instrumented_failed_transaction_method(self, *_, **__): def test_instrumented_failed_transaction_method(self, *_, **__):
async def _transaction_execute(): async def _transaction_execute():
@ -137,9 +129,7 @@ class TestFunctionalAsyncPG(TestBase):
}, },
spans[0].attributes, spans[0].attributes,
) )
self.assertEqual( self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
StatusCanonicalCode.OK, spans[0].status.canonical_code
)
self.assertEqual( self.assertEqual(
{ {
"db.instance": POSTGRES_DB_NAME, "db.instance": POSTGRES_DB_NAME,
@ -150,8 +140,7 @@ class TestFunctionalAsyncPG(TestBase):
spans[1].attributes, spans[1].attributes,
) )
self.assertEqual( self.assertEqual(
StatusCanonicalCode.INVALID_ARGUMENT, StatusCode.ERROR, spans[1].status.status_code,
spans[1].status.canonical_code,
) )
self.assertEqual( self.assertEqual(
{ {
@ -162,17 +151,13 @@ class TestFunctionalAsyncPG(TestBase):
}, },
spans[2].attributes, spans[2].attributes,
) )
self.assertEqual( self.assertIs(StatusCode.UNSET, spans[2].status.status_code)
StatusCanonicalCode.OK, spans[2].status.canonical_code
)
def test_instrumented_method_doesnt_capture_parameters(self, *_, **__): def test_instrumented_method_doesnt_capture_parameters(self, *_, **__):
async_call(self._connection.execute("SELECT $1;", "1")) async_call(self._connection.execute("SELECT $1;", "1"))
spans = self.memory_exporter.get_finished_spans() spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1) self.assertEqual(len(spans), 1)
self.assertEqual( self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
StatusCanonicalCode.OK, spans[0].status.canonical_code
)
self.assertEqual( self.assertEqual(
spans[0].attributes, spans[0].attributes,
{ {
@ -216,9 +201,7 @@ class TestFunctionalAsyncPG_CaptureParameters(TestBase):
async_call(self._connection.execute("SELECT $1;", "1")) async_call(self._connection.execute("SELECT $1;", "1"))
spans = self.memory_exporter.get_finished_spans() spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1) self.assertEqual(len(spans), 1)
self.assertEqual( self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
StatusCanonicalCode.OK, spans[0].status.canonical_code
)
self.assertEqual( self.assertEqual(
spans[0].attributes, spans[0].attributes,
{ {

View File

@ -22,7 +22,7 @@ from opentelemetry import trace as trace_api
from opentelemetry.instrumentation.celery import CeleryInstrumentor from opentelemetry.instrumentation.celery import CeleryInstrumentor
from opentelemetry.sdk import resources from opentelemetry.sdk import resources
from opentelemetry.sdk.trace import TracerProvider, export from opentelemetry.sdk.trace import TracerProvider, export
from opentelemetry.trace.status import StatusCanonicalCode from opentelemetry.trace.status import StatusCode
# set a high timeout for async executions due to issues in CI # set a high timeout for async executions due to issues in CI
ASYNC_GET_TIMEOUT = 120 ASYNC_GET_TIMEOUT = 120
@ -257,7 +257,7 @@ def test_fn_exception(celery_app, memory_exporter):
span.attributes.get("celery.task_name") span.attributes.get("celery.task_name")
== "test_celery_functional.fn_exception" == "test_celery_functional.fn_exception"
) )
assert span.status.canonical_code == StatusCanonicalCode.UNKNOWN assert span.status.status_code == StatusCode.ERROR
assert span.attributes.get("messaging.message_id") == result.task_id assert span.attributes.get("messaging.message_id") == result.task_id
assert "Task class is failing" in span.status.description assert "Task class is failing" in span.status.description
@ -278,7 +278,7 @@ def test_fn_exception_expected(celery_app, memory_exporter):
span = spans[0] span = spans[0]
assert span.status.is_ok is True assert span.status.is_ok is True
assert span.status.canonical_code == StatusCanonicalCode.OK assert span.status.status_code == StatusCode.UNSET
assert span.name == "run/test_celery_functional.fn_exception" assert span.name == "run/test_celery_functional.fn_exception"
assert span.attributes.get("celery.action") == "run" assert span.attributes.get("celery.action") == "run"
assert span.attributes.get("celery.state") == "FAILURE" assert span.attributes.get("celery.state") == "FAILURE"
@ -305,7 +305,7 @@ def test_fn_retry_exception(celery_app, memory_exporter):
span = spans[0] span = spans[0]
assert span.status.is_ok is True assert span.status.is_ok is True
assert span.status.canonical_code == StatusCanonicalCode.OK assert span.status.status_code == StatusCode.UNSET
assert span.name == "run/test_celery_functional.fn_exception" assert span.name == "run/test_celery_functional.fn_exception"
assert span.attributes.get("celery.action") == "run" assert span.attributes.get("celery.action") == "run"
assert span.attributes.get("celery.state") == "RETRY" assert span.attributes.get("celery.state") == "RETRY"
@ -377,7 +377,7 @@ def test_class_task_exception(celery_app, memory_exporter):
) )
assert span.attributes.get("celery.action") == "run" assert span.attributes.get("celery.action") == "run"
assert span.attributes.get("celery.state") == "FAILURE" assert span.attributes.get("celery.state") == "FAILURE"
assert span.status.canonical_code == StatusCanonicalCode.UNKNOWN assert span.status.status_code == StatusCode.ERROR
assert span.attributes.get("messaging.message_id") == result.task_id assert span.attributes.get("messaging.message_id") == result.task_id
assert "Task class is failing" in span.status.description assert "Task class is failing" in span.status.description
@ -406,7 +406,7 @@ def test_class_task_exception_excepted(celery_app, memory_exporter):
span = spans[0] span = spans[0]
assert span.status.is_ok is True assert span.status.is_ok is True
assert span.status.canonical_code == StatusCanonicalCode.OK assert span.status.status_code == StatusCode.UNSET
assert span.name == "run/test_celery_functional.BaseTask" assert span.name == "run/test_celery_functional.BaseTask"
assert span.attributes.get("celery.action") == "run" assert span.attributes.get("celery.action") == "run"
assert span.attributes.get("celery.state") == "FAILURE" assert span.attributes.get("celery.state") == "FAILURE"

View File

@ -36,9 +36,7 @@ class TestRedisInstrument(TestBase):
def _check_span(self, span): def _check_span(self, span):
self.assertEqual(span.attributes["service"], self.test_service) self.assertEqual(span.attributes["service"], self.test_service)
self.assertEqual(span.name, "redis.command") self.assertEqual(span.name, "redis.command")
self.assertIs( self.assertIs(span.status.status_code, trace.status.StatusCode.UNSET)
span.status.canonical_code, trace.status.StatusCanonicalCode.OK
)
self.assertEqual(span.attributes.get("db.instance"), 0) self.assertEqual(span.attributes.get("db.instance"), 0)
self.assertEqual( self.assertEqual(
span.attributes.get("db.url"), "redis://localhost:6379" span.attributes.get("db.url"), "redis://localhost:6379"

View File

@ -113,9 +113,7 @@ class SQLAlchemyTestMixin(TestBase):
self.assertEqual(span.name, "{}.query".format(self.VENDOR)) self.assertEqual(span.name, "{}.query".format(self.VENDOR))
self.assertEqual(span.attributes.get("service"), self.SERVICE) self.assertEqual(span.attributes.get("service"), self.SERVICE)
self.assertEqual(span.attributes.get(_DB), self.SQL_DB) self.assertEqual(span.attributes.get(_DB), self.SQL_DB)
self.assertIs( self.assertIs(span.status.status_code, trace.status.StatusCode.UNSET)
span.status.canonical_code, trace.status.StatusCanonicalCode.OK
)
self.assertGreater((span.end_time - span.start_time), 0) self.assertGreater((span.end_time - span.start_time), 0)
def test_orm_insert(self): def test_orm_insert(self):

View File

@ -66,7 +66,5 @@ class SQLAlchemyInstrumentTestCase(TestBase):
# check subset of span fields # check subset of span fields
self.assertEqual(span.name, "postgres.query") self.assertEqual(span.name, "postgres.query")
self.assertEqual(span.attributes.get("service"), "postgres") self.assertEqual(span.attributes.get("service"), "postgres")
self.assertIs( self.assertIs(span.status.status_code, trace.status.StatusCode.UNSET)
span.status.canonical_code, trace.status.StatusCanonicalCode.OK
)
self.assertGreater((span.end_time - span.start_time), 0) self.assertGreater((span.end_time - span.start_time), 0)

View File

@ -77,7 +77,6 @@ class MysqlConnectorTestCase(SQLAlchemyTestMixin):
self.assertTrue(span.end_time - span.start_time > 0) self.assertTrue(span.end_time - span.start_time > 0)
# check the error # check the error
self.assertIs( self.assertIs(
span.status.canonical_code, span.status.status_code, trace.status.StatusCode.ERROR,
trace.status.StatusCanonicalCode.UNKNOWN,
) )
self.assertIn("a_wrong_table", span.status.description) self.assertIn("a_wrong_table", span.status.description)

View File

@ -78,8 +78,7 @@ class PostgresTestCase(SQLAlchemyTestMixin):
self.assertTrue(span.end_time - span.start_time > 0) self.assertTrue(span.end_time - span.start_time > 0)
# check the error # check the error
self.assertIs( self.assertIs(
span.status.canonical_code, span.status.status_code, trace.status.StatusCode.ERROR,
trace.status.StatusCanonicalCode.UNKNOWN,
) )
self.assertIn("a_wrong_table", span.status.description) self.assertIn("a_wrong_table", span.status.description)

View File

@ -53,8 +53,7 @@ class SQLiteTestCase(SQLAlchemyTestMixin):
self.assertTrue((span.end_time - span.start_time) > 0) self.assertTrue((span.end_time - span.start_time) > 0)
# check the error # check the error
self.assertIs( self.assertIs(
span.status.canonical_code, span.status.status_code, trace.status.StatusCode.ERROR,
trace.status.StatusCanonicalCode.UNKNOWN,
) )
self.assertEqual( self.assertEqual(
span.status.description, "no such table: a_wrong_table" span.status.description, "no such table: a_wrong_table"