From d51636e2c0391394f168b65254828d9d442abbce Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sun, 22 Nov 2020 01:21:36 +0530 Subject: [PATCH 1/3] Update pymongo instrumentation to follow semantic conventions --- .../instrumentation/pymongo/__init__.py | 24 ++----------- .../tests/test_pymongo.py | 34 +++---------------- .../tests/pymongo/test_pymongo_functional.py | 8 ++--- 3 files changed, 11 insertions(+), 55 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 434984ec30..abcbe0d633 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py @@ -46,7 +46,6 @@ from opentelemetry.trace.status import Status, StatusCode DATABASE_TYPE = "mongodb" -COMMAND_ATTRIBUTES = ["filter", "sort", "skip", "limit", "pipeline"] class CommandTracer(monitoring.CommandListener): @@ -60,7 +59,7 @@ def started(self, event: monitoring.CommandStartedEvent): if not self.is_enabled: return command = event.command.get(event.command_name, "") - name = DATABASE_TYPE + "." + event.command_name + name = event.command_name statement = event.command_name if command: name += "." + str(command) @@ -69,23 +68,13 @@ def started(self, event: monitoring.CommandStartedEvent): try: span = self._tracer.start_span(name, kind=SpanKind.CLIENT) if span.is_recording(): - span.set_attribute("component", DATABASE_TYPE) - span.set_attribute("db.type", DATABASE_TYPE) - span.set_attribute("db.instance", event.database_name) + span.set_attribute("db.system", DATABASE_TYPE) + span.set_attribute("db.name", event.database_name) span.set_attribute("db.statement", statement) if event.connection_id is not None: span.set_attribute("net.peer.name", event.connection_id[0]) span.set_attribute("net.peer.port", event.connection_id[1]) - # pymongo specific, not specified by spec - span.set_attribute("db.mongo.operation_id", event.operation_id) - span.set_attribute("db.mongo.request_id", event.request_id) - - for attr in COMMAND_ATTRIBUTES: - _attr = event.command.get(attr) - if _attr is not None: - span.set_attribute("db.mongo." + attr, str(_attr)) - # Add Span to dictionary self._span_dict[_get_span_dict_key(event)] = span except Exception as ex: # noqa pylint: disable=broad-except @@ -101,10 +90,6 @@ def succeeded(self, event: monitoring.CommandSucceededEvent): span = self._pop_span(event) if span is None: return - if span.is_recording(): - span.set_attribute( - "db.mongo.duration_micros", event.duration_micros - ) span.end() def failed(self, event: monitoring.CommandFailedEvent): @@ -115,9 +100,6 @@ def failed(self, event: monitoring.CommandFailedEvent): if span is None: return if span.is_recording(): - span.set_attribute( - "db.mongo.duration_micros", event.duration_micros - ) span.set_status(Status(StatusCode.ERROR, event.failure)) span.end() diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py index a3bb7b2223..bfd3d8f52f 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py +++ b/instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py @@ -39,10 +39,6 @@ def test_pymongo_instrumentor(self): def test_started(self): command_attrs = { - "filter": "filter", - "sort": "sort", - "limit": "limit", - "pipeline": "pipeline", "command_name": "find", } command_tracer = CommandTracer(self.tracer) @@ -55,24 +51,12 @@ 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, "mongodb.command_name.find") - self.assertEqual(span.attributes["component"], "mongodb") - self.assertEqual(span.attributes["db.type"], "mongodb") - self.assertEqual(span.attributes["db.instance"], "database_name") + self.assertEqual(span.name, "command_name.find") + self.assertEqual(span.attributes["db.system"], "mongodb") + self.assertEqual(span.attributes["db.name"], "database_name") self.assertEqual(span.attributes["db.statement"], "command_name find") self.assertEqual(span.attributes["net.peer.name"], "test.com") self.assertEqual(span.attributes["net.peer.port"], "1234") - self.assertEqual( - span.attributes["db.mongo.operation_id"], "operation_id" - ) - self.assertEqual( - span.attributes["db.mongo.request_id"], "test_request_id" - ) - - self.assertEqual(span.attributes["db.mongo.filter"], "filter") - self.assertEqual(span.attributes["db.mongo.sort"], "sort") - self.assertEqual(span.attributes["db.mongo.limit"], "limit") - self.assertEqual(span.attributes["db.mongo.pipeline"], "pipeline") def test_succeeded(self): mock_event = MockEvent({}) @@ -82,9 +66,6 @@ def test_succeeded(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual( - span.attributes["db.mongo.duration_micros"], "duration_micros" - ) self.assertIs( span.status.status_code, trace_api.status.StatusCode.UNSET ) @@ -116,9 +97,6 @@ def test_failed(self): self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual( - span.attributes["db.mongo.duration_micros"], "duration_micros" - ) self.assertIs( span.status.status_code, trace_api.status.StatusCode.ERROR, ) @@ -139,13 +117,9 @@ def test_multiple_commands(self): first_span = spans_list[0] second_span = spans_list[1] - self.assertEqual(first_span.attributes["db.mongo.request_id"], "first") self.assertIs( first_span.status.status_code, trace_api.status.StatusCode.UNSET, ) - self.assertEqual( - second_span.attributes["db.mongo.request_id"], "second" - ) self.assertIs( second_span.status.status_code, trace_api.status.StatusCode.ERROR, ) @@ -165,7 +139,7 @@ def test_int_command(self): self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual(span.name, "mongodb.command_name.123") + self.assertEqual(span.name, "command_name.123") class MockCommand: 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 577477a2ab..149c78905a 100644 --- a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -20,9 +20,9 @@ from opentelemetry.instrumentation.pymongo import PymongoInstrumentor from opentelemetry.test.test_base import TestBase -MONGODB_HOST = os.getenv("MONGODB_HOST ", "localhost") -MONGODB_PORT = int(os.getenv("MONGODB_PORT ", "27017")) -MONGODB_DB_NAME = os.getenv("MONGODB_DB_NAME ", "opentelemetry-tests") +MONGODB_HOST = os.getenv("MONGODB_HOST", "localhost") +MONGODB_PORT = int(os.getenv("MONGODB_PORT", "27017")) +MONGODB_DB_NAME = os.getenv("MONGODB_DB_NAME", "opentelemetry-tests") MONGODB_COLLECTION_NAME = "test" @@ -54,7 +54,7 @@ def validate_spans(self): self.assertIs(pymongo_span.parent, root_span.get_span_context()) self.assertIs(pymongo_span.kind, trace_api.SpanKind.CLIENT) self.assertEqual( - pymongo_span.attributes["db.instance"], MONGODB_DB_NAME + pymongo_span.attributes["db.name"], MONGODB_DB_NAME ) self.assertEqual( pymongo_span.attributes["net.peer.name"], MONGODB_HOST From 963f65cb33f175d0820fcd08433e12bdfe6fe294 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sun, 22 Nov 2020 01:27:52 +0530 Subject: [PATCH 2/3] Fix lint --- .../tests/pymongo/test_pymongo_functional.py | 4 +--- 1 file changed, 1 insertion(+), 3 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 149c78905a..d662014f1b 100644 --- a/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -53,9 +53,7 @@ def validate_spans(self): self.assertIsNotNone(pymongo_span.parent) self.assertIs(pymongo_span.parent, root_span.get_span_context()) self.assertIs(pymongo_span.kind, trace_api.SpanKind.CLIENT) - self.assertEqual( - pymongo_span.attributes["db.name"], MONGODB_DB_NAME - ) + self.assertEqual(pymongo_span.attributes["db.name"], MONGODB_DB_NAME) self.assertEqual( pymongo_span.attributes["net.peer.name"], MONGODB_HOST ) From 0b09f53aa104c40962107af0715c8aff872d56d7 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 23 Nov 2020 19:50:11 +0530 Subject: [PATCH 3/3] Add CHANGELOG entry --- .../opentelemetry-instrumentation-pymongo/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-pymongo/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-pymongo/CHANGELOG.md index 30e36e0048..05716f473b 100644 --- a/instrumentation/opentelemetry-instrumentation-pymongo/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-pymongo/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Update pymongo instrumentation to follow semantic conventions +([#203](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/203)) + ## Version 0.14b0 Released 2020-10-13