Address review feedback

googleapis · Nov 14, 2024 · 8c5b50b · 8c5b50b
1 parent 6c8bb88
commit 8c5b50b
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 26 deletions.
diff --git a/google/cloud/spanner_v1/_opentelemetry_tracing.py b/google/cloud/spanner_v1/_opentelemetry_tracing.py
@@ -35,7 +35,7 @@
 TRACER_NAME = "cloud.google.com/python/spanner"
 TRACER_VERSION = gapic_version.__version__
 extended_tracing_globally_disabled = (
-    os.environ.get("SPANNER_ENABLE_EXTENDED_TRACING", "").lower() == "false"
+    os.getenv("SPANNER_ENABLE_EXTENDED_TRACING", "").lower() == "false"
 )
 
 
@@ -68,7 +68,7 @@ def trace_call(name, session, extra_attributes=None, observability_options=None)
     # on by default.
     enable_extended_tracing = True
 
-    if type(observability_options) == dict:  # Avoid false positives with mock.Mock
+    if isinstance(observability_options, dict):  # Avoid false positives with mock.Mock
         tracer_provider = observability_options.get("tracer_provider", None)
         enable_extended_tracing = observability_options.get(
             "enable_extended_tracing", enable_extended_tracing

diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py
@@ -126,7 +126,7 @@ class Client(ClientWithProject):
             for all ReadRequests and ExecuteSqlRequests that indicates which replicas
             or regions should be used for non-transactional reads or queries.
 
-    :type labels: dict (str -> any) or None
+    :type observability_options: dict (str -> any) or None
     :param observability_options: (Optional) the configuration to control
            the tracer's behavior.
            tracer_provider is the injected tracer provider

diff --git a/tests/system/test_observability_options.py b/tests/system/test_observability_options.py
@@ -14,10 +14,9 @@
 
 import mock
 import pytest
-import unittest
 
 from . import _helpers
-from google.cloud.spanner_v1 import Client, DirectedReadOptions
+from google.cloud.spanner_v1 import Client
 
 HAS_OTEL_INSTALLED = False
 
@@ -35,32 +34,20 @@
     pass
 
 
-@pytest.mark.skipif(not HAS_OTEL_INSTALLED, reason="OpenTelemetry needed.")
-@pytest.mark.skipif(not _helpers.USE_EMULATOR, reason="Emulator needed.")
+@pytest.mark.skipif(
+    not HAS_OTEL_INSTALLED, reason="OpenTelemetry is necessary to test traces."
+)
+@pytest.mark.skipif(
+    not _helpers.USE_EMULATOR, reason="mulator is necessary to test traces."
+)
 def test_observability_options_propagation():
     PROJECT = _helpers.EMULATOR_PROJECT
-    PATH = "projects/%s" % (PROJECT,)
     CONFIGURATION_NAME = "config-name"
     INSTANCE_ID = _helpers.INSTANCE_ID
-    INSTANCE_NAME = "%s/instances/%s" % (PATH, INSTANCE_ID)
     DISPLAY_NAME = "display-name"
     DATABASE_ID = _helpers.unique_id("temp_db")
     NODE_COUNT = 5
-    PROCESSING_UNITS = 5000
     LABELS = {"test": "true"}
-    TIMEOUT_SECONDS = 80
-    LEADER_OPTIONS = ["leader1", "leader2"]
-    DIRECTED_READ_OPTIONS = {
-        "include_replicas": {
-            "replica_selections": [
-                {
-                    "location": "us-west1",
-                    "type_": DirectedReadOptions.ReplicaSelection.Type.READ_ONLY,
-                },
-            ],
-            "auto_failover_disabled": True,
-        },
-    }
 
     def test_propagation(enable_extended_tracing):
         global_tracer_provider = TracerProvider(sampler=ALWAYS_ON)
@@ -95,13 +82,13 @@ def test_propagation(enable_extended_tracing):
 
         try:
             instance.create()
-        except:
+        except Exception:
             pass
 
         db = instance.database(DATABASE_ID)
         try:
             db.create()
-        except:
+        except Exception:
             pass
 
         assert db.observability_options == observability_options
@@ -134,7 +121,7 @@ def test_propagation(enable_extended_tracing):
         try:
             db.delete()
             instance.delete()
-        except:
+        except Exception:
             pass
 
     # Test the respective options for enable_extended_tracing