From 98d0a4adfefe9991d277e7560ca7f1987b9b42c2 Mon Sep 17 00:00:00 2001 From: Radoslav Kirilov Date: Tue, 1 Jul 2025 23:28:29 +0300 Subject: [PATCH 1/6] feat(pymongo): introduce `db.operation`, refactor `db.statement` This change updates the opentelemetry-instrumentation-pymongo to better align with OpenTelemetry semantic conventions and the behavior of other language instrumentations (e.g. NodeJS, Ruby, etc.). see https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts The key changes are: - The `db.operation` attribute is now used to record the MongoDB command name (e.g., find, insert). - The `db.statement` attribute is no longer populated by default. It is only recorded if the `capture_statement = True` option is passed to the instrumentor. - The span name has been made more specific, changing from `{db_name}.{command}` to `{db_name}.{collection}.{command}` when a collection is present. This provides more consistent and conventional telemetry data for MongoDB operations. --- .../instrumentation/pymongo/__init__.py | 33 ++++++++++++------- .../tests/test_pymongo.py | 21 +++++++++--- 2 files changed, 37 insertions(+), 17 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 7ada8d789a..77a881cbca 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -136,8 +136,7 @@ def started(self, event: monitoring.CommandStartedEvent): if not self.is_enabled or not is_instrumentation_enabled(): return command_name = event.command_name - span_name = f"{event.database_name}.{command_name}" - statement = self._get_statement_by_command_name(command_name, event) + span_name = _get_span_name(event) collection = event.command.get(event.command_name) try: @@ -147,7 +146,12 @@ def started(self, event: monitoring.CommandStartedEvent): SpanAttributes.DB_SYSTEM, DbSystemValues.MONGODB.value ) span.set_attribute(SpanAttributes.DB_NAME, event.database_name) - span.set_attribute(SpanAttributes.DB_STATEMENT, statement) + span.set_attribute(SpanAttributes.DB_OPERATION, command_name) + if self.capture_statement: + span.set_attribute( + SpanAttributes.DB_STATEMENT, + _get_statement(event), + ) if collection: span.set_attribute( SpanAttributes.DB_MONGODB_COLLECTION, collection @@ -210,16 +214,21 @@ def failed(self, event: monitoring.CommandFailedEvent): def _pop_span(self, event: CommandEvent) -> Span | None: return self._span_dict.pop(_get_span_dict_key(event), None) - def _get_statement_by_command_name( - self, command_name: str, event: CommandEvent - ) -> str: - 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_name(event: CommandEvent) -> str: + """Get the span name for a given pymongo event.""" + command_name = event.command_name + collection = event.command.get(command_name) + if collection: + return f"{event.database_name}.{collection}.{command_name}" + return f"{event.database_name}.{command_name}" + +def _get_statement(event: CommandEvent) -> str: + """Get the statement for a given pymongo event.""" + command_name = event.command_name + command_attribute = COMMAND_TO_ATTRIBUTE_MAPPING.get(command_name) + command = event.command.get(command_attribute) + return f"{command}" def _get_span_dict_key( event: CommandEvent, diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py index 8b082a4a14..089ce49707 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py @@ -62,7 +62,8 @@ 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_OPERATION], "find") + self.assertNotIn(SpanAttributes.DB_STATEMENT, span.attributes) self.assertEqual( span.attributes[SpanAttributes.NET_PEER_NAME], "test.com" ) @@ -210,7 +211,10 @@ def test_capture_statement_getmore(self): self.assertEqual( span.attributes[SpanAttributes.DB_STATEMENT], - "getMore test_collection", + "test_collection", + ) + self.assertEqual( + span.attributes[SpanAttributes.DB_OPERATION], "getMore" ) def test_capture_statement_aggregate(self): @@ -232,10 +236,13 @@ def test_capture_statement_aggregate(self): self.assertEqual(len(spans_list), 1) span = spans_list[0] - expected_statement = f"aggregate {pipeline}" + expected_statement = f"{pipeline}" self.assertEqual( span.attributes[SpanAttributes.DB_STATEMENT], expected_statement ) + self.assertEqual( + span.attributes[SpanAttributes.DB_OPERATION], "aggregate" + ) def test_capture_statement_disabled_getmore(self): command_attrs = { @@ -253,9 +260,11 @@ def test_capture_statement_disabled_getmore(self): span = spans_list[0] self.assertEqual( - span.attributes[SpanAttributes.DB_STATEMENT], "getMore" + span.attributes[SpanAttributes.DB_OPERATION], "getMore" ) + self.assertNotIn(SpanAttributes.DB_STATEMENT, span.attributes) + def test_capture_statement_disabled_aggregate(self): pipeline = [{"$match": {"status": "active"}}] command_attrs = { @@ -273,9 +282,11 @@ def test_capture_statement_disabled_aggregate(self): span = spans_list[0] self.assertEqual( - span.attributes[SpanAttributes.DB_STATEMENT], "aggregate" + span.attributes[SpanAttributes.DB_OPERATION], "aggregate" ) + self.assertNotIn(SpanAttributes.DB_STATEMENT, span.attributes) + class MockCommand: def __init__(self, command_attrs): From d0fea95eb265c988aba312c89fa83b5c22a49cc8 Mon Sep 17 00:00:00 2001 From: Radoslav Kirilov Date: Wed, 2 Jul 2025 11:09:52 +0300 Subject: [PATCH 2/6] chore: update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1acd65a2b..85d9917481 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-pymongo` `aggregate` and `getMore` capture statements support ([#3601](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3601)) +### Breaking changes + +- `opentelemetry-instrumentation-pymongo` introduce `db.operation`, refactor `db.statement`, refactor span name + ([#3606](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3606)) + ## Version 1.34.0/0.55b0 (2025-06-04) ### Fixed From 907b13b856654b292586daa2f66d6e2d4e7c5d03 Mon Sep 17 00:00:00 2001 From: Radoslav Kirilov Date: Wed, 2 Jul 2025 11:12:40 +0300 Subject: [PATCH 3/6] chore: ruff fixes --- .../src/opentelemetry/instrumentation/pymongo/__init__.py | 2 ++ 1 file changed, 2 insertions(+) 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 77a881cbca..08f1ef817d 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -223,6 +223,7 @@ def _get_span_name(event: CommandEvent) -> str: return f"{event.database_name}.{collection}.{command_name}" return f"{event.database_name}.{command_name}" + def _get_statement(event: CommandEvent) -> str: """Get the statement for a given pymongo event.""" command_name = event.command_name @@ -230,6 +231,7 @@ def _get_statement(event: CommandEvent) -> str: command = event.command.get(command_attribute) return f"{command}" + def _get_span_dict_key( event: CommandEvent, ) -> int | tuple[int, tuple[str, int | None]]: From 487e811e3eff4de8430dcb6dc71814df083ddc93 Mon Sep 17 00:00:00 2001 From: Radoslav Kirilov Date: Wed, 2 Jul 2025 11:20:01 +0300 Subject: [PATCH 4/6] chore: fix docker-tests --- .../tests/pymongo/test_pymongo_functional.py | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) 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 c506d0452a..0979c0cbf0 100644 --- a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -45,7 +45,9 @@ def tearDown(self): self.instrumentor.uninstrument() super().tearDown() - def validate_spans(self, expected_db_statement): + def validate_spans( + self, expected_db_operation, expected_db_statement=None + ): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) for span in spans: @@ -74,7 +76,11 @@ def validate_spans(self, expected_db_statement): MONGODB_COLLECTION_NAME, ) self.assertEqual( - pymongo_span.attributes[SpanAttributes.DB_STATEMENT], + pymongo_span.attributes[SpanAttributes.DB_OPERATION], + expected_db_operation, + ) + self.assertEqual( + pymongo_span.attributes.get(SpanAttributes.DB_STATEMENT, None), expected_db_statement, ) @@ -86,11 +92,12 @@ def test_insert(self): ) insert_result_id = insert_result.inserted_id + expected_db_operation = "insert" expected_db_statement = ( - f"insert [{{'name': 'testName', 'value': 'testValue', '_id': " + f"[{{'name': 'testName', 'value': 'testValue', '_id': " f"ObjectId('{insert_result_id}')}}]" ) - self.validate_spans(expected_db_statement) + self.validate_spans(expected_db_operation, expected_db_statement) def test_update(self): """Should create a child span for update""" @@ -99,29 +106,32 @@ def test_update(self): {"name": "testName"}, {"$set": {"value": "someOtherValue"}} ) + expected_db_operation = "update" expected_db_statement = ( - "update [SON([('q', {'name': 'testName'}), ('u', " + "[SON([('q', {'name': 'testName'}), ('u', " "{'$set': {'value': 'someOtherValue'}}), ('multi', False), ('upsert', False)])]" ) - self.validate_spans(expected_db_statement) + self.validate_spans(expected_db_operation, 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({"name": "testName"}) - expected_db_statement = "find {'name': 'testName'}" - self.validate_spans(expected_db_statement) + expected_db_operation = "find" + expected_db_statement = "{'name': 'testName'}" + self.validate_spans(expected_db_operation, 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"}) + expected_db_operation = "delete" expected_db_statement = ( - "delete [SON([('q', {'name': 'testName'}), ('limit', 1)])]" + "[SON([('q', {'name': 'testName'}), ('limit', 1)])]" ) - self.validate_spans(expected_db_statement) + self.validate_spans(expected_db_operation, expected_db_statement) def test_find_without_capture_statement(self): """Should create a child span for find""" @@ -130,8 +140,9 @@ def test_find_without_capture_statement(self): with self._tracer.start_as_current_span("rootSpan"): self._collection.find_one({"name": "testName"}) - expected_db_statement = "find" - self.validate_spans(expected_db_statement) + expected_db_operation = "find" + expected_db_statement = None + self.validate_spans(expected_db_operation, expected_db_statement) def test_uninstrument(self): # check that integration is working From 498a8ac9da5f16bc4f8a708019edeebf82891389 Mon Sep 17 00:00:00 2001 From: Radoslav Kirilov Date: Wed, 2 Jul 2025 14:33:41 +0300 Subject: [PATCH 5/6] fix: Invalid type in attribute sequence when using sessions fixes https://github.com/open-telemetry/opentelemetry-python-contrib/issues/1918 closes https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2010 --- .../instrumentation/pymongo/__init__.py | 4 +- .../tests/test_pymongo.py | 27 ++++++++++++ .../tests/pymongo/test_pymongo_functional.py | 41 +++++++++++++++++++ 3 files changed, 70 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 08f1ef817d..8805bf3d61 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -152,7 +152,7 @@ def started(self, event: monitoring.CommandStartedEvent): SpanAttributes.DB_STATEMENT, _get_statement(event), ) - if collection: + if collection and isinstance(collection, str): span.set_attribute( SpanAttributes.DB_MONGODB_COLLECTION, collection ) @@ -219,7 +219,7 @@ def _get_span_name(event: CommandEvent) -> str: """Get the span name for a given pymongo event.""" command_name = event.command_name collection = event.command.get(command_name) - if collection: + if collection and isinstance(collection, str): return f"{event.database_name}.{collection}.{command_name}" return f"{event.database_name}.{command_name}" diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py index 089ce49707..3a6b7e40b9 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py @@ -287,6 +287,33 @@ def test_capture_statement_disabled_aggregate(self): self.assertNotIn(SpanAttributes.DB_STATEMENT, span.attributes) + def test_endsessions_command_with_dict_list_collection(self): + # Test for https://github.com/open-telemetry/opentelemetry-python-contrib/issues/1918 + # endSessions command has a list of dictionaries as collection value + command_attrs = { + "command_name": "endSessions", + "endSessions": [ + {"id": {"id": "session1"}}, + {"id": {"id": "session2"}}, + ], + } + command_tracer = CommandTracer(self.tracer) + mock_event = MockEvent(command_attrs) + command_tracer.started(event=mock_event) + command_tracer.succeeded(event=mock_event) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + + # Should not have DB_MONGODB_COLLECTION attribute since collection is not a string + self.assertNotIn(SpanAttributes.DB_MONGODB_COLLECTION, span.attributes) + self.assertEqual( + span.attributes[SpanAttributes.DB_OPERATION], "endSessions" + ) + # Span name should not include collection name + self.assertEqual(span.name, "database_name.endSessions") + class MockCommand: def __init__(self, command_attrs): 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 0979c0cbf0..99815e9b03 100644 --- a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -163,3 +163,44 @@ def test_uninstrument(self): self._collection.find_one() spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) + + def test_session_end_no_error(self): + """Test that endSessions doesn't cause instrumentation errors (issue #1918)""" + client = MongoClient( + MONGODB_HOST, MONGODB_PORT, serverSelectionTimeoutMS=2000 + ) + + with self._tracer.start_as_current_span("rootSpan"): + session = client.start_session() + db = client[MONGODB_DB_NAME] + collection = db[MONGODB_COLLECTION_NAME] + # Do a simple operation within the session + collection.find_one({"test": "123"}) + # End the session - this should not cause an error + session.end_session() + + # Verify spans were created without errors + spans = self.memory_exporter.get_finished_spans() + # Should have at least the find and endSessions operations + self.assertGreaterEqual(len(spans), 2) + + session_end_spans = [ + s + for s in spans + if s.attributes.get(SpanAttributes.DB_OPERATION) == "endSessions" + ] + if session_end_spans: + span = session_end_spans[0] + # Should not have DB_MONGODB_COLLECTION attribute since endSessions collection is not a string + self.assertNotIn( + SpanAttributes.DB_MONGODB_COLLECTION, span.attributes + ) + # Should have other expected attributes + self.assertEqual( + span.attributes[SpanAttributes.DB_OPERATION], "endSessions" + ) + self.assertEqual( + span.attributes[SpanAttributes.DB_NAME], MONGODB_DB_NAME + ) + + client.close() From adfcfcab375e80d7570eefb987dbf0e214ef58bf Mon Sep 17 00:00:00 2001 From: Radoslav Kirilov Date: Wed, 2 Jul 2025 14:53:47 +0300 Subject: [PATCH 6/6] fix: handle None for `db.statement` --- .../instrumentation/pymongo/__init__.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 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 8805bf3d61..e7bf641b3f 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -148,10 +148,12 @@ def started(self, event: monitoring.CommandStartedEvent): span.set_attribute(SpanAttributes.DB_NAME, event.database_name) span.set_attribute(SpanAttributes.DB_OPERATION, command_name) if self.capture_statement: - span.set_attribute( - SpanAttributes.DB_STATEMENT, - _get_statement(event), - ) + db_statement = _get_statement(event) + if db_statement is not None: + span.set_attribute( + SpanAttributes.DB_STATEMENT, + _get_statement(event), + ) if collection and isinstance(collection, str): span.set_attribute( SpanAttributes.DB_MONGODB_COLLECTION, collection @@ -224,12 +226,14 @@ def _get_span_name(event: CommandEvent) -> str: return f"{event.database_name}.{command_name}" -def _get_statement(event: CommandEvent) -> str: +def _get_statement(event: CommandEvent) -> str | None: """Get the statement for a given pymongo event.""" command_name = event.command_name command_attribute = COMMAND_TO_ATTRIBUTE_MAPPING.get(command_name) command = event.command.get(command_attribute) - return f"{command}" + if command is not None: + return f"{command}" + return None def _get_span_dict_key(