Skip to content

Commit a0c3251

Browse files
luke6Lh43xrmxemdneto
authored
fix(pymssql): Complete semconv stability migration for attributes set in wrapped_connection (#4592)
* fix(pymssql): use semconv helpers for host/port/user attributes The pymssql instrumentation's wrapped_connection() was setting net.peer.name, net.peer.port, and db.user using hardcoded legacy attribute keys, bypassing the semconv stability helpers introduced in #4109. Replace hardcoded keys with _set_db_user, _set_http_net_peer_name_client, and _set_http_peer_port_client so that OTEL_SEMCONV_STABILITY_OPT_IN correctly emits stable attributes (server.address, server.port, etc.). Also updates existing test assertions for net.peer.port from string to int, matching the behavior of set_int_attribute used by the helper. * Update instrumentation/opentelemetry-instrumentation-pymssql/tests/test_pymssql_integration.py Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com> * Update instrumentation/opentelemetry-instrumentation-pymssql/tests/test_pymssql_integration.py Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com> * Add changelog fragment for #4592 * Fix linting and add changelog fragment --------- Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com> Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com>
1 parent c3b8e39 commit a0c3251

3 files changed

Lines changed: 105 additions & 6 deletions

File tree

‎.changelog/4592.fixed‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
`opentelemetry-instrumentation-pymssql`: Fix semconv stability migration for connection attributes (host, port, user) set in `wrapped_connection()` to respect `OTEL_SEMCONV_STABILITY_OPT_IN`. Note: `net.peer.port` is now emitted as `int` instead of `string` in default mode, aligning with other DB instrumentations.

‎instrumentation/opentelemetry-instrumentation-pymssql/src/opentelemetry/instrumentation/pymssql/__init__.py‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@
6464
import pymssql
6565

6666
from opentelemetry.instrumentation import dbapi
67+
from opentelemetry.instrumentation._semconv import (
68+
_set_db_user,
69+
_set_http_net_peer_name_client,
70+
_set_http_peer_port_client,
71+
)
6772
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
6873
from opentelemetry.instrumentation.pymssql.package import _instruments
6974
from opentelemetry.instrumentation.pymssql.version import __version__
@@ -104,7 +109,9 @@ def wrapped_connection(
104109

105110
user = kwargs.get("user") or connect_method_args.user
106111
if user is not None:
107-
self.span_attributes["db.user"] = user
112+
_set_db_user(
113+
self.span_attributes, user, self._sem_conv_opt_in_mode_db
114+
)
108115

109116
port = kwargs.get("port") or connect_method_args.port
110117
host = kwargs.get("server") or connect_method_args.server
@@ -120,9 +127,13 @@ def wrapped_connection(
120127
if len(tokens) > 1:
121128
port = tokens[1]
122129
if host is not None:
123-
self.span_attributes["net.peer.name"] = host
130+
_set_http_net_peer_name_client(
131+
self.span_attributes, host, self._sem_conv_opt_in_mode_http
132+
)
124133
if port is not None:
125-
self.span_attributes["net.peer.port"] = port
134+
_set_http_peer_port_client(
135+
self.span_attributes, port, self._sem_conv_opt_in_mode_http
136+
)
126137

127138
charset = kwargs.get("charset") or connect_method_args.charset
128139
if charset is not None:

‎instrumentation/opentelemetry-instrumentation-pymssql/tests/test_pymssql_integration.py‎

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
# Copyright The OpenTelemetry Authors
22
# SPDX-License-Identifier: Apache-2.0
33

4+
import contextlib
5+
from unittest import mock
46
from unittest.mock import Mock, patch
57

68
import pymssql # type: ignore
79

810
import opentelemetry.instrumentation.pymssql
11+
from opentelemetry.instrumentation._semconv import (
12+
OTEL_SEMCONV_STABILITY_OPT_IN,
13+
_OpenTelemetrySemanticConventionStability,
14+
)
915
from opentelemetry.instrumentation.pymssql import PyMSSQLInstrumentor
1016
from opentelemetry.sdk import resources
1117
from opentelemetry.test.test_base import TestBase
@@ -21,6 +27,21 @@ def cursor(self):
2127
return MockConnection()
2228

2329

30+
@contextlib.contextmanager
31+
def use_semconv_opt_in(sem_conv_mode):
32+
env_patch = mock.patch.dict(
33+
"os.environ",
34+
{OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode},
35+
)
36+
_OpenTelemetrySemanticConventionStability._initialized = False
37+
env_patch.start()
38+
try:
39+
yield
40+
finally:
41+
env_patch.stop()
42+
_OpenTelemetrySemanticConventionStability._initialized = False
43+
44+
2445
class TestPyMSSQLIntegration(TestBase):
2546
def tearDown(self):
2647
super().tearDown()
@@ -62,7 +83,7 @@ def test_instrumentor(self):
6283
self.assertEqual(span.attributes["db.statement"], "SELECT * FROM test")
6384
self.assertEqual(span.attributes["db.user"], "dbuser")
6485
self.assertEqual(span.attributes["net.peer.name"], "dbserver.local")
65-
self.assertEqual(span.attributes["net.peer.port"], "1433")
86+
self.assertEqual(span.attributes["net.peer.port"], 1433)
6687
self.assertEqual(span.attributes["db.charset"], "UTF-8")
6788
self.assertEqual(span.attributes["db.protocol.tds.version"], "7.1")
6889

@@ -87,7 +108,7 @@ def test_instrumentor_server_param(self):
87108
span = self._execute_query_and_get_span(cnx)
88109

89110
self.assertEqual(span.attributes["net.peer.name"], "dbserver.local")
90-
self.assertEqual(span.attributes["net.peer.port"], "1433")
111+
self.assertEqual(span.attributes["net.peer.port"], 1433)
91112

92113
@patch("pymssql.connect", new=mock_connect)
93114
# pylint: disable=unused-argument
@@ -101,7 +122,7 @@ def test_instrumentor_port_param(self):
101122
span = self._execute_query_and_get_span(cnx)
102123

103124
self.assertEqual(span.attributes["net.peer.name"], "dbserver.local")
104-
self.assertEqual(span.attributes["net.peer.port"], "1433")
125+
self.assertEqual(span.attributes["net.peer.port"], 1433)
105126

106127
@patch("pymssql.connect", new=mock_connect)
107128
# pylint: disable=unused-argument
@@ -184,3 +205,69 @@ def test_load_entry_point(self):
184205
).load(),
185206
PyMSSQLInstrumentor,
186207
)
208+
209+
@patch("pymssql.connect", new=mock_connect)
210+
def test_semconv_stable(self):
211+
"""database,http opt-in emits only stable attributes."""
212+
with use_semconv_opt_in("database,http"):
213+
PyMSSQLInstrumentor().instrument()
214+
cnx = pymssql.connect( # pylint: disable=no-member
215+
server="dbserver.local",
216+
port="1433",
217+
database="testdb",
218+
user="dbuser",
219+
password="dbpassw0rd",
220+
tds_version="7.1",
221+
)
222+
span = self._execute_query_and_get_span(cnx)
223+
224+
self.assertEqual(span.attributes["db.system.name"], "mssql")
225+
self.assertEqual(span.attributes["db.namespace"], "testdb")
226+
self.assertEqual(
227+
span.attributes["db.query.text"], "SELECT * FROM test"
228+
)
229+
self.assertEqual(
230+
span.attributes["server.address"], "dbserver.local"
231+
)
232+
self.assertEqual(span.attributes["server.port"], 1433)
233+
self.assertEqual(span.attributes["db.protocol.tds.version"], "7.1")
234+
self.assertNotIn("db.system", span.attributes)
235+
self.assertNotIn("db.name", span.attributes)
236+
self.assertNotIn("db.statement", span.attributes)
237+
self.assertNotIn("db.user", span.attributes)
238+
self.assertNotIn("net.peer.name", span.attributes)
239+
self.assertNotIn("net.peer.port", span.attributes)
240+
241+
@patch("pymssql.connect", new=mock_connect)
242+
def test_semconv_dup(self):
243+
"""database/dup,http/dup emits both legacy and stable."""
244+
with use_semconv_opt_in("database/dup,http/dup"):
245+
PyMSSQLInstrumentor().instrument()
246+
cnx = pymssql.connect( # pylint: disable=no-member
247+
server="dbserver.local",
248+
port="1433",
249+
database="testdb",
250+
user="dbuser",
251+
password="dbpassw0rd",
252+
)
253+
span = self._execute_query_and_get_span(cnx)
254+
255+
self.assertEqual(span.attributes["db.system"], "mssql")
256+
self.assertEqual(span.attributes["db.system.name"], "mssql")
257+
self.assertEqual(span.attributes["db.name"], "testdb")
258+
self.assertEqual(span.attributes["db.namespace"], "testdb")
259+
self.assertEqual(
260+
span.attributes["db.statement"], "SELECT * FROM test"
261+
)
262+
self.assertEqual(
263+
span.attributes["db.query.text"], "SELECT * FROM test"
264+
)
265+
self.assertEqual(span.attributes["db.user"], "dbuser")
266+
self.assertEqual(
267+
span.attributes["net.peer.name"], "dbserver.local"
268+
)
269+
self.assertEqual(
270+
span.attributes["server.address"], "dbserver.local"
271+
)
272+
self.assertEqual(span.attributes["net.peer.port"], 1433)
273+
self.assertEqual(span.attributes["server.port"], 1433)

0 commit comments

Comments
 (0)