Skip to content

Commit e86b2c4

Browse files
privatwolkeprivatwolke
committed
fixed: asyncpg connection params are a namedtuple
Follow-up on the apparently abbandonned #2114. The asyncpg instrumentation attempts to fall back on using the database name as the span name in case the first argument to the instrumented method is falsey. This has probably never worked since asyncpg defines the `_params` attribute as an instance of `ConnectionParams` (https://github.com/MagicStack/asyncpg/blob/master/asyncpg/connection.py#L62) which is a NamedTuple instance and thus don't define `get`. The proper way of safely accessing properties on a NamedTuple is using `getattr`. The only case that I've actually found which triggers this branch is if the supplied query is an empty string. This is something that causes an `AttributeError` for `Connection.execute` but is fine for `fetch()`, `fetchval()`, `fetchrow()` and `executemany()`. The tests have been expanded to check these cases. Also, more status code validation has been added where it was missing.
1 parent c0bc2c9 commit e86b2c4

File tree

2 files changed

+89
-4
lines changed

2 files changed

+89
-4
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ def _uninstrument(self, **__):
150150

151151
async def _do_execute(self, func, instance, args, kwargs):
152152
exception = None
153-
params = getattr(instance, "_params", {})
154-
name = args[0] if args[0] else params.get("database", "postgresql")
153+
params = getattr(instance, "_params")
154+
name = args[0] if args[0] else getattr(params, "database", "postgresql")
155155

156156
try:
157157
# Strip leading comments so we get the operation name.

‎tests/opentelemetry-docker-tests/tests/asyncpg/test_asyncpg_functional.py‎

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,84 @@ def test_instrumented_execute_method_without_arguments(self, *_, **__):
6767
spans[0].attributes[SpanAttributes.DB_STATEMENT], "SELECT 42;"
6868
)
6969

70+
def test_instrumented_execute_method_error(self, *_, **__):
71+
with self.assertRaises(AttributeError):
72+
async_call(self._connection.execute(""))
73+
spans = self.memory_exporter.get_finished_spans()
74+
self.assertEqual(len(spans), 1)
75+
self.assertIs(StatusCode.ERROR, spans[0].status.status_code)
76+
self.check_span(spans[0])
77+
self.assertEqual(spans[0].name, POSTGRES_DB_NAME)
78+
self.assertEqual(
79+
spans[0].attributes[SpanAttributes.DB_STATEMENT], ""
80+
)
81+
7082
def test_instrumented_fetch_method_without_arguments(self, *_, **__):
7183
async_call(self._connection.fetch("SELECT 42;"))
7284
spans = self.memory_exporter.get_finished_spans()
7385
self.assertEqual(len(spans), 1)
86+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
7487
self.check_span(spans[0])
7588
self.assertEqual(spans[0].name, "SELECT")
7689
self.assertEqual(
7790
spans[0].attributes[SpanAttributes.DB_STATEMENT], "SELECT 42;"
7891
)
7992

93+
def test_instrumented_fetch_method_empty_query(self, *_, **__):
94+
async_call(self._connection.fetch(""))
95+
spans = self.memory_exporter.get_finished_spans()
96+
self.assertEqual(len(spans), 1)
97+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
98+
self.check_span(spans[0])
99+
self.assertEqual(spans[0].name, POSTGRES_DB_NAME)
100+
self.assertEqual(
101+
spans[0].attributes[SpanAttributes.DB_STATEMENT], ""
102+
)
103+
104+
def test_instrumented_fetchval_method_without_arguments(self, *_, **__):
105+
async_call(self._connection.fetchval("SELECT 42;"))
106+
spans = self.memory_exporter.get_finished_spans()
107+
self.assertEqual(len(spans), 1)
108+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
109+
self.check_span(spans[0])
110+
self.assertEqual(spans[0].name, "SELECT")
111+
self.assertEqual(
112+
spans[0].attributes[SpanAttributes.DB_STATEMENT], "SELECT 42;"
113+
)
114+
115+
def test_instrumented_fetchval_method_empty_query(self, *_, **__):
116+
async_call(self._connection.fetchval(""))
117+
spans = self.memory_exporter.get_finished_spans()
118+
self.assertEqual(len(spans), 1)
119+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
120+
self.check_span(spans[0])
121+
self.assertEqual(spans[0].name, POSTGRES_DB_NAME)
122+
self.assertEqual(
123+
spans[0].attributes[SpanAttributes.DB_STATEMENT], ""
124+
)
125+
126+
def test_instrumented_fetchrow_method_without_arguments(self, *_, **__):
127+
async_call(self._connection.fetchval("SELECT 42;"))
128+
spans = self.memory_exporter.get_finished_spans()
129+
self.assertEqual(len(spans), 1)
130+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
131+
self.check_span(spans[0])
132+
self.assertEqual(spans[0].name, "SELECT")
133+
self.assertEqual(
134+
spans[0].attributes[SpanAttributes.DB_STATEMENT], "SELECT 42;"
135+
)
136+
137+
def test_instrumented_fetchrow_method_empty_query(self, *_, **__):
138+
async_call(self._connection.fetchrow(""))
139+
spans = self.memory_exporter.get_finished_spans()
140+
self.assertEqual(len(spans), 1)
141+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
142+
self.check_span(spans[0])
143+
self.assertEqual(spans[0].name, POSTGRES_DB_NAME)
144+
self.assertEqual(
145+
spans[0].attributes[SpanAttributes.DB_STATEMENT], ""
146+
)
147+
80148
def test_instrumented_remove_comments(self, *_, **__):
81149
async_call(self._connection.fetch("/* leading comment */ SELECT 42;"))
82150
async_call(
@@ -88,18 +156,21 @@ def test_instrumented_remove_comments(self, *_, **__):
88156
spans = self.memory_exporter.get_finished_spans()
89157
self.assertEqual(len(spans), 3)
90158
self.check_span(spans[0])
159+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
91160
self.assertEqual(spans[0].name, "SELECT")
92161
self.assertEqual(
93162
spans[0].attributes[SpanAttributes.DB_STATEMENT],
94163
"/* leading comment */ SELECT 42;",
95164
)
96165
self.check_span(spans[1])
166+
self.assertIs(StatusCode.UNSET, spans[1].status.status_code)
97167
self.assertEqual(spans[1].name, "SELECT")
98168
self.assertEqual(
99169
spans[1].attributes[SpanAttributes.DB_STATEMENT],
100170
"/* leading comment */ SELECT 42; /* trailing comment */",
101171
)
102172
self.check_span(spans[2])
173+
self.assertIs(StatusCode.UNSET, spans[2].status.status_code)
103174
self.assertEqual(spans[2].name, "SELECT")
104175
self.assertEqual(
105176
spans[2].attributes[SpanAttributes.DB_STATEMENT],
@@ -245,7 +316,7 @@ def test_instrumented_executemany_method_with_arguments(self, *_, **__):
245316
async_call(self._connection.executemany("SELECT $1;", [["1"], ["2"]]))
246317
spans = self.memory_exporter.get_finished_spans()
247318
self.assertEqual(len(spans), 1)
248-
319+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
249320
self.check_span(spans[0])
250321
self.assertEqual(
251322
spans[0].attributes[SpanAttributes.DB_STATEMENT], "SELECT $1;"
@@ -259,11 +330,25 @@ def test_instrumented_execute_interface_error_method(self, *_, **__):
259330
async_call(self._connection.execute("SELECT 42;", 1, 2, 3))
260331
spans = self.memory_exporter.get_finished_spans()
261332
self.assertEqual(len(spans), 1)
262-
333+
self.assertIs(StatusCode.ERROR, spans[0].status.status_code)
263334
self.check_span(spans[0])
264335
self.assertEqual(
265336
spans[0].attributes[SpanAttributes.DB_STATEMENT], "SELECT 42;"
266337
)
267338
self.assertEqual(
268339
spans[0].attributes["db.statement.parameters"], "(1, 2, 3)"
269340
)
341+
342+
def test_instrumented_executemany_method_empty_query(self, *_, **__):
343+
async_call(self._connection.executemany("", []))
344+
spans = self.memory_exporter.get_finished_spans()
345+
self.assertEqual(len(spans), 1)
346+
self.assertIs(StatusCode.UNSET, spans[0].status.status_code)
347+
self.check_span(spans[0])
348+
self.assertEqual(spans[0].name, POSTGRES_DB_NAME)
349+
self.assertEqual(
350+
spans[0].attributes[SpanAttributes.DB_STATEMENT], ""
351+
)
352+
self.assertEqual(
353+
spans[0].attributes["db.statement.parameters"], "([],)"
354+
)

0 commit comments

Comments
 (0)