From b74b98d163110c68c910b3cd67fd3cedd1ea77e9 Mon Sep 17 00:00:00 2001 From: avzis Date: Thu, 24 Nov 2022 13:39:46 +0200 Subject: [PATCH 01/11] save 'documents' property instead of command name add a boolean to decide whether to capture the information or hide it --- .../src/opentelemetry/instrumentation/pymongo/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index 91cd81aab3..11cf3ae74f 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -106,6 +106,7 @@ def __init__( request_hook: RequestHookT = dummy_callback, response_hook: ResponseHookT = dummy_callback, failed_hook: FailedHookT = dummy_callback, + capture_parameters: bool = False, ): self._tracer = tracer self._span_dict = {} @@ -113,6 +114,7 @@ def __init__( self.start_hook = request_hook self.success_hook = response_hook self.failed_hook = failed_hook + self.capture_parameters = capture_parameters def started(self, event: monitoring.CommandStartedEvent): """Method to handle a pymongo CommandStartedEvent""" @@ -120,11 +122,11 @@ def started(self, event: monitoring.CommandStartedEvent): _SUPPRESS_INSTRUMENTATION_KEY ): return - command = event.command.get(event.command_name, "") + command = event.command.get('documents', "") name = event.database_name name += "." + event.command_name statement = event.command_name - if command: + if command and self.capture_parameters: statement += " " + str(command) try: @@ -223,6 +225,7 @@ def _instrument(self, **kwargs): request_hook = kwargs.get("request_hook", dummy_callback) response_hook = kwargs.get("response_hook", dummy_callback) failed_hook = kwargs.get("failed_hook", dummy_callback) + capture_parameters = kwargs.get("capture_parameters") # Create and register a CommandTracer only the first time if self._commandtracer_instance is None: tracer = get_tracer(__name__, __version__, tracer_provider) @@ -232,6 +235,7 @@ def _instrument(self, **kwargs): request_hook=request_hook, response_hook=response_hook, failed_hook=failed_hook, + capture_parameters=capture_parameters, ) monitoring.register(self._commandtracer_instance) # If already created, just enable it From 5fb073bb996a87dff2447cc1af008b8f130fc10a Mon Sep 17 00:00:00 2001 From: avzis Date: Tue, 20 Dec 2022 17:36:43 +0200 Subject: [PATCH 02/11] capture statement --- .../instrumentation/pymongo/__init__.py | 30 ++++++++---- .../tests/test_pymongo.py | 48 +++++++------------ .../tests/pymongo/test_pymongo_functional.py | 39 ++++++++++++--- 3 files changed, 70 insertions(+), 47 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index adacc33f8a..f264ed43b8 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -46,6 +46,7 @@ failed_hook (Callable) - a function with extra user-defined logic to be performed after the query returns with a failed response this function signature is: def failed_hook(span: Span, event: CommandFailedEvent) -> None +capture_statement (bool) - an optional value to enable capturing the database statement that is being executed for example: @@ -106,7 +107,7 @@ def __init__( request_hook: RequestHookT = dummy_callback, response_hook: ResponseHookT = dummy_callback, failed_hook: FailedHookT = dummy_callback, - capture_parameters: bool = False, + capture_statement: bool = False, ): self._tracer = tracer self._span_dict = {} @@ -114,7 +115,7 @@ def __init__( self.start_hook = request_hook self.success_hook = response_hook self.failed_hook = failed_hook - self.capture_parameters = capture_parameters + self.capture_statement = capture_statement def started(self, event: monitoring.CommandStartedEvent): """Method to handle a pymongo CommandStartedEvent""" @@ -122,11 +123,11 @@ def started(self, event: monitoring.CommandStartedEvent): _SUPPRESS_INSTRUMENTATION_KEY ): return - command = event.command.get('documents', "") - name = event.database_name - name += "." + event.command_name - statement = event.command_name - if command and self.capture_parameters: + command_name = event.command_name + name = event.database_name + "." + command_name + statement = command_name + command = _get_command_by_command_name(command_name, event) + if command and self.capture_statement: statement += " " + str(command) try: @@ -194,6 +195,17 @@ def _pop_span(self, event): return self._span_dict.pop(_get_span_dict_key(event), None) +def _get_command_by_command_name(command_name, event): + mapping = { + 'insert': 'documents', + 'delete': 'deletes', + 'update': 'updates', + 'find': 'filter' + } + + return event.command.get(mapping.get(command_name), "") + + def _get_span_dict_key(event): if event.connection_id is not None: return event.request_id, event.connection_id @@ -225,7 +237,7 @@ def _instrument(self, **kwargs): request_hook = kwargs.get("request_hook", dummy_callback) response_hook = kwargs.get("response_hook", dummy_callback) failed_hook = kwargs.get("failed_hook", dummy_callback) - capture_parameters = kwargs.get("capture_parameters") + capture_statement = kwargs.get("capture_statement") # Create and register a CommandTracer only the first time if self._commandtracer_instance is None: tracer = get_tracer(__name__, __version__, tracer_provider) @@ -235,7 +247,7 @@ def _instrument(self, **kwargs): request_hook=request_hook, response_hook=response_hook, failed_hook=failed_hook, - capture_parameters=capture_parameters, + capture_statement=capture_statement, ) monitoring.register(self._commandtracer_instance) # If already created, just enable it diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py index 15a5a0be0b..8a7985d817 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py @@ -43,14 +43,14 @@ def test_pymongo_instrumentor(self): self.assertTrue(mock_register.called) def test_started(self): - command_attrs = { - "command_name": "find", + command = { + "filter": "123", } command_tracer = CommandTracer( self.tracer, request_hook=self.start_callback ) mock_event = MockEvent( - command_attrs, ("test.com", "1234"), "test_request_id" + command, "find", connection_id=("test.com", "1234"), request_id="test_request_id" ) command_tracer.started(event=mock_event) # the memory exporter can't be used here because the span isn't ended @@ -58,14 +58,11 @@ def test_started(self): # pylint: disable=protected-access span = command_tracer._pop_span(mock_event) self.assertIs(span.kind, trace_api.SpanKind.CLIENT) - self.assertEqual(span.name, "database_name.command_name") + self.assertEqual(span.name, "database_name.find") self.assertEqual(span.attributes[SpanAttributes.DB_SYSTEM], "mongodb") self.assertEqual( span.attributes[SpanAttributes.DB_NAME], "database_name" ) - self.assertEqual( - span.attributes[SpanAttributes.DB_STATEMENT], "command_name find" - ) self.assertEqual( span.attributes[SpanAttributes.NET_PEER_NAME], "test.com" ) @@ -109,9 +106,6 @@ def test_suppression_key(self): mock_span.is_recording.return_value = True mock_tracer.start_span.return_value = mock_span mock_event = MockEvent({}) - mock_event.command.get = mock.Mock() - mock_event.command.get.return_value = "dummy" - token = context.attach( context.set_value(_SUPPRESS_INSTRUMENTATION_KEY, True) ) @@ -123,10 +117,10 @@ def test_suppression_key(self): finally: context.detach(token) - # if suppression key is set, CommandTracer methods return immediately, so command.get is not invoked. - self.assertFalse( - mock_event.command.get.called # pylint: disable=no-member - ) + # if suppression key is set, CommandTracer methods return immediately, so span.set_attribute + # and span.set_status are not invoked. + self.assertFalse(mock_span.set_attribute.called) + self.assertFalse(mock_span.set_status.called) def test_failed(self): mock_event = MockEvent({}) @@ -152,8 +146,8 @@ def test_failed(self): self.failed_callback.assert_called_once() def test_multiple_commands(self): - first_mock_event = MockEvent({}, ("firstUrl", "123"), "first") - second_mock_event = MockEvent({}, ("secondUrl", "456"), "second") + first_mock_event = MockEvent({}, connection_id=("firstUrl", "123"), request_id="first") + second_mock_event = MockEvent({}, connection_id=("secondUrl", "456"), request_id="second") command_tracer = CommandTracer(self.tracer) command_tracer.started(event=first_mock_event) command_tracer.started(event=second_mock_event) @@ -175,10 +169,10 @@ def test_multiple_commands(self): ) def test_int_command(self): - command_attrs = { - "command_name": 123, + command = { + "filter": 123, } - mock_event = MockEvent(command_attrs) + mock_event = MockEvent(command, "find") command_tracer = CommandTracer(self.tracer) command_tracer.started(event=mock_event) @@ -188,20 +182,12 @@ def test_int_command(self): self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual(span.name, "database_name.command_name") - - -class MockCommand: - def __init__(self, command_attrs): - self.command_attrs = command_attrs - - def get(self, key, default=""): - return self.command_attrs.get(key, default) - + self.assertEqual(span.name, "database_name.find") class MockEvent: - def __init__(self, command_attrs, connection_id=None, request_id=""): - self.command = MockCommand(command_attrs) + def __init__(self, command, command_name="", connection_id=None, request_id=""): + self.command = command + self.command_name = command_name self.connection_id = connection_id self.request_id = request_id diff --git a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py index 92c694953b..085a1393ad 100644 --- a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -34,6 +34,7 @@ def setUp(self): self.instrumentor = PymongoInstrumentor() self.instrumentor.instrument() self.instrumentor._commandtracer_instance._tracer = self._tracer + self.instrumentor._commandtracer_instance.capture_statement = True client = MongoClient( MONGODB_HOST, MONGODB_PORT, serverSelectionTimeoutMS=2000 ) @@ -44,7 +45,7 @@ def tearDown(self): self.instrumentor.uninstrument() super().tearDown() - def validate_spans(self): + def validate_spans(self, expected_db_statement): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) for span in spans: @@ -68,14 +69,21 @@ def validate_spans(self): self.assertEqual( pymongo_span.attributes[SpanAttributes.NET_PEER_PORT], MONGODB_PORT ) + self.assertEqual( + pymongo_span.attributes[SpanAttributes.DB_STATEMENT], expected_db_statement + ) def test_insert(self): """Should create a child span for insert""" with self._tracer.start_as_current_span("rootSpan"): - self._collection.insert_one( + insert_result = self._collection.insert_one( {"name": "testName", "value": "testValue"} ) - self.validate_spans() + insert_result_id = insert_result.inserted_id + + expected_db_statement = f"insert [{{'name': 'testName', 'value': 'testValue', '_id': " \ + f"ObjectId('{insert_result_id}')}}]" + self.validate_spans(expected_db_statement) def test_update(self): """Should create a child span for update""" @@ -83,19 +91,36 @@ def test_update(self): self._collection.update_one( {"name": "testName"}, {"$set": {"value": "someOtherValue"}} ) - self.validate_spans() + + expected_db_statement = "update [SON([('q', {'name': 'testName'}), ('u', " \ + "{'$set': {'value': 'someOtherValue'}}), ('multi', False), ('upsert', False)])]" + self.validate_spans(expected_db_statement) def test_find(self): """Should create a child span for find""" with self._tracer.start_as_current_span("rootSpan"): - self._collection.find_one() - self.validate_spans() + self._collection.find_one({"name": "testName"}) + + expected_db_statement = "find {'name': 'testName'}" + self.validate_spans(expected_db_statement) def test_delete(self): """Should create a child span for delete""" with self._tracer.start_as_current_span("rootSpan"): self._collection.delete_one({"name": "testName"}) - self.validate_spans() + + expected_db_statement = "delete [SON([('q', {'name': 'testName'}), ('limit', 1)])]" + self.validate_spans(expected_db_statement) + + def test_find_without_capture_statement(self): + """Should create a child span for find""" + self.instrumentor._commandtracer_instance.capture_statement = False + + with self._tracer.start_as_current_span("rootSpan"): + self._collection.find_one({"name": "testName"}) + + expected_db_statement = "find" + self.validate_spans(expected_db_statement) def test_uninstrument(self): # check that integration is working From 232b58d3b35eab982f54cff5b2e5f0cd48399099 Mon Sep 17 00:00:00 2001 From: avzis Date: Wed, 21 Dec 2022 10:31:51 +0200 Subject: [PATCH 03/11] fix lint --- .../instrumentation/pymongo/__init__.py | 12 ++++++------ .../tests/test_pymongo.py | 17 +++++++++++++---- .../tests/pymongo/test_pymongo_functional.py | 16 +++++++++++----- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index f264ed43b8..96bc608087 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -196,14 +196,14 @@ def _pop_span(self, event): def _get_command_by_command_name(command_name, event): - mapping = { - 'insert': 'documents', - 'delete': 'deletes', - 'update': 'updates', - 'find': 'filter' + command_mapping = { + "insert": "documents", + "delete": "deletes", + "update": "updates", + "find": "filter" } - return event.command.get(mapping.get(command_name), "") + return event.command.get(command_mapping.get(command_name), "") def _get_span_dict_key(event): diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py index 8a7985d817..16add34bf2 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py @@ -50,7 +50,10 @@ def test_started(self): self.tracer, request_hook=self.start_callback ) mock_event = MockEvent( - command, "find", connection_id=("test.com", "1234"), request_id="test_request_id" + command, + "find", + connection_id=("test.com", "1234"), + request_id="test_request_id" ) command_tracer.started(event=mock_event) # the memory exporter can't be used here because the span isn't ended @@ -146,8 +149,12 @@ def test_failed(self): self.failed_callback.assert_called_once() def test_multiple_commands(self): - first_mock_event = MockEvent({}, connection_id=("firstUrl", "123"), request_id="first") - second_mock_event = MockEvent({}, connection_id=("secondUrl", "456"), request_id="second") + first_mock_event = MockEvent( + {}, connection_id=("firstUrl", "123"), request_id="first" + ) + second_mock_event = MockEvent( + {}, connection_id=("secondUrl", "456"), request_id="second" + ) command_tracer = CommandTracer(self.tracer) command_tracer.started(event=first_mock_event) command_tracer.started(event=second_mock_event) @@ -185,7 +192,9 @@ def test_int_command(self): self.assertEqual(span.name, "database_name.find") class MockEvent: - def __init__(self, command, command_name="", connection_id=None, request_id=""): + def __init__( + self, command, command_name="", connection_id=None, request_id="" + ): self.command = command self.command_name = command_name self.connection_id = connection_id diff --git a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py index 085a1393ad..889a1ff22d 100644 --- a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -81,8 +81,10 @@ def test_insert(self): ) insert_result_id = insert_result.inserted_id - expected_db_statement = f"insert [{{'name': 'testName', 'value': 'testValue', '_id': " \ - f"ObjectId('{insert_result_id}')}}]" + expected_db_statement = ( + f"insert [{{'name': 'testName', 'value': 'testValue', '_id': " + f"ObjectId('{insert_result_id}')}}]" + ) self.validate_spans(expected_db_statement) def test_update(self): @@ -92,8 +94,10 @@ def test_update(self): {"name": "testName"}, {"$set": {"value": "someOtherValue"}} ) - expected_db_statement = "update [SON([('q', {'name': 'testName'}), ('u', " \ - "{'$set': {'value': 'someOtherValue'}}), ('multi', False), ('upsert', False)])]" + expected_db_statement = ( + "update [SON([('q', {'name': 'testName'}), ('u', " + "{'$set': {'value': 'someOtherValue'}}), ('multi', False), ('upsert', False)])]" + ) self.validate_spans(expected_db_statement) def test_find(self): @@ -109,7 +113,9 @@ def test_delete(self): with self._tracer.start_as_current_span("rootSpan"): self._collection.delete_one({"name": "testName"}) - expected_db_statement = "delete [SON([('q', {'name': 'testName'}), ('limit', 1)])]" + expected_db_statement = ( + "delete [SON([('q', {'name': 'testName'}), ('limit', 1)])]" + ) self.validate_spans(expected_db_statement) def test_find_without_capture_statement(self): From c7535ce0c4b6f63338c886f5a714ef89ba1666a3 Mon Sep 17 00:00:00 2001 From: avzis Date: Wed, 21 Dec 2022 11:00:07 +0200 Subject: [PATCH 04/11] refactor and changelog --- CHANGELOG.md | 2 ++ .../src/opentelemetry/instrumentation/pymongo/__init__.py | 2 +- .../tests/test_pymongo.py | 7 +++++-- .../tests/pymongo/test_pymongo_functional.py | 3 ++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bd207b86c..6b5fdbdef2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-aws-lambda` Adds an option to configure `disable_aws_context_propagation` by environment variable: `OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION` ([#1507](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1507)) +- mongo db - fix db statement capturing + ([#1512](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1512)) ## Version 1.15.0/0.36b0 (2022-12-10) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index 96bc608087..24478de923 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -200,7 +200,7 @@ def _get_command_by_command_name(command_name, event): "insert": "documents", "delete": "deletes", "update": "updates", - "find": "filter" + "find": "filter", } return event.command.get(command_mapping.get(command_name), "") diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py index 16add34bf2..b1a51595d1 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py @@ -53,7 +53,7 @@ def test_started(self): command, "find", connection_id=("test.com", "1234"), - request_id="test_request_id" + request_id="test_request_id", ) command_tracer.started(event=mock_event) # the memory exporter can't be used here because the span isn't ended @@ -66,6 +66,9 @@ def test_started(self): self.assertEqual( span.attributes[SpanAttributes.DB_NAME], "database_name" ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], "find" + ) self.assertEqual( span.attributes[SpanAttributes.NET_PEER_NAME], "test.com" ) @@ -193,7 +196,7 @@ def test_int_command(self): class MockEvent: def __init__( - self, command, command_name="", connection_id=None, request_id="" + self, command, command_name="", connection_id=None, request_id="" ): self.command = command self.command_name = command_name diff --git a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py index 889a1ff22d..7df5c1bf16 100644 --- a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -70,7 +70,8 @@ def validate_spans(self, expected_db_statement): pymongo_span.attributes[SpanAttributes.NET_PEER_PORT], MONGODB_PORT ) self.assertEqual( - pymongo_span.attributes[SpanAttributes.DB_STATEMENT], expected_db_statement + pymongo_span.attributes[SpanAttributes.DB_STATEMENT], + expected_db_statement ) def test_insert(self): From 141b2a06fe41926f478031f734fef46efe95eefb Mon Sep 17 00:00:00 2001 From: avzis Date: Wed, 21 Dec 2022 16:33:11 +0200 Subject: [PATCH 05/11] refactor --- .../instrumentation/pymongo/__init__.py | 27 ++++++++----------- .../instrumentation/pymongo/utils.py | 20 ++++++++++++++ .../tests/test_pymongo.py | 4 +-- .../tests/pymongo/test_pymongo_functional.py | 2 +- 4 files changed, 33 insertions(+), 20 deletions(-) create mode 100644 instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/utils.py diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index 24478de923..1959a6df9a 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -83,6 +83,7 @@ def failed_hook(span, event): from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.pymongo.package import _instruments from opentelemetry.instrumentation.pymongo.version import __version__ +from opentelemetry.instrumentation.pymongo.utils import COMMAND_TO_ATTRIBUTE_MAPPING from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY from opentelemetry.semconv.trace import DbSystemValues, SpanAttributes from opentelemetry.trace import SpanKind, get_tracer @@ -124,14 +125,11 @@ def started(self, event: monitoring.CommandStartedEvent): ): return command_name = event.command_name - name = event.database_name + "." + command_name - statement = command_name - command = _get_command_by_command_name(command_name, event) - if command and self.capture_statement: - statement += " " + str(command) + span_name = event.database_name + "." + command_name + statement = self._get_statement_by_command_name(command_name) try: - span = self._tracer.start_span(name, kind=SpanKind.CLIENT) + span = self._tracer.start_span(span_name, kind=SpanKind.CLIENT) if span.is_recording(): span.set_attribute( SpanAttributes.DB_SYSTEM, DbSystemValues.MONGODB.value @@ -194,16 +192,13 @@ def failed(self, event: monitoring.CommandFailedEvent): def _pop_span(self, event): return self._span_dict.pop(_get_span_dict_key(event), None) - -def _get_command_by_command_name(command_name, event): - command_mapping = { - "insert": "documents", - "delete": "deletes", - "update": "updates", - "find": "filter", - } - - return event.command.get(command_mapping.get(command_name), "") + def _get_statement_by_command_name(self, command_name, event): + statement = command_name + command_attribute = COMMAND_TO_ATTRIBUTE_MAPPING.get(command_name) + command = event.command.get(command_attribute) + if command and self.capture_statement: + statement += " " + str(command) + return statement def _get_span_dict_key(event): diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/utils.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/utils.py new file mode 100644 index 0000000000..9b3b3c2bec --- /dev/null +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/utils.py @@ -0,0 +1,20 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +COMMAND_TO_ATTRIBUTE_MAPPING = { + "insert": "documents", + "delete": "deletes", + "update": "updates", + "find": "filter", +} \ No newline at end of file diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py index b1a51595d1..3c32906629 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py @@ -66,9 +66,7 @@ def test_started(self): self.assertEqual( span.attributes[SpanAttributes.DB_NAME], "database_name" ) - self.assertEqual( - span.attributes[SpanAttributes.DB_STATEMENT], "find" - ) + self.assertEqual(span.attributes[SpanAttributes.DB_STATEMENT], "find") self.assertEqual( span.attributes[SpanAttributes.NET_PEER_NAME], "test.com" ) diff --git a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py index 7df5c1bf16..9a6492556e 100644 --- a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -71,7 +71,7 @@ def validate_spans(self, expected_db_statement): ) self.assertEqual( pymongo_span.attributes[SpanAttributes.DB_STATEMENT], - expected_db_statement + expected_db_statement, ) def test_insert(self): From 270d0619d4596360b39e64c48430ba346f92b453 Mon Sep 17 00:00:00 2001 From: avzis Date: Wed, 21 Dec 2022 18:17:05 +0200 Subject: [PATCH 06/11] revert changes in basic instrumentation tests --- .../tests/test_pymongo.py | 58 ++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py index 3c32906629..6981e33c25 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py @@ -43,17 +43,14 @@ def test_pymongo_instrumentor(self): self.assertTrue(mock_register.called) def test_started(self): - command = { - "filter": "123", + command_attrs = { + "command_name": "find", } command_tracer = CommandTracer( self.tracer, request_hook=self.start_callback ) mock_event = MockEvent( - command, - "find", - connection_id=("test.com", "1234"), - request_id="test_request_id", + command_attrs, ("test.com", "1234"), "test_request_id" ) command_tracer.started(event=mock_event) # the memory exporter can't be used here because the span isn't ended @@ -61,12 +58,14 @@ def test_started(self): # pylint: disable=protected-access span = command_tracer._pop_span(mock_event) self.assertIs(span.kind, trace_api.SpanKind.CLIENT) - self.assertEqual(span.name, "database_name.find") + self.assertEqual(span.name, "database_name.command_name") self.assertEqual(span.attributes[SpanAttributes.DB_SYSTEM], "mongodb") self.assertEqual( span.attributes[SpanAttributes.DB_NAME], "database_name" ) - self.assertEqual(span.attributes[SpanAttributes.DB_STATEMENT], "find") + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], "command_name" + ) self.assertEqual( span.attributes[SpanAttributes.NET_PEER_NAME], "test.com" ) @@ -110,6 +109,9 @@ def test_suppression_key(self): mock_span.is_recording.return_value = True mock_tracer.start_span.return_value = mock_span mock_event = MockEvent({}) + mock_event.command.get = mock.Mock() + mock_event.command.get.return_value = "dummy" + token = context.attach( context.set_value(_SUPPRESS_INSTRUMENTATION_KEY, True) ) @@ -121,10 +123,10 @@ def test_suppression_key(self): finally: context.detach(token) - # if suppression key is set, CommandTracer methods return immediately, so span.set_attribute - # and span.set_status are not invoked. - self.assertFalse(mock_span.set_attribute.called) - self.assertFalse(mock_span.set_status.called) + # if suppression key is set, CommandTracer methods return immediately, so command.get is not invoked. + self.assertFalse( + mock_event.command.get.called # pylint: disable=no-member + ) def test_failed(self): mock_event = MockEvent({}) @@ -150,12 +152,8 @@ def test_failed(self): self.failed_callback.assert_called_once() def test_multiple_commands(self): - first_mock_event = MockEvent( - {}, connection_id=("firstUrl", "123"), request_id="first" - ) - second_mock_event = MockEvent( - {}, connection_id=("secondUrl", "456"), request_id="second" - ) + first_mock_event = MockEvent({}, ("firstUrl", "123"), "first") + second_mock_event = MockEvent({}, ("secondUrl", "456"), "second") command_tracer = CommandTracer(self.tracer) command_tracer.started(event=first_mock_event) command_tracer.started(event=second_mock_event) @@ -177,10 +175,10 @@ def test_multiple_commands(self): ) def test_int_command(self): - command = { - "filter": 123, + command_attrs = { + "command_name": 123, } - mock_event = MockEvent(command, "find") + mock_event = MockEvent(command_attrs) command_tracer = CommandTracer(self.tracer) command_tracer.started(event=mock_event) @@ -190,14 +188,20 @@ def test_int_command(self): self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual(span.name, "database_name.find") + self.assertEqual(span.name, "database_name.command_name") + + +class MockCommand: + def __init__(self, command_attrs): + self.command_attrs = command_attrs + + def get(self, key, default=""): + return self.command_attrs.get(key, default) + class MockEvent: - def __init__( - self, command, command_name="", connection_id=None, request_id="" - ): - self.command = command - self.command_name = command_name + def __init__(self, command_attrs, connection_id=None, request_id=""): + self.command = MockCommand(command_attrs) self.connection_id = connection_id self.request_id = request_id From 680c9854bda207d168b0a95a150ce470bf401a0e Mon Sep 17 00:00:00 2001 From: avzis Date: Wed, 21 Dec 2022 18:45:24 +0200 Subject: [PATCH 07/11] fix function calling --- .../src/opentelemetry/instrumentation/pymongo/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index 1959a6df9a..b391897084 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -126,7 +126,7 @@ def started(self, event: monitoring.CommandStartedEvent): return command_name = event.command_name span_name = event.database_name + "." + command_name - statement = self._get_statement_by_command_name(command_name) + statement = self._get_statement_by_command_name(command_name, event) try: span = self._tracer.start_span(span_name, kind=SpanKind.CLIENT) From a3d9670e40ac6b4f6cef3c8137a58b3cfa217987 Mon Sep 17 00:00:00 2001 From: avzis Date: Wed, 21 Dec 2022 19:41:22 +0200 Subject: [PATCH 08/11] lint --- .../src/opentelemetry/instrumentation/pymongo/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index b391897084..db7859d0a9 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -83,7 +83,9 @@ def failed_hook(span, event): from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.pymongo.package import _instruments from opentelemetry.instrumentation.pymongo.version import __version__ -from opentelemetry.instrumentation.pymongo.utils import COMMAND_TO_ATTRIBUTE_MAPPING +from opentelemetry.instrumentation.pymongo.utils import ( + COMMAND_TO_ATTRIBUTE_MAPPING, +) from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY from opentelemetry.semconv.trace import DbSystemValues, SpanAttributes from opentelemetry.trace import SpanKind, get_tracer From 7c0e76fc8473d9a4238a2a332c2a72b051115e84 Mon Sep 17 00:00:00 2001 From: avzis Date: Wed, 21 Dec 2022 20:00:04 +0200 Subject: [PATCH 09/11] lint --- .../src/opentelemetry/instrumentation/pymongo/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/utils.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/utils.py index 9b3b3c2bec..47f5653f0e 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/utils.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/utils.py @@ -17,4 +17,4 @@ "delete": "deletes", "update": "updates", "find": "filter", -} \ No newline at end of file +} From 2d2b2dc349f2209b6a5619ff5dcc04e2f41c56d9 Mon Sep 17 00:00:00 2001 From: avzis Date: Thu, 22 Dec 2022 01:20:32 +0200 Subject: [PATCH 10/11] lint --- .../src/opentelemetry/instrumentation/pymongo/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index db7859d0a9..de026639ed 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -82,10 +82,10 @@ def failed_hook(span, event): from opentelemetry import context from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.pymongo.package import _instruments -from opentelemetry.instrumentation.pymongo.version import __version__ from opentelemetry.instrumentation.pymongo.utils import ( COMMAND_TO_ATTRIBUTE_MAPPING, ) +from opentelemetry.instrumentation.pymongo.version import __version__ from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY from opentelemetry.semconv.trace import DbSystemValues, SpanAttributes from opentelemetry.trace import SpanKind, get_tracer From f6ac26c6d728f931eeacb07dd0e1b3eee91ab439 Mon Sep 17 00:00:00 2001 From: avzis Date: Thu, 12 Jan 2023 19:08:26 +0200 Subject: [PATCH 11/11] refactor string --- .../src/opentelemetry/instrumentation/pymongo/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py index de026639ed..6161dd411e 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -127,7 +127,7 @@ def started(self, event: monitoring.CommandStartedEvent): ): return command_name = event.command_name - span_name = event.database_name + "." + command_name + span_name = f"{event.database_name}.{command_name}" statement = self._get_statement_by_command_name(command_name, event) try: