From ed365cb8cf89bffbad2031c334706d5f1636bc3b Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Wed, 19 Jul 2023 22:45:30 +0200 Subject: [PATCH 1/5] Initial GCP firestore instrumentation commit. --- newrelic/config.py | 17 +++++++++++++++++ tests/testing_support/db_settings.py | 20 ++++++++++++++++++++ tox.ini | 4 ++++ 3 files changed, 41 insertions(+) diff --git a/newrelic/config.py b/newrelic/config.py index 1692601747..1424616aa2 100644 --- a/newrelic/config.py +++ b/newrelic/config.py @@ -2269,6 +2269,23 @@ def _process_module_builtin_defaults(): "instrument_graphql_validate", ) + _process_module_definition( + "google.cloud.firestore_v1.document", + "newrelic.hooks.database_firestore", + "instrument_google_cloud_firestore_v1_document", + ) + + _process_module_definition( + "google.cloud.firestore_v1.collection", + "newrelic.hooks.database_firestore", + "instrument_google_cloud_firestore_v1_collection", + ) + + _process_module_definition( + "google.cloud.firestore_v1.base_client", + "newrelic.hooks.database_firestore", + "instrument_google_cloud_firestore_v1_base_client", + ) _process_module_definition( "ariadne.asgi", "newrelic.hooks.framework_ariadne", diff --git a/tests/testing_support/db_settings.py b/tests/testing_support/db_settings.py index ef9a3419c1..6bb9b30628 100644 --- a/tests/testing_support/db_settings.py +++ b/tests/testing_support/db_settings.py @@ -165,6 +165,26 @@ def mongodb_settings(): return settings +def firestore_settings(): + """Return a list of dict of settings for connecting to mongodb. + + Will return the correct settings, depending on which of the environments it + is running in. It attempts to set variables in the following order, where + later environments override earlier ones. + + 1. Local + 2. Github Actions + """ + + host = "host.docker.internal" if "GITHUB_ACTIONS" in os.environ else "127.0.0.1" + instances = 2 + settings = [ + {"host": host, "port": 8080 + instance_num, "collection": "firestore_collection_" + str(os.getpid())} + for instance_num in range(instances) + ] + return settings + + def elasticsearch_settings(): """Return a list of dict of settings for connecting to elasticsearch. diff --git a/tox.ini b/tox.ini index 32ab90995a..5c0496f69c 100644 --- a/tox.ini +++ b/tox.ini @@ -83,6 +83,7 @@ envlist = memcached-datastore_memcache-{py27,py37,py38,py39,py310,py311,pypy27,pypy38}-memcached01, mysql-datastore_mysql-mysql080023-py27, mysql-datastore_mysql-mysqllatest-{py37,py38,py39,py310,py311}, + firestore-datastore_firestore-{py37,py38,py39,py310,py311,pypy38}, postgres-datastore_postgresql-{py37,py38,py39}, postgres-datastore_psycopg2-{py27,py37,py38,py39,py310,py311}-psycopg2latest postgres-datastore_psycopg2cffi-{py27,pypy27,py37,py38,py39,py310,py311}-psycopg2cffilatest, @@ -170,6 +171,7 @@ envlist = python-template_jinja2-{py37,py311}-jinja2latest python-template_mako-{py27,py37,py310,py311} + [testenv] deps = # Base Dependencies @@ -238,6 +240,7 @@ deps = datastore_elasticsearch: requests datastore_elasticsearch-elasticsearch07: elasticsearch<8.0 datastore_elasticsearch-elasticsearch08: elasticsearch<9.0 + datastore_firestore: firebase-admin datastore_memcache-memcached01: python-memcached<2 datastore_mysql-mysqllatest: mysql-connector-python datastore_mysql-mysql080023: mysql-connector-python<8.0.24 @@ -445,6 +448,7 @@ changedir = datastore_asyncpg: tests/datastore_asyncpg datastore_bmemcached: tests/datastore_bmemcached datastore_elasticsearch: tests/datastore_elasticsearch + datastore_firestore: tests/datastore_firestore datastore_memcache: tests/datastore_memcache datastore_mysql: tests/datastore_mysql datastore_postgresql: tests/datastore_postgresql From 961325afc6ec8e3cb4bee7a049bdaf487152a1cf Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Fri, 21 Jul 2023 16:10:18 +0200 Subject: [PATCH 2/5] Add testing for documents and collections + test generators Co-authored-by: Tim Pansino Co-authored-by: Lalleh Rafeei Co-authored-by: Hannah Stepanek --- newrelic/config.py | 12 ++ newrelic/hooks/datastore_firestore.py | 116 ++++++++++++++++++ tests/datastore_firestore/conftest.py | 57 +++++++++ tests/datastore_firestore/test_collections.py | 96 +++++++++++++++ tests/datastore_firestore/test_documents.py | 101 +++++++++++++++ tests/testing_support/db_settings.py | 4 +- 6 files changed, 384 insertions(+), 2 deletions(-) create mode 100644 newrelic/hooks/datastore_firestore.py create mode 100644 tests/datastore_firestore/conftest.py create mode 100644 tests/datastore_firestore/test_collections.py create mode 100644 tests/datastore_firestore/test_documents.py diff --git a/newrelic/config.py b/newrelic/config.py index 1424616aa2..992ea4a7f1 100644 --- a/newrelic/config.py +++ b/newrelic/config.py @@ -2269,6 +2269,18 @@ def _process_module_builtin_defaults(): "instrument_graphql_validate", ) + _process_module_definition( + "google.cloud.firestore_v1.base_document", + "newrelic.hooks.database_firestore", + "instrument_google_cloud_firestore_v1_base_document", + ) + + _process_module_definition( + "google.cloud.firestore_v1.base_collection", + "newrelic.hooks.database_firestore", + "instrument_google_cloud_firestore_v1_base_collection", + ) + _process_module_definition( "google.cloud.firestore_v1.document", "newrelic.hooks.database_firestore", diff --git a/newrelic/hooks/datastore_firestore.py b/newrelic/hooks/datastore_firestore.py new file mode 100644 index 0000000000..672438c606 --- /dev/null +++ b/newrelic/hooks/datastore_firestore.py @@ -0,0 +1,116 @@ +# Copyright 2010 New Relic, Inc. +# +# 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. + +from newrelic.common.object_wrapper import wrap_function_wrapper +from newrelic.api.datastore_trace import wrap_datastore_trace +from newrelic.api.function_trace import wrap_function_trace +from newrelic.common.async_wrapper import generator_wrapper +from newrelic.api.datastore_trace import DatastoreTrace + +_firestore_document_commands = ( + "create", + "delete", + "get", + "set", + "update", +) +_firestore_document_generator_commands = ( + "collections", +) + +_firestore_collection_commands = ( + "add", + "get", +) +_firestore_collection_generator_commands = ( + "stream", + "list_documents" +) + +_get_object_id = lambda obj, *args, **kwargs: obj.id + + +def wrap_generator_method(module, class_name, method_name): + def _wrapper(wrapped, instance, args, kwargs): + trace = DatastoreTrace(product="Firestore", target=instance.id, operation=method_name) + wrapped = generator_wrapper(wrapped, trace) + return wrapped(*args, **kwargs) + + class_ = getattr(module, class_name) + if class_ is not None: + if hasattr(class_, method_name): + wrap_function_wrapper(module, "%s.%s" % (class_name, method_name), _wrapper) + + +def instrument_google_cloud_firestore_v1_base_client(module): + rollup = ("Datastore/all", "Datastore/Firestore/all") + wrap_function_trace( + module, "BaseClient.__init__", name="%s:BaseClient.__init__" % module.__name__, terminal=True, rollup=rollup + ) + + +def instrument_google_cloud_firestore_v1_base_collection(module): + for name in _firestore_collection_commands: + if hasattr(module.BaseCollectionReference, name) and not getattr(getattr(module.BaseCollectionReference, name), "_nr_wrapped", False): + wrap_datastore_trace( + module, "BaseCollectionReference.%s" % name, product="Firestore", target=_get_object_id, operation=name + ) + getattr(module.BaseCollectionReference, name)._nr_wrapped = True + + for name in _firestore_collection_generator_commands: + if hasattr(module.BaseCollectionReference, name) and not getattr(getattr(module.BaseCollectionReference, name), "_nr_wrapped", False): + wrap_generator_method(module, "BaseCollectionReference", name) + getattr(module.BaseCollectionReference, name)._nr_wrapped = True + + +def instrument_google_cloud_firestore_v1_collection(module): + for name in _firestore_collection_commands: + if hasattr(module.CollectionReference, name) and not getattr(getattr(module.CollectionReference, name), "_nr_wrapped", False): + wrap_datastore_trace( + module, "CollectionReference.%s" % name, product="Firestore", target=_get_object_id, operation=name + ) + getattr(module.CollectionReference, name)._nr_wrapped = True + + for name in _firestore_collection_generator_commands: + if hasattr(module.CollectionReference, name) and not getattr(getattr(module.CollectionReference, name), "_nr_wrapped", False): + wrap_generator_method(module, "CollectionReference", name) + getattr(module.CollectionReference, name)._nr_wrapped = True + + +def instrument_google_cloud_firestore_v1_base_document(module): + for name in _firestore_document_commands: + if hasattr(module.BaseDocumentReference, name) and not getattr(getattr(module.BaseDocumentReference, name), "_nr_wrapped", False): + wrap_datastore_trace( + module, "BaseDocumentReference.%s" % name, product="Firestore", target=_get_object_id, operation=name + ) + getattr(module.BaseDocumentReference, name)._nr_wrapped = True + + for name in _firestore_document_generator_commands: + if hasattr(module.BaseDocumentReference, name) and not getattr(getattr(module.BaseDocumentReference, name), "_nr_wrapped", False): + wrap_generator_method(module, "BaseDocumentReference", name) + getattr(module.BaseDocumentReference, name)._nr_wrapped = True + + +def instrument_google_cloud_firestore_v1_document(module): + for name in _firestore_document_commands: + if hasattr(module.DocumentReference, name) and not getattr(getattr(module.DocumentReference, name), "_nr_wrapped", False): + wrap_datastore_trace( + module, "DocumentReference.%s" % name, product="Firestore", target=_get_object_id, operation=name + ) + getattr(module.DocumentReference, name)._nr_wrapped = True + + for name in _firestore_document_generator_commands: + if hasattr(module.DocumentReference, name) and not getattr(getattr(module.DocumentReference, name), "_nr_wrapped", False): + wrap_generator_method(module, "DocumentReference", name) + getattr(module.DocumentReference, name)._nr_wrapped = True diff --git a/tests/datastore_firestore/conftest.py b/tests/datastore_firestore/conftest.py new file mode 100644 index 0000000000..166135756a --- /dev/null +++ b/tests/datastore_firestore/conftest.py @@ -0,0 +1,57 @@ +# Copyright 2010 New Relic, Inc. +# +# 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. + +import uuid + +import pytest + +import firebase_admin +from firebase_admin import credentials +from firebase_admin import firestore + +from testing_support.fixtures import collector_agent_registration_fixture, collector_available_fixture # noqa: F401; pylint: disable=W0611 + + +_default_settings = { + 'transaction_tracer.explain_threshold': 0.0, + 'transaction_tracer.transaction_threshold': 0.0, + 'transaction_tracer.stack_trace_threshold': 0.0, + 'debug.log_data_collector_payloads': True, + 'debug.record_transaction_failure': True, + 'debug.log_explain_plan_queries': True +} + +collector_agent_registration = collector_agent_registration_fixture( + app_name='Python Agent Test (datastore_firestore)', + default_settings=_default_settings, + linked_applications=['Python Agent Test (datastore)']) + + +@pytest.fixture(scope="session") +def client(): + creds = credentials.ApplicationDefault() + + firebase_admin.initialize_app(creds) + client = firestore.client() + return client + + +@pytest.fixture(scope="function") +def collection(client): + collection = client.collection("firestore_collection_" + str(uuid.uuid4())) + + yield collection + + for doc in collection.list_documents(): + doc.delete() diff --git a/tests/datastore_firestore/test_collections.py b/tests/datastore_firestore/test_collections.py new file mode 100644 index 0000000000..c416ec4e28 --- /dev/null +++ b/tests/datastore_firestore/test_collections.py @@ -0,0 +1,96 @@ +# Copyright 2010 New Relic, Inc. +# +# 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. + +from newrelic.api.time_trace import current_trace +from newrelic.api.datastore_trace import DatastoreTrace +from testing_support.db_settings import firestore_settings +from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics +from newrelic.api.background_task import background_task +from testing_support.validators.validate_database_duration import ( + validate_database_duration, +) + +DB_SETTINGS = firestore_settings()[0] +FIRESTORE_HOST = DB_SETTINGS["host"] +FIRESTORE_PORT = DB_SETTINGS["port"] + + +def _exercise_firestore(collection): + collection.document("DoesNotExist") + collection.add({"capital": "Rome", "currency": "Euro", "language": "Italian"}, "Italy") + collection.add({"capital": "Mexico City", "currency": "Peso", "language": "Spanish"}, "Mexico") + + documents_get = collection.get() + assert len(documents_get) == 2 + documents_stream = list(collection.stream()) + assert len(documents_stream) == 2 + documents_list = list(collection.list_documents()) + assert len(documents_list) == 2 + + +def test_firestore_collections(collection): + _test_scoped_metrics = [ + ("Datastore/statement/Firestore/%s/stream" % collection.id, 1), + ("Datastore/statement/Firestore/%s/get" % collection.id, 1), + ("Datastore/statement/Firestore/%s/list_documents" % collection.id, 1), + ("Datastore/statement/Firestore/%s/add" % collection.id, 2), + ] + + _test_rollup_metrics = [ + ("Datastore/operation/Firestore/add", 2), + ("Datastore/operation/Firestore/get", 1), + ("Datastore/operation/Firestore/stream", 1), + ("Datastore/operation/Firestore/list_documents", 1), + ("Datastore/all", 5), + ("Datastore/allOther", 5), + ] + @validate_transaction_metrics( + "test_firestore_collections", + scoped_metrics=_test_scoped_metrics, + rollup_metrics=_test_rollup_metrics, + background_task=True, + ) + @background_task(name="test_firestore_collections") + def _test(): + _exercise_firestore(collection) + + _test() + + +@background_task() +def test_firestore_collections_generators(collection): + txn = current_trace() + collection.add({}) + collection.add({}) + assert len(list(collection.list_documents())) == 2 + + # Check for generator trace on stream + _trace_check = [] + for _ in collection.stream(): + _trace_check.append(isinstance(current_trace(), DatastoreTrace)) + assert _trace_check and all(_trace_check) + assert current_trace() is txn + + # Check for generator trace on list_documents + _trace_check = [] + for _ in collection.list_documents(): + _trace_check.append(isinstance(current_trace(), DatastoreTrace)) + assert _trace_check and all(_trace_check) + assert current_trace() is txn + + +@validate_database_duration() +@background_task() +def test_firestore_collections_db_duration(collection): + _exercise_firestore(collection) diff --git a/tests/datastore_firestore/test_documents.py b/tests/datastore_firestore/test_documents.py new file mode 100644 index 0000000000..9850c2c58d --- /dev/null +++ b/tests/datastore_firestore/test_documents.py @@ -0,0 +1,101 @@ +# Copyright 2010 New Relic, Inc. +# +# 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. + +from newrelic.api.time_trace import current_trace +from newrelic.api.datastore_trace import DatastoreTrace +from testing_support.db_settings import firestore_settings +from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics +from newrelic.api.background_task import background_task +from testing_support.validators.validate_database_duration import ( + validate_database_duration, +) + +DB_SETTINGS = firestore_settings()[0] +FIRESTORE_HOST = DB_SETTINGS["host"] +FIRESTORE_PORT = DB_SETTINGS["port"] + + +def _exercise_firestore(collection): + italy_doc = collection.document("Italy") + italy_doc.set({"capital": "Rome", "currency": "Euro", "language": "Italian"}) + italy_doc.get() + italian_cities = italy_doc.collection("cities") + italian_cities.add({"capital": "Rome"}) + retrieved_coll = list(italy_doc.collections()) + assert len(retrieved_coll) == 1 + + usa_doc = collection.document("USA") + usa_doc.create({"capital": "Washington D.C.", "currency": "Dollar", "language": "English"}) + usa_doc.update({"president": "Joe Biden"}) + + collection.document("USA").delete() + + +def test_firestore_documents(collection): + _test_scoped_metrics = [ + ("Datastore/statement/Firestore/Italy/set", 1), + ("Datastore/statement/Firestore/Italy/get", 1), + ("Datastore/statement/Firestore/Italy/collections", 1), + ("Datastore/statement/Firestore/cities/add", 1), + ("Datastore/statement/Firestore/USA/create", 1), + ("Datastore/statement/Firestore/USA/update", 1), + ("Datastore/statement/Firestore/USA/delete", 1), + ] + + _test_rollup_metrics = [ + ("Datastore/operation/Firestore/set", 1), + ("Datastore/operation/Firestore/get", 1), + ("Datastore/operation/Firestore/add", 1), + ("Datastore/operation/Firestore/collections", 1), + ("Datastore/operation/Firestore/create", 1), + ("Datastore/operation/Firestore/update", 1), + ("Datastore/operation/Firestore/delete", 1), + ("Datastore/all", 7), + ("Datastore/allOther", 7), + ] + @validate_transaction_metrics( + "test_firestore_documents", + scoped_metrics=_test_scoped_metrics, + rollup_metrics=_test_rollup_metrics, + background_task=True, + ) + @background_task(name="test_firestore_documents") + def _test(): + _exercise_firestore(collection) + + _test() + + +@background_task() +def test_firestore_documents_generators(collection): + txn = current_trace() + + subcollection_doc = collection.document("SubCollections") + subcollection_doc.set({}) + subcollection_doc.collection("collection1").add({}) + subcollection_doc.collection("collection2").add({}) + assert len(list(subcollection_doc.collections())) == 2 + + # Check for generator trace on collections + _trace_check = [] + for _ in subcollection_doc.collections(): + _trace_check.append(isinstance(current_trace(), DatastoreTrace)) + assert _trace_check and all(_trace_check) + assert current_trace() is txn + + +@validate_database_duration() +@background_task() +def test_firestore_documents_db_duration(collection): + _exercise_firestore(collection) diff --git a/tests/testing_support/db_settings.py b/tests/testing_support/db_settings.py index 6bb9b30628..17cbeed213 100644 --- a/tests/testing_support/db_settings.py +++ b/tests/testing_support/db_settings.py @@ -166,7 +166,7 @@ def mongodb_settings(): def firestore_settings(): - """Return a list of dict of settings for connecting to mongodb. + """Return a list of dict of settings for connecting to firestore. Will return the correct settings, depending on which of the environments it is running in. It attempts to set variables in the following order, where @@ -179,7 +179,7 @@ def firestore_settings(): host = "host.docker.internal" if "GITHUB_ACTIONS" in os.environ else "127.0.0.1" instances = 2 settings = [ - {"host": host, "port": 8080 + instance_num, "collection": "firestore_collection_" + str(os.getpid())} + {"host": host, "port": 8080 + instance_num} for instance_num in range(instances) ] return settings From bab40e639d1d009aee6cde8142f783c5e2e2d2d1 Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Fri, 21 Jul 2023 16:13:22 +0200 Subject: [PATCH 3/5] Add co-authors. Co-authored-by: Tim Pansino Co-authored-by: Lalleh Rafeei Co-authored-by: Hannah Stepanek From 30863aaa71c41821e43d2d0ec6e8b8cc844537bc Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Fri, 21 Jul 2023 16:16:30 +0200 Subject: [PATCH 4/5] Add co-authors. Co-authored-by: Tim Pansino Co-authored-by: Lalleh Rafeei Co-authored-by: Hannah Stepanek --- tests/testing_support/db_settings.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/testing_support/db_settings.py b/tests/testing_support/db_settings.py index bf50311e39..f6b539b666 100644 --- a/tests/testing_support/db_settings.py +++ b/tests/testing_support/db_settings.py @@ -168,6 +168,8 @@ def mongodb_settings(): def firestore_settings(): """Return a list of dict of settings for connecting to firestore. + This only includes the host and port as the collection name is defined in + the firestore conftest file. Will return the correct settings, depending on which of the environments it is running in. It attempts to set variables in the following order, where later environments override earlier ones. From ffcd2484ff3afd6e04aadee2a5429d034822b11a Mon Sep 17 00:00:00 2001 From: Timothy Pansino <11214426+TimPansino@users.noreply.github.com> Date: Fri, 21 Jul 2023 11:33:57 -0700 Subject: [PATCH 5/5] Trim whitespace --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index 019911830a..2123127f36 100644 --- a/tox.ini +++ b/tox.ini @@ -170,7 +170,6 @@ envlist = python-template_jinja2-{py37,py311}-jinja2latest python-template_mako-{py27,py37,py310,py311} - [testenv] deps = # Base Dependencies