fix: use standardized OpenTelemetry attributes when tracing

This change modernizes trace span attributes by using OpenTelemetry's semantic conventions that are standardized and allow for much better common ground adoption by broader systems, even more as Google Cloud Tracing & Monitoring pushes towards OpenTelemetry more. With this change we've made the replacement of these fields, directly with imports from `opentelemetry.semconv.trace.SpanAttributes`, as: * "db.type" => DB_SYSTEM aka "db.system" * "db.url" => DB_CONNECTION_STRING aka "db.connection_string" * "db.instance" => DB_NAME aka "db.name" * "net.host.name" => NET_HOST_NAME aka "net.host.name" Fixes googleapis#1170
odeke-em · Jul 25, 2024 · a045aa5 · a045aa5
1 parent fe20d41
commit a045aa5
Show file tree

Hide file tree

Showing 12 changed files with 119 additions and 65 deletions.
diff --git a/docs/opentelemetry-tracing.rst b/docs/opentelemetry-tracing.rst
@@ -8,7 +8,7 @@ To take advantage of these traces, we first need to install OpenTelemetry:
 
 .. code-block:: sh
 
-    pip install opentelemetry-api opentelemetry-sdk opentelemetry-instrumentation
+    pip install opentelemetry-api opentelemetry-sdk opentelemetry-instrumentation opentelemetry-semantic-conventions
 
     # [Optional] Installs the cloud monitoring exporter, however you can use any exporter of your choice
     pip install opentelemetry-exporter-google-cloud

diff --git a/google/cloud/spanner_v1/_opentelemetry_tracing.py b/google/cloud/spanner_v1/_opentelemetry_tracing.py
@@ -22,8 +22,13 @@
 try:
     from opentelemetry import trace
     from opentelemetry.trace.status import Status, StatusCode
+    from opentelemetry.semconv.trace import SpanAttributes
 
     HAS_OPENTELEMETRY_INSTALLED = True
+    DB_SYSTEM = SpanAttributes.DB_SYSTEM
+    DB_NAME = SpanAttributes.DB_NAME
+    DB_CONNECTION_STRING = SpanAttributes.DB_CONNECTION_STRING
+    NET_HOST_NAME = SpanAttributes.NET_HOST_NAME
 except ImportError:
     HAS_OPENTELEMETRY_INSTALLED = False
 
@@ -39,10 +44,10 @@ def trace_call(name, session, extra_attributes=None):
 
     # Set base attributes that we know for every trace created
     attributes = {
-        "db.type": "spanner",
-        "db.url": SpannerClient.DEFAULT_ENDPOINT,
-        "db.instance": session._database.name,
-        "net.host.name": SpannerClient.DEFAULT_ENDPOINT,
+        DB_SYSTEM: "google.cloud.spanner",
+        DB_CONNECTION_STRING: SpannerClient.DEFAULT_ENDPOINT,
+        DB_NAME: session._database.name,
+        NET_HOST_NAME: SpannerClient.DEFAULT_ENDPOINT,
     }
 
     if extra_attributes:

diff --git a/setup.py b/setup.py
@@ -47,9 +47,10 @@
 ]
 extras = {
     "tracing": [
-        "opentelemetry-api >= 1.1.0",
-        "opentelemetry-sdk >= 1.1.0",
-        "opentelemetry-instrumentation >= 0.20b0, < 0.23dev",
+        "opentelemetry-api >= 1.25.0",
+        "opentelemetry-sdk >= 1.25.0",
+        "opentelemetry-instrumentation >= 0.46b0",
+        "opentelemetry-semantic-conventions >= 0.46b0",
     ],
     "libcst": "libcst >= 0.2.5",
 }

diff --git a/testing/constraints-3.7.txt b/testing/constraints-3.7.txt
@@ -10,9 +10,10 @@ grpc-google-iam-v1==0.12.4
 libcst==0.2.5
 proto-plus==1.22.0
 sqlparse==0.4.4
-opentelemetry-api==1.1.0
-opentelemetry-sdk==1.1.0
-opentelemetry-instrumentation==0.20b0
+opentelemetry-api>=1.25.0
+opentelemetry-sdk>=1.25.0
+opentelemetry-instrumentation>=0.46b0
+opentelemetry-semantic-conventions>=0.46b0
 protobuf==3.20.2
 deprecated==1.2.14
 grpc-interceptor==0.15.4
diff --git a/tests/_helpers.py b/tests/_helpers.py
@@ -9,14 +9,24 @@
         InMemorySpanExporter,
     )
     from opentelemetry.trace.status import StatusCode
-
-    trace.set_tracer_provider(TracerProvider())
+    from opentelemetry.semconv.trace import SpanAttributes
 
     HAS_OPENTELEMETRY_INSTALLED = True
-except ImportError:
+
+    DB_SYSTEM = SpanAttributes.DB_SYSTEM
+    DB_NAME = SpanAttributes.DB_NAME
+    DB_CONNECTION_STRING = SpanAttributes.DB_CONNECTION_STRING
+    NET_HOST_NAME = SpanAttributes.NET_HOST_NAME
+
+    trace.set_tracer_provider(TracerProvider())
+except ImportError as e:
     HAS_OPENTELEMETRY_INSTALLED = False
 
     StatusCode = mock.Mock()
+    DB_SYSTEM = "db.system"
+    DB_NAME = "db.name"
+    DB_CONNECTION_STRING = "db.connection_string"
+    NET_HOST_NAME = "net.host.name"
 
 _TEST_OT_EXPORTER = None
 _TEST_OT_PROVIDER_INITIALIZED = False

diff --git a/tests/system/test_session_api.py b/tests/system/test_session_api.py
@@ -341,10 +341,10 @@ def assert_span_attributes(
 
 def _make_attributes(db_instance, **kwargs):
     attributes = {
-        "db.type": "spanner",
-        "db.url": "spanner.googleapis.com",
-        "net.host.name": "spanner.googleapis.com",
-        "db.instance": db_instance,
+        ot_helpers.DB_SYSTEM: "google.cloud.spanner",
+        ot_helpers.DB_CONNECTION_STRING: "spanner.googleapis.com",
+        ot_helpers.DB_NAME: db_instance,
+        ot_helpers.NET_HOST_NAME: "spanner.googleapis.com",
     }
     attributes.update(kwargs)
 

diff --git a/tests/unit/test__opentelemetry_tracing.py b/tests/unit/test__opentelemetry_tracing.py
@@ -12,8 +12,15 @@
 from google.api_core.exceptions import GoogleAPICallError
 from google.cloud.spanner_v1 import _opentelemetry_tracing
 
-from tests._helpers import OpenTelemetryBase, HAS_OPENTELEMETRY_INSTALLED
-
+from tests._helpers import (
+    OpenTelemetryBase,
+    StatusCode,
+    DB_SYSTEM,
+    DB_NAME,
+    DB_CONNECTION_STRING,
+    HAS_OPENTELEMETRY_INSTALLED,
+    NET_HOST_NAME
+)
 
 def _make_rpc_error(error_cls, trailing_metadata=None):
     import grpc
@@ -51,14 +58,14 @@ class TestTracing(OpenTelemetryBase):
         def test_trace_call(self):
             extra_attributes = {
                 "attribute1": "value1",
-                # Since our database is mocked, we have to override the db.instance parameter so it is a string
-                "db.instance": "database_name",
+                # Since our database is mocked, we have to override the DB_NAME parameter so it is a string
+                DB_NAME: "database_name",
             }
 
             expected_attributes = {
-                "db.type": "spanner",
-                "db.url": "spanner.googleapis.com",
-                "net.host.name": "spanner.googleapis.com",
+                DB_SYSTEM: "google.cloud.spanner",
+                DB_CONNECTION_STRING: "spanner.googleapis.com",
+                NET_HOST_NAME: "spanner.googleapis.com",
             }
             expected_attributes.update(extra_attributes)
 
@@ -78,13 +85,14 @@ def test_trace_call(self):
             self.assertEqual(span.status.status_code, StatusCode.OK)
 
         def test_trace_error(self):
-            extra_attributes = {"db.instance": "database_name"}
+            extra_attributes = {DB_NAME: "database_name"}
 
             expected_attributes = {
-                "db.type": "spanner",
-                "db.url": "spanner.googleapis.com",
-                "net.host.name": "spanner.googleapis.com",
+                DB_SYSTEM: "google.cloud.spanner",
+                DB_CONNECTION_STRING: "spanner.googleapis.com",
+                NET_HOST_NAME: "spanner.googleapis.com",
             }
+
             expected_attributes.update(extra_attributes)
 
             with self.assertRaises(GoogleAPICallError):
@@ -104,13 +112,14 @@ def test_trace_error(self):
             self.assertEqual(span.status.status_code, StatusCode.ERROR)
 
         def test_trace_grpc_error(self):
-            extra_attributes = {"db.instance": "database_name"}
+            extra_attributes = {DB_NAME: "database_name"}
 
             expected_attributes = {
-                "db.type": "spanner",
-                "db.url": "spanner.googleapis.com:443",
-                "net.host.name": "spanner.googleapis.com:443",
+                DB_SYSTEM: "google.cloud.spanner",
+                DB_CONNECTION_STRING: "spanner.googleapis.com",
+                NET_HOST_NAME: "spanner.googleapis.com",
             }
+
             expected_attributes.update(extra_attributes)
 
             with self.assertRaises(GoogleAPICallError):
@@ -127,13 +136,14 @@ def test_trace_grpc_error(self):
             self.assertEqual(span.status.status_code, StatusCode.ERROR)
 
         def test_trace_codeless_error(self):
-            extra_attributes = {"db.instance": "database_name"}
+            extra_attributes = {DB_NAME: "database_name"}
 
             expected_attributes = {
-                "db.type": "spanner",
-                "db.url": "spanner.googleapis.com:443",
-                "net.host.name": "spanner.googleapis.com:443",
+                DB_SYSTEM: "google.cloud.spanner",
+                DB_CONNECTION_STRING: "spanner.googleapis.com",
+                NET_HOST_NAME: "spanner.googleapis.com",
             }
+
             expected_attributes.update(extra_attributes)
 
             with self.assertRaises(GoogleAPICallError):

diff --git a/tests/unit/test_batch.py b/tests/unit/test_batch.py
@@ -14,7 +14,14 @@
 
 
 import unittest
-from tests._helpers import OpenTelemetryBase, StatusCode
+from tests._helpers import (
+    OpenTelemetryBase,
+    StatusCode,
+    DB_SYSTEM,
+    DB_NAME,
+    DB_CONNECTION_STRING,
+    NET_HOST_NAME,
+)
 from google.cloud.spanner_v1 import RequestOptions
 
 TABLE_NAME = "citizens"
@@ -24,10 +31,10 @@
     ["[email protected]", "Bharney", "Rhubble", 31],
 ]
 BASE_ATTRIBUTES = {
-    "db.type": "spanner",
-    "db.url": "spanner.googleapis.com",
-    "db.instance": "testing",
-    "net.host.name": "spanner.googleapis.com",
+    DB_SYSTEM: "google.cloud.spanner",
+    DB_CONNECTION_STRING: "spanner.googleapis.com",
+    DB_NAME: "testing",
+    NET_HOST_NAME: "spanner.googleapis.com",
 }
 
 

diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py
@@ -20,6 +20,10 @@
     OpenTelemetryBase,
     StatusCode,
     HAS_OPENTELEMETRY_INSTALLED,
+    DB_SYSTEM,
+    DB_NAME,
+    DB_CONNECTION_STRING,
+    NET_HOST_NAME,
 )
 
 
@@ -46,12 +50,11 @@ class TestSession(OpenTelemetryBase):
     SESSION_NAME = DATABASE_NAME + "/sessions/" + SESSION_ID
     DATABASE_ROLE = "dummy-role"
     BASE_ATTRIBUTES = {
-        "db.type": "spanner",
-        "db.url": "spanner.googleapis.com",
-        "db.instance": DATABASE_NAME,
-        "net.host.name": "spanner.googleapis.com",
+        DB_SYSTEM: "google.cloud.spanner",
+        DB_CONNECTION_STRING: "spanner.googleapis.com",
+        DB_NAME: DATABASE_NAME,
+        NET_HOST_NAME: "spanner.googleapis.com",
     }
-
     def _getTargetClass(self):
         from google.cloud.spanner_v1.session import Session
 

diff --git a/tests/unit/test_snapshot.py b/tests/unit/test_snapshot.py
@@ -21,6 +21,10 @@
     OpenTelemetryBase,
     StatusCode,
     HAS_OPENTELEMETRY_INSTALLED,
+    DB_SYSTEM,
+    DB_NAME,
+    DB_CONNECTION_STRING,
+    NET_HOST_NAME,
 )
 from google.cloud.spanner_v1.param_types import INT64
 from google.api_core.retry import Retry
@@ -41,10 +45,10 @@
 SECONDS = 3
 MICROS = 123456
 BASE_ATTRIBUTES = {
-    "db.type": "spanner",
-    "db.url": "spanner.googleapis.com",
-    "db.instance": "testing",
-    "net.host.name": "spanner.googleapis.com",
+    DB_SYSTEM: "google.cloud.spanner",
+    DB_CONNECTION_STRING: "spanner.googleapis.com",
+    DB_NAME: "testing",
+    NET_HOST_NAME: "spanner.googleapis.com",
 }
 DIRECTED_READ_OPTIONS = {
     "include_replicas": {
@@ -531,10 +535,10 @@ def test_iteration_w_multiple_span_creation(self):
                 self.assertEqual(
                     dict(span.attributes),
                     {
-                        "db.type": "spanner",
-                        "db.url": "spanner.googleapis.com",
-                        "db.instance": "testing",
-                        "net.host.name": "spanner.googleapis.com",
+                        DB_SYSTEM: "google.cloud.spanner",
+                        DB_CONNECTION_STRING: "spanner.googleapis.com",
+                        DB_NAME: "testing",
+                        NET_HOST_NAME: "spanner.googleapis.com",
                     },
                 )
 

diff --git a/tests/unit/test_spanner.py b/tests/unit/test_spanner.py
@@ -45,7 +45,13 @@
 
 from google.api_core import gapic_v1
 
-from tests._helpers import OpenTelemetryBase
+from tests._helpers import (
+    OpenTelemetryBase,
+    DB_SYSTEM,
+    DB_NAME,
+    DB_CONNECTION_STRING,
+    NET_HOST_NAME,
+)
 
 TABLE_NAME = "citizens"
 COLUMNS = ["email", "first_name", "last_name", "age"]
@@ -110,10 +116,10 @@ class TestTransaction(OpenTelemetryBase):
     TRANSACTION_TAG = "transaction-tag"
 
     BASE_ATTRIBUTES = {
-        "db.type": "spanner",
-        "db.url": "spanner.googleapis.com",
-        "db.instance": "testing",
-        "net.host.name": "spanner.googleapis.com",
+        DB_SYSTEM: "google.cloud.spanner",
+        DB_CONNECTION_STRING: "spanner.googleapis.com",
+        DB_NAME: "testing",
+        NET_HOST_NAME: "spanner.googleapis.com",
     }
 
     def _getTargetClass(self):

diff --git a/tests/unit/test_transaction.py b/tests/unit/test_transaction.py
@@ -21,7 +21,14 @@
 from google.api_core.retry import Retry
 from google.api_core import gapic_v1
 
-from tests._helpers import OpenTelemetryBase, StatusCode
+from tests._helpers import (
+    OpenTelemetryBase,
+    StatusCode,
+    DB_SYSTEM,
+    DB_NAME,
+    DB_CONNECTION_STRING,
+    NET_HOST_NAME,
+)
 
 TABLE_NAME = "citizens"
 COLUMNS = ["email", "first_name", "last_name", "age"]
@@ -53,10 +60,10 @@ class TestTransaction(OpenTelemetryBase):
     TRANSACTION_TAG = "transaction-tag"
 
     BASE_ATTRIBUTES = {
-        "db.type": "spanner",
-        "db.url": "spanner.googleapis.com",
-        "db.instance": "testing",
-        "net.host.name": "spanner.googleapis.com",
+        DB_SYSTEM: "google.cloud.spanner",
+        DB_CONNECTION_STRING: "spanner.googleapis.com",
+        DB_NAME: "testing",
+        NET_HOST_NAME: "spanner.googleapis.com",
     }
 
     def _getTargetClass(self):