Address review feedback by attaching observability_options to Instance

odeke-em · odeke-em · commit fef29eab6c13 · 2024-11-11T22:02:19.000-08:00
diff --git a/google/cloud/spanner_v1/_opentelemetry_tracing.py b/google/cloud/spanner_v1/_opentelemetry_tracing.py
@@ -73,8 +73,8 @@ def trace_call(name, session, extra_attributes=None, observability_options=None)
 
     tracer_provider = None
     enable_extended_tracing = False
-    if getattr(session, "_observability_options", None):
-        opts = session._observability_options
+    if getattr(session, "_database", None):
+        opts = getattr(session._database, "observability_options", None)
         if opts:
             tracer_provider = opts.tracer_provider
             enable_extended_tracing = opts.enable_extended_tracing
diff --git a/google/cloud/spanner_v1/database.py b/google/cloud/spanner_v1/database.py
@@ -156,7 +156,6 @@ def __init__(
         database_role=None,
         enable_drop_protection=False,
         proto_descriptors=None,
-        observability_options=None,
     ):
         self.database_id = database_id
         self._instance = instance
@@ -179,16 +178,13 @@ def __init__(
         self._reconciling = False
         self._directed_read_options = self._instance._client.directed_read_options
         self._proto_descriptors = proto_descriptors
-        self._observability_options = observability_options
 
         if pool is None:
             pool = BurstyPool(
                 database_role=database_role,
-                observability_options=self._observability_options,
             )
 
         self._pool = pool
-        self._pool._observability_options = observability_options
         pool.bind(self)
 
     @classmethod
@@ -752,7 +748,6 @@ def session(self, labels=None, database_role=None):
             self,
             labels=labels,
             database_role=role,
-            observability_options=self._observability_options,
         )
 
     def snapshot(self, **kw):
@@ -1722,6 +1717,10 @@ def close(self):
         if self._session is not None:
             self._session.delete()
 
+    @property
+    def observability_options(self):
+        return self._instance._observability_options
+
 
 def _check_ddl_statements(value):
     """Validate DDL Statements used to define database schema.
diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py
@@ -508,7 +508,6 @@ def database(
                 database_role=database_role,
                 enable_drop_protection=enable_drop_protection,
                 proto_descriptors=proto_descriptors,
-                observability_options=self._observability_options,
             )
         else:
             return TestDatabase(
diff --git a/google/cloud/spanner_v1/pool.py b/google/cloud/spanner_v1/pool.py
@@ -42,12 +42,11 @@ class AbstractSessionPool(object):
 
     _database = None
 
-    def __init__(self, labels=None, database_role=None, observability_options=None):
+    def __init__(self, labels=None, database_role=None):
         if labels is None:
             labels = {}
         self._labels = labels
         self._database_role = database_role
-        self._observability_options = observability_options
 
     @property
     def labels(self):
@@ -179,12 +178,10 @@ def __init__(
         default_timeout=DEFAULT_TIMEOUT,
         labels=None,
         database_role=None,
-        observability_options=None,
     ):
         super(FixedSizePool, self).__init__(
             labels=labels,
             database_role=database_role,
-            observability_options=observability_options,
         )
         self.size = size
         self.default_timeout = default_timeout
@@ -295,12 +292,10 @@ def __init__(
         target_size=10,
         labels=None,
         database_role=None,
-        observability_options=None,
     ):
         super(BurstyPool, self).__init__(
             labels=labels,
             database_role=database_role,
-            observability_options=observability_options,
         )
         self.target_size = target_size
         self._database = None
@@ -408,12 +403,10 @@ def __init__(
         ping_interval=3000,
         labels=None,
         database_role=None,
-        observability_options=None,
     ):
         super(PingingPool, self).__init__(
             labels=labels,
             database_role=database_role,
-            observability_options=observability_options,
         )
         self.size = size
         self.default_timeout = default_timeout
@@ -567,7 +560,6 @@ def __init__(
         ping_interval=3000,
         labels=None,
         database_role=None,
-        observability_options=None,
     ):
         """This throws a deprecation warning on initialization."""
         warn(
@@ -583,7 +575,6 @@ def __init__(
             ping_interval,
             labels=labels,
             database_role=database_role,
-            observability_options=observability_options,
         )
 
         self.begin_pending_transactions()
diff --git a/google/cloud/spanner_v1/session.py b/google/cloud/spanner_v1/session.py
@@ -63,15 +63,12 @@ class Session(object):
     _session_id = None
     _transaction = None
 
-    def __init__(
-        self, database, labels=None, database_role=None, observability_options=None
-    ):
+    def __init__(self, database, labels=None, database_role=None):
         self._database = database
         if labels is None:
             labels = {}
         self._labels = labels
         self._database_role = database_role
-        self._observability_options = observability_options
 
     def __lt__(self, other):
         return self._session_id < other._session_id
diff --git a/tests/unit/test__opentelemetry_tracing.py b/tests/unit/test__opentelemetry_tracing.py
@@ -35,7 +35,8 @@ def _make_session():
     # of mock._tracer_provider spuriously failing tests, because per
     # unittest.mock.Mock's definition invoking any attribute or method
     # returns another mock.
-    setattr(session, "_observability_options", None)
+    db = session._database
+    setattr(db, "observability_options", None)
     return session
 
 

Original file line number	Diff line number	Diff line change
`@@ -508,7 +508,6 @@ def database(`
`508`	`508`	`database_role=database_role,`
`509`	`509`	`enable_drop_protection=enable_drop_protection,`
`510`	`510`	`proto_descriptors=proto_descriptors,`
`511`		`- observability_options=self._observability_options,`
`512`	`511`	`)`
`513`	`512`	`else:`
`514`	`513`	`return TestDatabase(`