opensource/opentelemetry-python-contrib
1
0
Fork 0
mirror of https://github.com/open-telemetry/opentelemetry-python-contrib.git synced 2025-07-30 05:32:30 +08:00
Code Issues Packages Projects Releases Wiki Activity

SQLCommenter semicolon bug fix (#1200)

Browse Source
This commit is contained in:
Thiyagu55
2022-07-22 07:52:40 +05:30
committed by GitHub
parent d75194aed7
commit 7c75b3867b
11 changed files with 92 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.12.0rc2-0.32b0...HEAD)
- Adding multiple db connections support for django-instrumentation's sqlcommenter
([#1187](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1187))
- SQLCommenter semicolon bug fix
([#1200](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1200/files))
### Added
- `opentelemetry-instrumentation-redis` add support to instrument RedisCluster clients

26
instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py
View File

@ -46,12 +46,12 @@ import wrapt
from opentelemetry import trace as trace_api
from opentelemetry.instrumentation.dbapi.version import __version__
from opentelemetry.instrumentation.utils import (
_generate_opentelemetry_traceparent,
_generate_sql_comment,
_add_sql_comment,
_get_opentelemetry_values,
unwrap,
)
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.trace import Span, SpanKind, TracerProvider, get_tracer
from opentelemetry.trace import SpanKind, TracerProvider, get_tracer
_logger = logging.getLogger(__name__)
@ -375,15 +375,6 @@ class CursorTracer:
return statement.decode("utf8", "replace")
return statement
@staticmethod
def _generate_comment(span: Span) -> str:
span_context = span.get_span_context()
meta = {}
if span_context.is_valid:
meta.update(_generate_opentelemetry_traceparent(span))
# TODO(schekuri): revisit to enrich with info such as route, db_driver etc...
return _generate_sql_comment(**meta)
def traced_execution(
self,
cursor,
@ -405,11 +396,14 @@ class CursorTracer:
self._populate_span(span, cursor, *args)
if args and self._commenter_enabled:
try:
comment = self._generate_comment(span)
if isinstance(args[0], bytes):
comment = comment.encode("utf8")
args_list = list(args)
args_list[0] += comment
commenter_data = {}
commenter_data.update(_get_opentelemetry_values())
statement = _add_sql_comment(
args_list[0], **commenter_data
)
args_list[0] = statement
args = tuple(args_list)
except Exception as exc: # pylint: disable=broad-except
_logger.exception(

11
instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py
View File

@ -236,12 +236,11 @@ class TestDBApiIntegration(TestBase):
mock_connect, {}, {}
)
cursor = mock_connection.cursor()
cursor.executemany("Test query")
spans_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans_list), 1)
span = spans_list[0]
comment = dbapi.CursorTracer._generate_comment(span)
self.assertIn(comment, cursor.query)
cursor.executemany("Select 1;")
self.assertRegex(
cursor.query,
r"Select 1 /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;",
)
def test_callproc(self):
db_integration = dbapi.DatabaseApiIntegration(

6
instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/sqlcommenter_middleware.py
View File

@ -23,7 +23,7 @@ from django.db import connections
from django.db.backends.utils import CursorDebugWrapper
from opentelemetry.instrumentation.utils import (
_generate_sql_comment,
_add_sql_comment,
_get_opentelemetry_values,
)
from opentelemetry.trace.propagation.tracecontext import (
@ -84,7 +84,8 @@ class _QueryWrapper:
db_driver = context["connection"].settings_dict.get("ENGINE", "")
resolver_match = self.request.resolver_match
sql_comment = _generate_sql_comment(
sql = _add_sql_comment(
sql,
# Information about the controller.
controller=resolver_match.view_name
if resolver_match and with_controller
@ -112,7 +113,6 @@ class _QueryWrapper:
# See:
# * https://github.com/basecamp/marginalia/issues/61
# * https://github.com/basecamp/marginalia/pull/80
sql += sql_comment
# Add the query to the query log if debugging.
if isinstance(context["cursor"], CursorDebugWrapper):

4
instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py
View File

@ -88,7 +88,7 @@ class TestMiddleware(WsgiTestBase):
execute_mock_obj = MagicMock()
qw_instance(
execute_mock_obj,
"Select 1",
"Select 1;",
MagicMock("test"),
MagicMock("test1"),
MagicMock(),
@ -97,7 +97,7 @@ class TestMiddleware(WsgiTestBase):
self.assertEqual(
output_sql,
"Select 1 /*app_name='app',controller='view',route='route',traceparent='%%2Atraceparent%%3D%%2700-0000000"
"00000000000000000deadbeef-000000000000beef-00'*/",
"00000000000000000deadbeef-000000000000beef-00'*/;",
)
@patch(

19
instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py
View File

@ -21,11 +21,10 @@ from opentelemetry.instrumentation.sqlalchemy.package import (
)
from opentelemetry.instrumentation.sqlalchemy.version import __version__
from opentelemetry.instrumentation.utils import (
_generate_opentelemetry_traceparent,
_generate_sql_comment,
_add_sql_comment,
_get_opentelemetry_values,
)
from opentelemetry.semconv.trace import NetTransportValues, SpanAttributes
from opentelemetry.trace import Span
from opentelemetry.trace.status import Status, StatusCode
@ -141,21 +140,15 @@ class EngineTracer:
span.set_attribute(SpanAttributes.DB_SYSTEM, self.vendor)
for key, value in attrs.items():
span.set_attribute(key, value)
if self.enable_commenter:
commenter_data = {}
commenter_data.update(_get_opentelemetry_values())
statement = _add_sql_comment(statement, **commenter_data)
context._otel_span = span
if self.enable_commenter:
statement = statement + EngineTracer._generate_comment(span=span)
return statement, params
@staticmethod
def _generate_comment(span: Span) -> str:
span_context = span.get_span_context()
meta = {}
if span_context.is_valid:
meta.update(_generate_opentelemetry_traceparent(span))
return _generate_sql_comment(**meta)
# pylint: disable=unused-argument
def _after_cur_exec(conn, cursor, statement, params, context, executemany):

21
instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py
View File

@ -12,7 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import asyncio
import logging
from unittest import mock
import pytest
@ -21,7 +20,6 @@ from sqlalchemy import create_engine
from opentelemetry import trace
from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor
from opentelemetry.instrumentation.sqlalchemy.engine import EngineTracer
from opentelemetry.sdk.resources import Resource
from opentelemetry.sdk.trace import TracerProvider, export
from opentelemetry.test.test_base import TestBase
@ -217,22 +215,3 @@ class TestSqlalchemyInstrumentation(TestBase):
)
asyncio.get_event_loop().run_until_complete(run())
def test_generate_commenter(self):
logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO)
engine = create_engine("sqlite:///:memory:")
SQLAlchemyInstrumentor().instrument(
engine=engine,
tracer_provider=self.tracer_provider,
enable_commenter=True,
)
cnx = engine.connect()
cnx.execute("SELECT 1 + 1;").fetchall()
spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 2)
span = spans[1]
self.assertIn(
EngineTracer._generate_comment(span),
self.caplog.records[-2].getMessage(),
)

24
instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py
View File

@ -11,6 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
import pytest
from sqlalchemy import create_engine
@ -28,6 +29,17 @@ class TestSqlalchemyInstrumentationWithSQLCommenter(TestBase):
super().tearDown()
SQLAlchemyInstrumentor().uninstrument()
def test_sqlcommenter_disabled(self):
logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO)
engine = create_engine("sqlite:///:memory:", echo=True)
SQLAlchemyInstrumentor().instrument(
engine=engine, tracer_provider=self.tracer_provider
)
cnx = engine.connect()
cnx.execute("SELECT 1;").fetchall()
self.assertEqual(self.caplog.records[-2].getMessage(), "SELECT 1;")
def test_sqlcommenter_enabled(self):
engine = create_engine("sqlite:///:memory:")
SQLAlchemyInstrumentor().instrument(
@ -39,15 +51,5 @@ class TestSqlalchemyInstrumentationWithSQLCommenter(TestBase):
cnx.execute("SELECT 1;").fetchall()
self.assertRegex(
self.caplog.records[-2].getMessage(),
r"SELECT 1; /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/",
r"SELECT 1 /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;",
)
def test_sqlcommenter_disabled(self):
engine = create_engine("sqlite:///:memory:", echo=True)
SQLAlchemyInstrumentor().instrument(
engine=engine, tracer_provider=self.tracer_provider
)
cnx = engine.connect()
cnx.execute("SELECT 1;").fetchall()
self.assertEqual(self.caplog.records[-2].getMessage(), "SELECT 1;")

30
opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py
View File

@ -24,7 +24,7 @@ from opentelemetry import context, trace
# pylint: disable=E0611
from opentelemetry.context import _SUPPRESS_INSTRUMENTATION_KEY # noqa: F401
from opentelemetry.propagate import extract
from opentelemetry.trace import Span, StatusCode
from opentelemetry.trace import StatusCode
from opentelemetry.trace.propagation.tracecontext import (
TraceContextTextMapPropagator,
)
@ -147,7 +147,7 @@ def _generate_sql_comment(**meta) -> str:
)
def _url_quote(s): # pylint: disable=invalid-name
def _url_quote(s) -> str: # pylint: disable=invalid-name
if not isinstance(s, (str, bytes)):
return s
quoted = urllib.parse.quote(s)
@ -158,7 +158,7 @@ def _url_quote(s): # pylint: disable=invalid-name
return quoted.replace("%", "%%")
def _get_opentelemetry_values():
def _get_opentelemetry_values() -> dict:
"""
Return the OpenTelemetry Trace and Span IDs if Span ID is set in the
OpenTelemetry execution context.
@ -169,20 +169,22 @@ def _get_opentelemetry_values():
return _headers
def _generate_opentelemetry_traceparent(span: Span) -> str:
meta = {}
_version = "00"
_span_id = trace.format_span_id(span.context.span_id)
_trace_id = trace.format_trace_id(span.context.trace_id)
_flags = str(trace.TraceFlags.SAMPLED)
_traceparent = _version + "-" + _trace_id + "-" + _span_id + "-" + _flags
meta.update({"traceparent": _traceparent})
return meta
def _python_path_without_directory(python_path, directory, path_separator):
return sub(
rf"{escape(directory)}{path_separator}(?!$)",
"",
python_path,
)
def _add_sql_comment(sql, **meta) -> str:
"""
Appends comments to the sql statement and returns it
"""
comment = _generate_sql_comment(**meta)
sql = sql.rstrip()
if sql[-1] == ";":
sql = sql[:-1] + comment + ";"
else:
sql = sql + comment
return sql

34
opentelemetry-instrumentation/tests/test_utils.py
View File

@ -16,6 +16,7 @@ import unittest
from http import HTTPStatus
from opentelemetry.instrumentation.utils import (
_add_sql_comment,
_python_path_without_directory,
http_status_to_status_code,
)
@ -152,3 +153,36 @@ class TestUtils(unittest.TestCase):
python_path, directory, path_separator
)
self.assertEqual(actual_python_path, python_path)
def test_add_sql_comments_with_semicolon(self):
sql_query_without_semicolon = "Select 1;"
comments = {"comment_1": "value 1", "comment 2": "value 3"}
commented_sql_without_semicolon = _add_sql_comment(
sql_query_without_semicolon, **comments
)
self.assertEqual(
commented_sql_without_semicolon,
"Select 1 /*comment%%202='value%%203',comment_1='value%%201'*/;",
)
def test_add_sql_comments_without_semicolon(self):
sql_query_without_semicolon = "Select 1"
comments = {"comment_1": "value 1", "comment 2": "value 3"}
commented_sql_without_semicolon = _add_sql_comment(
sql_query_without_semicolon, **comments
)
self.assertEqual(
commented_sql_without_semicolon,
"Select 1 /*comment%%202='value%%203',comment_1='value%%201'*/",
)
def test_add_sql_comments_without_comments(self):
sql_query_without_semicolon = "Select 1"
comments = {}
commented_sql_without_semicolon = _add_sql_comment(
sql_query_without_semicolon, **comments
)
self.assertEqual(commented_sql_without_semicolon, "Select 1")

2
tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_sqlcommenter.py
View File

@ -50,5 +50,5 @@ class TestFunctionalPsycopg(TestBase):
self._cursor.execute("SELECT 1;")
self.assertRegex(
self._cursor.query.decode("ascii"),
r"SELECT 1; /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/",
r"SELECT 1 /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;",
)