From d2ebdb8700b0d96c56c7c62dd62b83fb6066f2c3 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Wed, 2 Jun 2021 23:07:58 +0530 Subject: [PATCH 01/14] fix: for opentelemetry status code spec change --- google/cloud/spanner_v1/_opentelemetry_tracing.py | 12 +++--------- tests/unit/test__opentelemetry_tracing.py | 12 +++++------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/google/cloud/spanner_v1/_opentelemetry_tracing.py b/google/cloud/spanner_v1/_opentelemetry_tracing.py index 71ac518992..75add4f1dd 100644 --- a/google/cloud/spanner_v1/_opentelemetry_tracing.py +++ b/google/cloud/spanner_v1/_opentelemetry_tracing.py @@ -21,8 +21,7 @@ try: from opentelemetry import trace - from opentelemetry.trace.status import Status, StatusCanonicalCode - from opentelemetry.instrumentation.utils import http_status_to_canonical_code + from opentelemetry.trace.status import Status, StatusCode HAS_OPENTELEMETRY_INSTALLED = True except ImportError: @@ -55,11 +54,6 @@ def trace_call(name, session, extra_attributes=None): try: yield span except GoogleAPICallError as error: - if error.code is not None: - span.set_status(Status(http_status_to_canonical_code(error.code))) - elif error.grpc_status_code is not None: - span.set_status( - # OpenTelemetry's StatusCanonicalCode maps 1-1 with grpc status codes - Status(StatusCanonicalCode(error.grpc_status_code.value[0])) - ) + if error.code is not None or error.grpc_status_code is not None: + span.set_status(Status(StatusCode.ERROR)) raise diff --git a/tests/unit/test__opentelemetry_tracing.py b/tests/unit/test__opentelemetry_tracing.py index cfd3241718..24a6334b99 100644 --- a/tests/unit/test__opentelemetry_tracing.py +++ b/tests/unit/test__opentelemetry_tracing.py @@ -5,7 +5,7 @@ try: from opentelemetry import trace as trace_api - from opentelemetry.trace.status import StatusCanonicalCode + from opentelemetry.trace.status import StatusCode except ImportError: pass @@ -75,7 +75,7 @@ def test_trace_call(self): self.assertEqual(span.kind, trace_api.SpanKind.CLIENT) self.assertEqual(span.attributes, expected_attributes) self.assertEqual(span.name, "CloudSpanner.Test") - self.assertEqual(span.status.canonical_code, StatusCanonicalCode.OK) + self.assertEqual(span.status.status_code, StatusCode.OK) def test_trace_error(self): extra_attributes = {"db.instance": "database_name"} @@ -101,9 +101,7 @@ def test_trace_error(self): self.assertEqual(span.kind, trace_api.SpanKind.CLIENT) self.assertEqual(dict(span.attributes), expected_attributes) self.assertEqual(span.name, "CloudSpanner.Test") - self.assertEqual( - span.status.canonical_code, StatusCanonicalCode.INVALID_ARGUMENT - ) + self.assertEqual(span.status.status_code, StatusCode.ERROR) def test_trace_grpc_error(self): extra_attributes = {"db.instance": "database_name"} @@ -126,7 +124,7 @@ def test_trace_grpc_error(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) span = span_list[0] - self.assertEqual(span.status.canonical_code, StatusCanonicalCode.DATA_LOSS) + self.assertEqual(span.status.status_code, StatusCode.ERROR) def test_trace_codeless_error(self): extra_attributes = {"db.instance": "database_name"} @@ -147,4 +145,4 @@ def test_trace_codeless_error(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) span = span_list[0] - self.assertEqual(span.status.canonical_code, StatusCanonicalCode.UNKNOWN) + self.assertEqual(span.status.status_code, StatusCode.ERROR) From fef06d7fc816312cc9cb41b7f79d54d68f0adde2 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Thu, 3 Jun 2021 13:51:40 +0530 Subject: [PATCH 02/14] fix: corrected open telemetry tests to work with latest open telemetry specs --- .../spanner_v1/_opentelemetry_tracing.py | 5 +++-- tests/_helpers.py | 22 ++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/google/cloud/spanner_v1/_opentelemetry_tracing.py b/google/cloud/spanner_v1/_opentelemetry_tracing.py index 75add4f1dd..8f9f8559ef 100644 --- a/google/cloud/spanner_v1/_opentelemetry_tracing.py +++ b/google/cloud/spanner_v1/_opentelemetry_tracing.py @@ -52,8 +52,9 @@ def trace_call(name, session, extra_attributes=None): name, kind=trace.SpanKind.CLIENT, attributes=attributes ) as span: try: + span.set_status(Status(StatusCode.OK)) yield span except GoogleAPICallError as error: - if error.code is not None or error.grpc_status_code is not None: - span.set_status(Status(StatusCode.ERROR)) + span.set_status(Status(StatusCode.ERROR)) + span.record_exception(error) raise diff --git a/tests/_helpers.py b/tests/_helpers.py index 036c777845..10dbe0a3cf 100644 --- a/tests/_helpers.py +++ b/tests/_helpers.py @@ -3,7 +3,7 @@ try: from opentelemetry import trace as trace_api - from opentelemetry.trace.status import StatusCanonicalCode + from opentelemetry.trace.status import StatusCode from opentelemetry.sdk.trace import TracerProvider, export from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( @@ -14,21 +14,23 @@ except ImportError: HAS_OPENTELEMETRY_INSTALLED = False - StatusCanonicalCode = mock.Mock() + StatusCode = mock.Mock() class OpenTelemetryBase(unittest.TestCase): - def setUp(self): + @classmethod + def setUpClass(cls): if HAS_OPENTELEMETRY_INSTALLED: - self.original_tracer_provider = trace_api.get_tracer_provider() - self.tracer_provider = TracerProvider() - self.memory_exporter = InMemorySpanExporter() - span_processor = export.SimpleExportSpanProcessor(self.memory_exporter) - self.tracer_provider.add_span_processor(span_processor) - trace_api.set_tracer_provider(self.tracer_provider) + cls.original_tracer_provider = trace_api.get_tracer_provider() + cls.tracer_provider = TracerProvider() + cls.memory_exporter = InMemorySpanExporter() + cls.span_processor = export.SimpleSpanProcessor(cls.memory_exporter) + cls.tracer_provider.add_span_processor(cls.span_processor) + trace_api.set_tracer_provider(cls.tracer_provider) def tearDown(self): if HAS_OPENTELEMETRY_INSTALLED: + self.memory_exporter.clear() trace_api.set_tracer_provider(self.original_tracer_provider) def assertNoSpans(self): @@ -37,7 +39,7 @@ def assertNoSpans(self): self.assertEqual(len(span_list), 0) def assertSpanAttributes( - self, name, status=StatusCanonicalCode.OK, attributes=None, span=None + self, name, status=StatusCode.OK, attributes=None, span=None ): if HAS_OPENTELEMETRY_INSTALLED: if not span: From cf86d83cc332f85f37c250dea4d1f1eb8cf24ad0 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 00:58:46 +0530 Subject: [PATCH 03/14] fix: correct open telemetry tests status code --- tests/_helpers.py | 4 +--- tests/unit/test_batch.py | 4 ++-- tests/unit/test_session.py | 10 +++++----- tests/unit/test_snapshot.py | 22 +++++++++++----------- tests/unit/test_transaction.py | 8 ++++---- 5 files changed, 23 insertions(+), 25 deletions(-) diff --git a/tests/_helpers.py b/tests/_helpers.py index 10dbe0a3cf..577caccf87 100644 --- a/tests/_helpers.py +++ b/tests/_helpers.py @@ -21,7 +21,6 @@ class OpenTelemetryBase(unittest.TestCase): @classmethod def setUpClass(cls): if HAS_OPENTELEMETRY_INSTALLED: - cls.original_tracer_provider = trace_api.get_tracer_provider() cls.tracer_provider = TracerProvider() cls.memory_exporter = InMemorySpanExporter() cls.span_processor = export.SimpleSpanProcessor(cls.memory_exporter) @@ -31,7 +30,6 @@ def setUpClass(cls): def tearDown(self): if HAS_OPENTELEMETRY_INSTALLED: self.memory_exporter.clear() - trace_api.set_tracer_provider(self.original_tracer_provider) def assertNoSpans(self): if HAS_OPENTELEMETRY_INSTALLED: @@ -48,5 +46,5 @@ def assertSpanAttributes( span = span_list[0] self.assertEqual(span.name, name) - self.assertEqual(span.status.canonical_code, status) + self.assertEqual(span.status.status_code, status) self.assertEqual(dict(span.attributes), attributes) diff --git a/tests/unit/test_batch.py b/tests/unit/test_batch.py index 187d44913f..3112f17ecf 100644 --- a/tests/unit/test_batch.py +++ b/tests/unit/test_batch.py @@ -14,7 +14,7 @@ import unittest -from tests._helpers import OpenTelemetryBase, StatusCanonicalCode +from tests._helpers import OpenTelemetryBase, StatusCode TABLE_NAME = "citizens" COLUMNS = ["email", "first_name", "last_name", "age"] @@ -207,7 +207,7 @@ def test_commit_grpc_error(self): self.assertSpanAttributes( "CloudSpanner.Commit", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=dict(BASE_ATTRIBUTES, num_mutations=1), ) diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py index f80b360b96..597f64a8bc 100644 --- a/tests/unit/test_session.py +++ b/tests/unit/test_session.py @@ -17,7 +17,7 @@ import mock from tests._helpers import ( OpenTelemetryBase, - StatusCanonicalCode, + StatusCode, HAS_OPENTELEMETRY_INSTALLED, ) @@ -192,7 +192,7 @@ def test_create_error(self): self.assertSpanAttributes( "CloudSpanner.CreateSession", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=TestSession.BASE_ATTRIBUTES, ) @@ -311,7 +311,7 @@ def test_exists_error(self): self.assertSpanAttributes( "CloudSpanner.GetSession", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=TestSession.BASE_ATTRIBUTES, ) @@ -427,7 +427,7 @@ def test_delete_miss(self): self.assertSpanAttributes( "CloudSpanner.DeleteSession", - status=StatusCanonicalCode.NOT_FOUND, + status=StatusCode.ERROR, attributes=TestSession.BASE_ATTRIBUTES, ) @@ -451,7 +451,7 @@ def test_delete_error(self): self.assertSpanAttributes( "CloudSpanner.DeleteSession", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=TestSession.BASE_ATTRIBUTES, ) diff --git a/tests/unit/test_snapshot.py b/tests/unit/test_snapshot.py index 24f87a30fc..c03ae23e91 100644 --- a/tests/unit/test_snapshot.py +++ b/tests/unit/test_snapshot.py @@ -17,7 +17,7 @@ import mock from tests._helpers import ( OpenTelemetryBase, - StatusCanonicalCode, + StatusCode, HAS_OPENTELEMETRY_INSTALLED, ) from google.cloud.spanner_v1.param_types import INT64 @@ -386,7 +386,7 @@ def test_read_other_error(self): self.assertSpanAttributes( "CloudSpanner.ReadOnlyTransaction", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=dict( BASE_ATTRIBUTES, table_id=TABLE_NAME, columns=tuple(COLUMNS) ), @@ -568,7 +568,7 @@ def test_execute_sql_other_error(self): self.assertSpanAttributes( "CloudSpanner.ReadWriteTransaction", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=dict(BASE_ATTRIBUTES, **{"db.statement": SQL_QUERY}), ) @@ -709,7 +709,7 @@ def _execute_sql_helper( self.assertSpanAttributes( "CloudSpanner.ReadWriteTransaction", - status=StatusCanonicalCode.OK, + status=StatusCode.OK, attributes=dict(BASE_ATTRIBUTES, **{"db.statement": SQL_QUERY_WITH_PARAM}), ) @@ -824,7 +824,7 @@ def _partition_read_helper( self.assertSpanAttributes( "CloudSpanner.PartitionReadOnlyTransaction", - status=StatusCanonicalCode.OK, + status=StatusCode.OK, attributes=dict( BASE_ATTRIBUTES, table_id=TABLE_NAME, columns=tuple(COLUMNS) ), @@ -855,7 +855,7 @@ def test_partition_read_other_error(self): self.assertSpanAttributes( "CloudSpanner.PartitionReadOnlyTransaction", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=dict( BASE_ATTRIBUTES, table_id=TABLE_NAME, columns=tuple(COLUMNS) ), @@ -961,7 +961,7 @@ def _partition_query_helper( self.assertSpanAttributes( "CloudSpanner.PartitionReadWriteTransaction", - status=StatusCanonicalCode.OK, + status=StatusCode.OK, attributes=dict(BASE_ATTRIBUTES, **{"db.statement": SQL_QUERY_WITH_PARAM}), ) @@ -979,7 +979,7 @@ def test_partition_query_other_error(self): self.assertSpanAttributes( "CloudSpanner.PartitionReadWriteTransaction", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=dict(BASE_ATTRIBUTES, **{"db.statement": SQL_QUERY}), ) @@ -1308,7 +1308,7 @@ def test_begin_w_other_error(self): self.assertSpanAttributes( "CloudSpanner.BeginTransaction", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=BASE_ATTRIBUTES, ) @@ -1345,7 +1345,7 @@ def test_begin_ok_exact_staleness(self): self.assertSpanAttributes( "CloudSpanner.BeginTransaction", - status=StatusCanonicalCode.OK, + status=StatusCode.OK, attributes=BASE_ATTRIBUTES, ) @@ -1379,7 +1379,7 @@ def test_begin_ok_exact_strong(self): self.assertSpanAttributes( "CloudSpanner.BeginTransaction", - status=StatusCanonicalCode.OK, + status=StatusCode.OK, attributes=BASE_ATTRIBUTES, ) diff --git a/tests/unit/test_transaction.py b/tests/unit/test_transaction.py index 923a6ec47d..99f986d99e 100644 --- a/tests/unit/test_transaction.py +++ b/tests/unit/test_transaction.py @@ -14,7 +14,7 @@ import mock -from tests._helpers import OpenTelemetryBase, StatusCanonicalCode +from tests._helpers import OpenTelemetryBase, StatusCode from google.cloud.spanner_v1 import Type from google.cloud.spanner_v1 import TypeCode from google.api_core.retry import Retry @@ -161,7 +161,7 @@ def test_begin_w_other_error(self): self.assertSpanAttributes( "CloudSpanner.BeginTransaction", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=TestTransaction.BASE_ATTRIBUTES, ) @@ -234,7 +234,7 @@ def test_rollback_w_other_error(self): self.assertSpanAttributes( "CloudSpanner.Rollback", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=TestTransaction.BASE_ATTRIBUTES, ) @@ -307,7 +307,7 @@ def test_commit_w_other_error(self): self.assertSpanAttributes( "CloudSpanner.Commit", - status=StatusCanonicalCode.UNKNOWN, + status=StatusCode.ERROR, attributes=dict(TestTransaction.BASE_ATTRIBUTES, num_mutations=1), ) From 40e6dcd8a05d7a0db6ee18f1ef1d2337bbee599f Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 11:10:19 +0530 Subject: [PATCH 04/14] fix: open telemetry schema related changes and fixes for tests to work with in memory exporter --- tests/_helpers.py | 47 +++++++++++++++++++++++++++---------- tests/unit/test_snapshot.py | 2 +- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/tests/_helpers.py b/tests/_helpers.py index 577caccf87..42178fd439 100644 --- a/tests/_helpers.py +++ b/tests/_helpers.py @@ -2,13 +2,15 @@ import mock try: - from opentelemetry import trace as trace_api - from opentelemetry.trace.status import StatusCode - - from opentelemetry.sdk.trace import TracerProvider, export + from opentelemetry import trace + from opentelemetry.sdk.trace import TracerProvider + from opentelemetry.sdk.trace.export import SimpleSpanProcessor from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( InMemorySpanExporter, ) + from opentelemetry.trace.status import StatusCode + + trace.set_tracer_provider(TracerProvider()) HAS_OPENTELEMETRY_INSTALLED = True except ImportError: @@ -16,24 +18,45 @@ StatusCode = mock.Mock() +_TEST_OT_EXPORTER = None +_TEST_OT_PROVIDER_INITIALIZED = False + + +def get_test_ot_exporter(): + global _TEST_OT_EXPORTER + + if _TEST_OT_EXPORTER is None: + _TEST_OT_EXPORTER = InMemorySpanExporter() + return _TEST_OT_EXPORTER + + +def use_test_ot_exporter(): + global _TEST_OT_PROVIDER_INITIALIZED + + if _TEST_OT_PROVIDER_INITIALIZED: + return + + provider = trace.get_tracer_provider() + if not hasattr(provider, "add_span_processor"): + return + provider.add_span_processor(SimpleSpanProcessor(get_test_ot_exporter())) + _TEST_OT_PROVIDER_INITIALIZED = True + class OpenTelemetryBase(unittest.TestCase): @classmethod def setUpClass(cls): if HAS_OPENTELEMETRY_INSTALLED: - cls.tracer_provider = TracerProvider() - cls.memory_exporter = InMemorySpanExporter() - cls.span_processor = export.SimpleSpanProcessor(cls.memory_exporter) - cls.tracer_provider.add_span_processor(cls.span_processor) - trace_api.set_tracer_provider(cls.tracer_provider) + use_test_ot_exporter() + cls.ot_exporter = get_test_ot_exporter() def tearDown(self): if HAS_OPENTELEMETRY_INSTALLED: - self.memory_exporter.clear() + self.ot_exporter.clear() def assertNoSpans(self): if HAS_OPENTELEMETRY_INSTALLED: - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 0) def assertSpanAttributes( @@ -41,7 +64,7 @@ def assertSpanAttributes( ): if HAS_OPENTELEMETRY_INSTALLED: if not span: - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) span = span_list[0] diff --git a/tests/unit/test_snapshot.py b/tests/unit/test_snapshot.py index c03ae23e91..bbc1753474 100644 --- a/tests/unit/test_snapshot.py +++ b/tests/unit/test_snapshot.py @@ -296,7 +296,7 @@ def test_iteration_w_multiple_span_creation(self): self.assertEqual(len(restart.mock_calls), 2) self.assertEqual(request.resume_token, RESUME_TOKEN) - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 2) for span in span_list: self.assertEqual(span.name, name) From 5b26aeeedb92b4caa1e3b1458aca36a95bd75901 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 13:28:26 +0530 Subject: [PATCH 05/14] fix: variable name correction for ot_exporter --- tests/system/test_system.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 2704e27b53..9dd1462483 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -1165,6 +1165,8 @@ class TestSessionAPI(OpenTelemetryBase, _TestData): @classmethod def setUpClass(cls): + # Call SetUpClass from parent (OpenTelemetryBase) + super(TestSessionAPI, cls).setUpClass() pool = BurstyPool(labels={"testcase": "session_api"}) ddl_statements = EMULATOR_DDL_STATEMENTS if USE_EMULATOR else DDL_STATEMENTS cls._db = Config.INSTANCE.database( @@ -1187,6 +1189,8 @@ def tearDown(self): super(TestSessionAPI, self).tearDown() for doomed in self.to_delete: doomed.delete() + if HAS_OPENTELEMETRY_INSTALLED: + self.ot_exporter.clear() # Clear any ot spans from above step. def test_session_crud(self): retry_true = RetryResult(operator.truth) @@ -1211,7 +1215,7 @@ def test_batch_insert_then_read(self): self._check_rows_data(rows) if HAS_OPENTELEMETRY_INSTALLED: - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 4) self.assertSpanAttributes( "CloudSpanner.GetSession", @@ -1355,7 +1359,7 @@ def test_transaction_read_and_insert_then_rollback(self): self.assertEqual(rows, []) if HAS_OPENTELEMETRY_INSTALLED: - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 8) self.assertSpanAttributes( "CloudSpanner.CreateSession", @@ -1768,7 +1772,7 @@ def unit_of_work(transaction, self): with tracer.start_as_current_span("Test Span"): session.run_in_transaction(unit_of_work, self) - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 6) self.assertEqual( list(map(lambda span: span.name, span_list)), From 1bad305ca31573f29e61f5bfd3e3ddd1678b55bd Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 15:01:25 +0530 Subject: [PATCH 06/14] fix: correct variable name from memeory_exporter to ot_exporter --- tests/unit/test__opentelemetry_tracing.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test__opentelemetry_tracing.py b/tests/unit/test__opentelemetry_tracing.py index 24a6334b99..25870227bf 100644 --- a/tests/unit/test__opentelemetry_tracing.py +++ b/tests/unit/test__opentelemetry_tracing.py @@ -69,7 +69,7 @@ def test_trace_call(self): expected_attributes["after_setup_attribute"] = 1 - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) span = span_list[0] self.assertEqual(span.kind, trace_api.SpanKind.CLIENT) @@ -95,7 +95,7 @@ def test_trace_error(self): raise _make_rpc_error(InvalidArgument) - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) span = span_list[0] self.assertEqual(span.kind, trace_api.SpanKind.CLIENT) @@ -121,7 +121,7 @@ def test_trace_grpc_error(self): raise DataLoss("error") - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) span = span_list[0] self.assertEqual(span.status.status_code, StatusCode.ERROR) @@ -142,7 +142,7 @@ def test_trace_codeless_error(self): ) as span: raise GoogleAPICallError("error") - span_list = self.memory_exporter.get_finished_spans() + span_list = self.ot_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) span = span_list[0] self.assertEqual(span.status.status_code, StatusCode.ERROR) From a265a3452a309fa850bc8e0ea9520457ec01bee1 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 16:53:32 +0530 Subject: [PATCH 07/14] fix: remove patch for opentelemetry.util.time with _constant_time as it was not used --- tests/unit/test_session.py | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py index 597f64a8bc..f0586a6db5 100644 --- a/tests/unit/test_session.py +++ b/tests/unit/test_session.py @@ -30,11 +30,6 @@ def _make_rpc_error(error_cls, trailing_metadata=None): return error_cls("error", errors=(grpc_error,)) -class _ConstantTime: - def time(self): - return 1 - - class TestSession(OpenTelemetryBase): PROJECT_ID = "project-id" @@ -1189,17 +1184,9 @@ def _time(_results=[1, 1.5]): return _results.pop(0) with mock.patch("time.time", _time): - if HAS_OPENTELEMETRY_INSTALLED: - with mock.patch("opentelemetry.util.time", _ConstantTime()): - with mock.patch("time.sleep") as sleep_mock: - with self.assertRaises(Aborted): - session.run_in_transaction( - unit_of_work, "abc", timeout_secs=1 - ) - else: - with mock.patch("time.sleep") as sleep_mock: - with self.assertRaises(Aborted): - session.run_in_transaction(unit_of_work, "abc", timeout_secs=1) + with mock.patch("time.sleep") as sleep_mock: + with self.assertRaises(Aborted): + session.run_in_transaction(unit_of_work, "abc", timeout_secs=1) sleep_mock.assert_not_called() @@ -1262,15 +1249,9 @@ def _time(_results=[1, 2, 4, 8]): return _results.pop(0) with mock.patch("time.time", _time): - if HAS_OPENTELEMETRY_INSTALLED: - with mock.patch("opentelemetry.util.time", _ConstantTime()): - with mock.patch("time.sleep") as sleep_mock: - with self.assertRaises(Aborted): - session.run_in_transaction(unit_of_work, timeout_secs=8) - else: - with mock.patch("time.sleep") as sleep_mock: - with self.assertRaises(Aborted): - session.run_in_transaction(unit_of_work, timeout_secs=8) + with mock.patch("time.sleep") as sleep_mock: + with self.assertRaises(Aborted): + session.run_in_transaction(unit_of_work, timeout_secs=8) # unpacking call args into list call_args = [call_[0][0] for call_ in sleep_mock.call_args_list] From b7596a73586b6767d9b4040e918f6afcb3425587 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 18:40:36 +0530 Subject: [PATCH 08/14] refactor: correct opentelemetry.util.time to opentelemetry.util._time --- tests/unit/test_session.py | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py index f0586a6db5..9c2e9dce3c 100644 --- a/tests/unit/test_session.py +++ b/tests/unit/test_session.py @@ -30,6 +30,11 @@ def _make_rpc_error(error_cls, trailing_metadata=None): return error_cls("error", errors=(grpc_error,)) +class _ConstantTime: + def time(self): + return 1 + + class TestSession(OpenTelemetryBase): PROJECT_ID = "project-id" @@ -1184,9 +1189,17 @@ def _time(_results=[1, 1.5]): return _results.pop(0) with mock.patch("time.time", _time): - with mock.patch("time.sleep") as sleep_mock: - with self.assertRaises(Aborted): - session.run_in_transaction(unit_of_work, "abc", timeout_secs=1) + if HAS_OPENTELEMETRY_INSTALLED: + with mock.patch("opentelemetry.util._time", _ConstantTime()): + with mock.patch("time.sleep") as sleep_mock: + with self.assertRaises(Aborted): + session.run_in_transaction( + unit_of_work, "abc", timeout_secs=1 + ) + else: + with mock.patch("time.sleep") as sleep_mock: + with self.assertRaises(Aborted): + session.run_in_transaction(unit_of_work, "abc", timeout_secs=1) sleep_mock.assert_not_called() @@ -1249,9 +1262,15 @@ def _time(_results=[1, 2, 4, 8]): return _results.pop(0) with mock.patch("time.time", _time): - with mock.patch("time.sleep") as sleep_mock: - with self.assertRaises(Aborted): - session.run_in_transaction(unit_of_work, timeout_secs=8) + if HAS_OPENTELEMETRY_INSTALLED: + with mock.patch("opentelemetry.util._time", _ConstantTime()): + with mock.patch("time.sleep") as sleep_mock: + with self.assertRaises(Aborted): + session.run_in_transaction(unit_of_work, timeout_secs=8) + else: + with mock.patch("time.sleep") as sleep_mock: + with self.assertRaises(Aborted): + session.run_in_transaction(unit_of_work, timeout_secs=8) # unpacking call args into list call_args = [call_[0][0] for call_ in sleep_mock.call_args_list] From 0d769b7750d33a9b3fd25597bd7e35b885fd5a8a Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 19:55:00 +0530 Subject: [PATCH 09/14] ci: update packages for open telemetry --- testing/constraints-3.6.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/constraints-3.6.txt b/testing/constraints-3.6.txt index bfb81c38a2..8444f845b8 100644 --- a/testing/constraints-3.6.txt +++ b/testing/constraints-3.6.txt @@ -11,6 +11,6 @@ grpc-google-iam-v1==0.12.3 libcst==0.2.5 proto-plus==1.13.0 sqlparse==0.3.0 -opentelemetry-api==0.11b0 -opentelemetry-sdk==0.11b0 -opentelemetry-instrumentation==0.11b0 +opentelemetry-api==1.3.0 +opentelemetry-sdk==1.3.0 +opentelemetry-instrumentation==0.22b0 From c096925c796973f80bac2e55c90f8cc593e4bc97 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 22:25:07 +0530 Subject: [PATCH 10/14] refactor: increased version of open telemetry as per new specs --- setup.py | 6 +++--- testing/constraints-3.10.txt | 16 ++++++++++++++++ testing/constraints-3.11.txt | 16 ++++++++++++++++ testing/constraints-3.6.txt | 4 ++-- testing/constraints-3.7.txt | 16 ++++++++++++++++ testing/constraints-3.8.txt | 16 ++++++++++++++++ testing/constraints-3.9.txt | 16 ++++++++++++++++ 7 files changed, 85 insertions(+), 5 deletions(-) diff --git a/setup.py b/setup.py index b12cd90f09..370b3c0293 100644 --- a/setup.py +++ b/setup.py @@ -37,9 +37,9 @@ ] extras = { "tracing": [ - "opentelemetry-api >= 0.11b0", - "opentelemetry-sdk >= 0.11b0", - "opentelemetry-instrumentation >= 0.11b0", + "opentelemetry-api >= 1.3.0", + "opentelemetry-sdk >= 1.3.0", + "opentelemetry-instrumentation >= 0.22b0", ], "libcst": "libcst >= 0.2.5", } diff --git a/testing/constraints-3.10.txt b/testing/constraints-3.10.txt index e69de29bb2..d77afd5a13 100644 --- a/testing/constraints-3.10.txt +++ b/testing/constraints-3.10.txt @@ -0,0 +1,16 @@ +# This constraints file is used to check that lower bounds +# are correct in setup.py +# List *all* library dependencies and extras in this file. +# Pin the version to the lower bound. +# +# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", +# Then this file should have foo==1.14.0 +google-api-core==1.22.2 +google-cloud-core==1.4.1 +grpc-google-iam-v1==0.12.3 +libcst==0.2.5 +proto-plus==1.13.0 +sqlparse==0.3.0 +opentelemetry-api==1.1.0 +opentelemetry-sdk==1.1.0 +opentelemetry-instrumentation==0.22b0 diff --git a/testing/constraints-3.11.txt b/testing/constraints-3.11.txt index e69de29bb2..d77afd5a13 100644 --- a/testing/constraints-3.11.txt +++ b/testing/constraints-3.11.txt @@ -0,0 +1,16 @@ +# This constraints file is used to check that lower bounds +# are correct in setup.py +# List *all* library dependencies and extras in this file. +# Pin the version to the lower bound. +# +# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", +# Then this file should have foo==1.14.0 +google-api-core==1.22.2 +google-cloud-core==1.4.1 +grpc-google-iam-v1==0.12.3 +libcst==0.2.5 +proto-plus==1.13.0 +sqlparse==0.3.0 +opentelemetry-api==1.1.0 +opentelemetry-sdk==1.1.0 +opentelemetry-instrumentation==0.22b0 diff --git a/testing/constraints-3.6.txt b/testing/constraints-3.6.txt index 8444f845b8..d77afd5a13 100644 --- a/testing/constraints-3.6.txt +++ b/testing/constraints-3.6.txt @@ -11,6 +11,6 @@ grpc-google-iam-v1==0.12.3 libcst==0.2.5 proto-plus==1.13.0 sqlparse==0.3.0 -opentelemetry-api==1.3.0 -opentelemetry-sdk==1.3.0 +opentelemetry-api==1.1.0 +opentelemetry-sdk==1.1.0 opentelemetry-instrumentation==0.22b0 diff --git a/testing/constraints-3.7.txt b/testing/constraints-3.7.txt index e69de29bb2..d77afd5a13 100644 --- a/testing/constraints-3.7.txt +++ b/testing/constraints-3.7.txt @@ -0,0 +1,16 @@ +# This constraints file is used to check that lower bounds +# are correct in setup.py +# List *all* library dependencies and extras in this file. +# Pin the version to the lower bound. +# +# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", +# Then this file should have foo==1.14.0 +google-api-core==1.22.2 +google-cloud-core==1.4.1 +grpc-google-iam-v1==0.12.3 +libcst==0.2.5 +proto-plus==1.13.0 +sqlparse==0.3.0 +opentelemetry-api==1.1.0 +opentelemetry-sdk==1.1.0 +opentelemetry-instrumentation==0.22b0 diff --git a/testing/constraints-3.8.txt b/testing/constraints-3.8.txt index e69de29bb2..d77afd5a13 100644 --- a/testing/constraints-3.8.txt +++ b/testing/constraints-3.8.txt @@ -0,0 +1,16 @@ +# This constraints file is used to check that lower bounds +# are correct in setup.py +# List *all* library dependencies and extras in this file. +# Pin the version to the lower bound. +# +# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", +# Then this file should have foo==1.14.0 +google-api-core==1.22.2 +google-cloud-core==1.4.1 +grpc-google-iam-v1==0.12.3 +libcst==0.2.5 +proto-plus==1.13.0 +sqlparse==0.3.0 +opentelemetry-api==1.1.0 +opentelemetry-sdk==1.1.0 +opentelemetry-instrumentation==0.22b0 diff --git a/testing/constraints-3.9.txt b/testing/constraints-3.9.txt index e69de29bb2..d77afd5a13 100644 --- a/testing/constraints-3.9.txt +++ b/testing/constraints-3.9.txt @@ -0,0 +1,16 @@ +# This constraints file is used to check that lower bounds +# are correct in setup.py +# List *all* library dependencies and extras in this file. +# Pin the version to the lower bound. +# +# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", +# Then this file should have foo==1.14.0 +google-api-core==1.22.2 +google-cloud-core==1.4.1 +grpc-google-iam-v1==0.12.3 +libcst==0.2.5 +proto-plus==1.13.0 +sqlparse==0.3.0 +opentelemetry-api==1.1.0 +opentelemetry-sdk==1.1.0 +opentelemetry-instrumentation==0.22b0 From 7eb8f6f5c8d8d1bdf93e5674e14901930ecbae60 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Fri, 4 Jun 2021 23:11:50 +0530 Subject: [PATCH 11/14] fix: changed opentelemetry dependency version --- setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 370b3c0293..3687fbbe45 100644 --- a/setup.py +++ b/setup.py @@ -37,8 +37,8 @@ ] extras = { "tracing": [ - "opentelemetry-api >= 1.3.0", - "opentelemetry-sdk >= 1.3.0", + "opentelemetry-api >= 1.1.0", + "opentelemetry-sdk >= 1.1.0", "opentelemetry-instrumentation >= 0.22b0", ], "libcst": "libcst >= 0.2.5", From 9bcad20e37bea6a7258056d25d3846a2c16d74dc Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Tue, 8 Jun 2021 13:45:01 +0530 Subject: [PATCH 12/14] updated constraints file with opentelemetry-instrumentation >= 0.20b0 --- setup.py | 2 +- testing/constraints-3.10.txt | 2 +- testing/constraints-3.11.txt | 2 +- testing/constraints-3.6.txt | 2 +- testing/constraints-3.7.txt | 2 +- testing/constraints-3.8.txt | 2 +- testing/constraints-3.9.txt | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/setup.py b/setup.py index 3687fbbe45..5a33b75ee8 100644 --- a/setup.py +++ b/setup.py @@ -39,7 +39,7 @@ "tracing": [ "opentelemetry-api >= 1.1.0", "opentelemetry-sdk >= 1.1.0", - "opentelemetry-instrumentation >= 0.22b0", + "opentelemetry-instrumentation >= 0.20b0", ], "libcst": "libcst >= 0.2.5", } diff --git a/testing/constraints-3.10.txt b/testing/constraints-3.10.txt index d77afd5a13..f3d4031bf4 100644 --- a/testing/constraints-3.10.txt +++ b/testing/constraints-3.10.txt @@ -13,4 +13,4 @@ proto-plus==1.13.0 sqlparse==0.3.0 opentelemetry-api==1.1.0 opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.22b0 +opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.11.txt b/testing/constraints-3.11.txt index d77afd5a13..f3d4031bf4 100644 --- a/testing/constraints-3.11.txt +++ b/testing/constraints-3.11.txt @@ -13,4 +13,4 @@ proto-plus==1.13.0 sqlparse==0.3.0 opentelemetry-api==1.1.0 opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.22b0 +opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.6.txt b/testing/constraints-3.6.txt index d77afd5a13..f3d4031bf4 100644 --- a/testing/constraints-3.6.txt +++ b/testing/constraints-3.6.txt @@ -13,4 +13,4 @@ proto-plus==1.13.0 sqlparse==0.3.0 opentelemetry-api==1.1.0 opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.22b0 +opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.7.txt b/testing/constraints-3.7.txt index d77afd5a13..f3d4031bf4 100644 --- a/testing/constraints-3.7.txt +++ b/testing/constraints-3.7.txt @@ -13,4 +13,4 @@ proto-plus==1.13.0 sqlparse==0.3.0 opentelemetry-api==1.1.0 opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.22b0 +opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.8.txt b/testing/constraints-3.8.txt index d77afd5a13..f3d4031bf4 100644 --- a/testing/constraints-3.8.txt +++ b/testing/constraints-3.8.txt @@ -13,4 +13,4 @@ proto-plus==1.13.0 sqlparse==0.3.0 opentelemetry-api==1.1.0 opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.22b0 +opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.9.txt b/testing/constraints-3.9.txt index d77afd5a13..f3d4031bf4 100644 --- a/testing/constraints-3.9.txt +++ b/testing/constraints-3.9.txt @@ -13,4 +13,4 @@ proto-plus==1.13.0 sqlparse==0.3.0 opentelemetry-api==1.1.0 opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.22b0 +opentelemetry-instrumentation==0.20b0 From 4e90ca857746274ac8535dda24986b0b02d57492 Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Tue, 8 Jun 2021 14:30:14 +0530 Subject: [PATCH 13/14] fix: reverted the change in constraints files from 3.7 to 3.11 --- testing/constraints-3.10.txt | 16 ---------------- testing/constraints-3.11.txt | 16 ---------------- testing/constraints-3.7.txt | 16 ---------------- testing/constraints-3.8.txt | 16 ---------------- testing/constraints-3.9.txt | 16 ---------------- 5 files changed, 80 deletions(-) diff --git a/testing/constraints-3.10.txt b/testing/constraints-3.10.txt index f3d4031bf4..e69de29bb2 100644 --- a/testing/constraints-3.10.txt +++ b/testing/constraints-3.10.txt @@ -1,16 +0,0 @@ -# This constraints file is used to check that lower bounds -# are correct in setup.py -# List *all* library dependencies and extras in this file. -# Pin the version to the lower bound. -# -# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", -# Then this file should have foo==1.14.0 -google-api-core==1.22.2 -google-cloud-core==1.4.1 -grpc-google-iam-v1==0.12.3 -libcst==0.2.5 -proto-plus==1.13.0 -sqlparse==0.3.0 -opentelemetry-api==1.1.0 -opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.11.txt b/testing/constraints-3.11.txt index f3d4031bf4..e69de29bb2 100644 --- a/testing/constraints-3.11.txt +++ b/testing/constraints-3.11.txt @@ -1,16 +0,0 @@ -# This constraints file is used to check that lower bounds -# are correct in setup.py -# List *all* library dependencies and extras in this file. -# Pin the version to the lower bound. -# -# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", -# Then this file should have foo==1.14.0 -google-api-core==1.22.2 -google-cloud-core==1.4.1 -grpc-google-iam-v1==0.12.3 -libcst==0.2.5 -proto-plus==1.13.0 -sqlparse==0.3.0 -opentelemetry-api==1.1.0 -opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.7.txt b/testing/constraints-3.7.txt index f3d4031bf4..e69de29bb2 100644 --- a/testing/constraints-3.7.txt +++ b/testing/constraints-3.7.txt @@ -1,16 +0,0 @@ -# This constraints file is used to check that lower bounds -# are correct in setup.py -# List *all* library dependencies and extras in this file. -# Pin the version to the lower bound. -# -# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", -# Then this file should have foo==1.14.0 -google-api-core==1.22.2 -google-cloud-core==1.4.1 -grpc-google-iam-v1==0.12.3 -libcst==0.2.5 -proto-plus==1.13.0 -sqlparse==0.3.0 -opentelemetry-api==1.1.0 -opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.8.txt b/testing/constraints-3.8.txt index f3d4031bf4..e69de29bb2 100644 --- a/testing/constraints-3.8.txt +++ b/testing/constraints-3.8.txt @@ -1,16 +0,0 @@ -# This constraints file is used to check that lower bounds -# are correct in setup.py -# List *all* library dependencies and extras in this file. -# Pin the version to the lower bound. -# -# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", -# Then this file should have foo==1.14.0 -google-api-core==1.22.2 -google-cloud-core==1.4.1 -grpc-google-iam-v1==0.12.3 -libcst==0.2.5 -proto-plus==1.13.0 -sqlparse==0.3.0 -opentelemetry-api==1.1.0 -opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.20b0 diff --git a/testing/constraints-3.9.txt b/testing/constraints-3.9.txt index f3d4031bf4..e69de29bb2 100644 --- a/testing/constraints-3.9.txt +++ b/testing/constraints-3.9.txt @@ -1,16 +0,0 @@ -# This constraints file is used to check that lower bounds -# are correct in setup.py -# List *all* library dependencies and extras in this file. -# Pin the version to the lower bound. -# -# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", -# Then this file should have foo==1.14.0 -google-api-core==1.22.2 -google-cloud-core==1.4.1 -grpc-google-iam-v1==0.12.3 -libcst==0.2.5 -proto-plus==1.13.0 -sqlparse==0.3.0 -opentelemetry-api==1.1.0 -opentelemetry-sdk==1.1.0 -opentelemetry-instrumentation==0.20b0 From dcc0792fbf5bcd58efa0674b9ff16a4595987c4c Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Wed, 9 Jun 2021 12:32:54 +0530 Subject: [PATCH 14/14] fix: added ot_exporter clear call after reload to clear out the exporter memeory --- tests/system/test_system.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 9dd1462483..7c1c0d6f64 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -1740,6 +1740,9 @@ def test_transaction_batch_update_w_parent_span(self): retry = RetryInstanceState(_has_all_ddl) retry(self._db.reload)() + if HAS_OPENTELEMETRY_INSTALLED: + self.ot_exporter.clear() # Clear any ot spans from above steps. + session = self._db.session() session.create() self.to_delete.append(session)