From d95d477cdfd54de4490211e3c4dd7de2504057f3 Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Mon, 31 Jul 2023 15:20:41 -0700 Subject: [PATCH 1/8] Update datastore trace wrapper to take instance info. --- newrelic/api/datastore_trace.py | 65 +++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 7 deletions(-) diff --git a/newrelic/api/datastore_trace.py b/newrelic/api/datastore_trace.py index fb40abcab..bc2646f92 100644 --- a/newrelic/api/datastore_trace.py +++ b/newrelic/api/datastore_trace.py @@ -82,6 +82,9 @@ def __enter__(self): self.product = transaction._intern_string(self.product) self.target = transaction._intern_string(self.target) self.operation = transaction._intern_string(self.operation) + self.host = transaction._intern_string(self.host) + self.port_path_or_id = transaction._intern_string(self.port_path_or_id) + self.database_name = transaction._intern_string(self.database_name) datastore_tracer_settings = transaction.settings.datastore_tracer self.instance_reporting_enabled = datastore_tracer_settings.instance_reporting.enabled @@ -92,7 +95,7 @@ def __repr__(self): return "<%s object at 0x%x %s>" % ( self.__class__.__name__, id(self), - dict(product=self.product, target=self.target, operation=self.operation), + dict(product=self.product, target=self.target, operation=self.operation, host=self.host, port_path_or_id=self.port_path_or_id, database_name=self.database_name), ) def finalize_data(self, transaction, exc=None, value=None, tb=None): @@ -125,7 +128,7 @@ def create_node(self): ) -def DatastoreTraceWrapper(wrapped, product, target, operation): +def DatastoreTraceWrapper(wrapped, product, target, operation, host, port_path_or_id, database_name): """Wraps a method to time datastore queries. :param wrapped: The function to apply the trace to. @@ -140,6 +143,14 @@ def DatastoreTraceWrapper(wrapped, product, target, operation): or the name of any API function/method in the client library. :type operation: str or callable + :param host: The name of the server hosting the actual datastore. + :type host: str + :param port_path_or_id: The value passed in can represent either the port, + path, or id of the datastore being connected to. + :type port_path_or_id: str + :param database_name: The name of database where the current query is being + executed. + :type database_name: str :rtype: :class:`newrelic.common.object_wrapper.FunctionWrapper` This is typically used to wrap datastore queries such as calls to Redis or @@ -187,7 +198,31 @@ def _nr_datastore_trace_wrapper_(wrapped, instance, args, kwargs): else: _operation = operation - trace = DatastoreTrace(_product, _target, _operation, parent=parent, source=wrapped) + if callable(host): + if instance is not None: + _host = host(instance, *args, **kwargs) + else: + _host = host(*args, **kwargs) + else: + _host = host + + if callable(port_path_or_id): + if instance is not None: + _port_path_or_id = port_path_or_id(instance, *args, **kwargs) + else: + _port_path_or_id = port_path_or_id(*args, **kwargs) + else: + _port_path_or_id = port_path_or_id + + if callable(database_name): + if instance is not None: + _database_name = database_name(instance, *args, **kwargs) + else: + _database_name = database_name(*args, **kwargs) + else: + _database_name = database_name + + trace = DatastoreTrace(_product, _target, _operation, _host, _port_path_or_id, _database_name, parent=parent, source=wrapped) if wrapper: # pylint: disable=W0125,W0126 return wrapper(wrapped, trace)(*args, **kwargs) @@ -198,7 +233,7 @@ def _nr_datastore_trace_wrapper_(wrapped, instance, args, kwargs): return FunctionWrapper(wrapped, _nr_datastore_trace_wrapper_) -def datastore_trace(product, target, operation): +def datastore_trace(product, target, operation, host, port_path_or_id, database_name): """Decorator allows datastore query to be timed. :param product: The name of the vendor. @@ -211,6 +246,14 @@ def datastore_trace(product, target, operation): or the name of any API function/method in the client library. :type operation: str + :param host: The name of the server hosting the actual datastore. + :type host: str + :param port_path_or_id: The value passed in can represent either the port, + path, or id of the datastore being connected to. + :type port_path_or_id: str + :param database_name: The name of database where the current query is being + executed. + :type database_name: str This is typically used to decorate datastore queries such as calls to Redis or ElasticSearch. @@ -224,10 +267,10 @@ def datastore_trace(product, target, operation): ... time.sleep(*args, **kwargs) """ - return functools.partial(DatastoreTraceWrapper, product=product, target=target, operation=operation) + return functools.partial(DatastoreTraceWrapper, product=product, target=target, operation=operation, host=host, port_path_or_id=port_path_or_id, database_name=database_name) -def wrap_datastore_trace(module, object_path, product, target, operation): +def wrap_datastore_trace(module, object_path, product, target, operation, host, port_path_or_id, database_name): """Method applies custom timing to datastore query. :param module: Module containing the method to be instrumented. @@ -244,6 +287,14 @@ def wrap_datastore_trace(module, object_path, product, target, operation): or the name of any API function/method in the client library. :type operation: str + :param host: The name of the server hosting the actual datastore. + :type host: str + :param port_path_or_id: The value passed in can represent either the port, + path, or id of the datastore being connected to. + :type port_path_or_id: str + :param database_name: The name of database where the current query is being + executed. + :type database_name: str This is typically used to time database query method calls such as Redis GET. @@ -256,4 +307,4 @@ def wrap_datastore_trace(module, object_path, product, target, operation): ... 'sleep') """ - wrap_object(module, object_path, DatastoreTraceWrapper, (product, target, operation)) + wrap_object(module, object_path, DatastoreTraceWrapper, (product, target, operation, host, port_path_or_id, database_name)) From 53f8400ce0d0e8b53bfcaba4b54f898a63e3d68b Mon Sep 17 00:00:00 2001 From: umaannamalai Date: Mon, 31 Jul 2023 22:23:20 +0000 Subject: [PATCH 2/8] [Mega-Linter] Apply linters fixes --- newrelic/api/datastore_trace.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/newrelic/api/datastore_trace.py b/newrelic/api/datastore_trace.py index bc2646f92..d890ebb3f 100644 --- a/newrelic/api/datastore_trace.py +++ b/newrelic/api/datastore_trace.py @@ -95,7 +95,14 @@ def __repr__(self): return "<%s object at 0x%x %s>" % ( self.__class__.__name__, id(self), - dict(product=self.product, target=self.target, operation=self.operation, host=self.host, port_path_or_id=self.port_path_or_id, database_name=self.database_name), + dict( + product=self.product, + target=self.target, + operation=self.operation, + host=self.host, + port_path_or_id=self.port_path_or_id, + database_name=self.database_name, + ), ) def finalize_data(self, transaction, exc=None, value=None, tb=None): @@ -222,7 +229,9 @@ def _nr_datastore_trace_wrapper_(wrapped, instance, args, kwargs): else: _database_name = database_name - trace = DatastoreTrace(_product, _target, _operation, _host, _port_path_or_id, _database_name, parent=parent, source=wrapped) + trace = DatastoreTrace( + _product, _target, _operation, _host, _port_path_or_id, _database_name, parent=parent, source=wrapped + ) if wrapper: # pylint: disable=W0125,W0126 return wrapper(wrapped, trace)(*args, **kwargs) @@ -267,7 +276,15 @@ def datastore_trace(product, target, operation, host, port_path_or_id, database_ ... time.sleep(*args, **kwargs) """ - return functools.partial(DatastoreTraceWrapper, product=product, target=target, operation=operation, host=host, port_path_or_id=port_path_or_id, database_name=database_name) + return functools.partial( + DatastoreTraceWrapper, + product=product, + target=target, + operation=operation, + host=host, + port_path_or_id=port_path_or_id, + database_name=database_name, + ) def wrap_datastore_trace(module, object_path, product, target, operation, host, port_path_or_id, database_name): @@ -307,4 +324,6 @@ def wrap_datastore_trace(module, object_path, product, target, operation, host, ... 'sleep') """ - wrap_object(module, object_path, DatastoreTraceWrapper, (product, target, operation, host, port_path_or_id, database_name)) + wrap_object( + module, object_path, DatastoreTraceWrapper, (product, target, operation, host, port_path_or_id, database_name) + ) From 7687c0695783fe40a86e705ec9790c19248f0c1e Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Mon, 31 Jul 2023 15:47:09 -0700 Subject: [PATCH 3/8] Make instance info args optional. --- newrelic/api/datastore_trace.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/newrelic/api/datastore_trace.py b/newrelic/api/datastore_trace.py index d890ebb3f..d3a57ff92 100644 --- a/newrelic/api/datastore_trace.py +++ b/newrelic/api/datastore_trace.py @@ -135,7 +135,7 @@ def create_node(self): ) -def DatastoreTraceWrapper(wrapped, product, target, operation, host, port_path_or_id, database_name): +def DatastoreTraceWrapper(wrapped, product, target, operation, host=None, port_path_or_id=None, database_name=None): """Wraps a method to time datastore queries. :param wrapped: The function to apply the trace to. @@ -242,7 +242,7 @@ def _nr_datastore_trace_wrapper_(wrapped, instance, args, kwargs): return FunctionWrapper(wrapped, _nr_datastore_trace_wrapper_) -def datastore_trace(product, target, operation, host, port_path_or_id, database_name): +def datastore_trace(product, target, operation, host=None, port_path_or_id=None, database_name=None): """Decorator allows datastore query to be timed. :param product: The name of the vendor. @@ -287,7 +287,7 @@ def datastore_trace(product, target, operation, host, port_path_or_id, database_ ) -def wrap_datastore_trace(module, object_path, product, target, operation, host, port_path_or_id, database_name): +def wrap_datastore_trace(module, object_path, product, target, operation, host=None, port_path_or_id=None, database_name=None): """Method applies custom timing to datastore query. :param module: Module containing the method to be instrumented. From 1c426c84b2c8ee36c6a40bf6bbfcb862c90db1cf Mon Sep 17 00:00:00 2001 From: umaannamalai Date: Mon, 31 Jul 2023 23:01:49 +0000 Subject: [PATCH 4/8] [Mega-Linter] Apply linters fixes --- newrelic/api/datastore_trace.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/newrelic/api/datastore_trace.py b/newrelic/api/datastore_trace.py index d3a57ff92..1561293f6 100644 --- a/newrelic/api/datastore_trace.py +++ b/newrelic/api/datastore_trace.py @@ -287,7 +287,9 @@ def datastore_trace(product, target, operation, host=None, port_path_or_id=None, ) -def wrap_datastore_trace(module, object_path, product, target, operation, host=None, port_path_or_id=None, database_name=None): +def wrap_datastore_trace( + module, object_path, product, target, operation, host=None, port_path_or_id=None, database_name=None +): """Method applies custom timing to datastore query. :param module: Module containing the method to be instrumented. From e322301377d077a17778c198fd99293c2aee3818 Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Tue, 1 Aug 2023 17:54:04 -0700 Subject: [PATCH 5/8] Add datastore trace testing. --- tests/agent_features/test_datastore_trace.py | 58 +++++++++++++++++++ .../validate_datastore_trace_inputs.py | 9 ++- 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 tests/agent_features/test_datastore_trace.py diff --git a/tests/agent_features/test_datastore_trace.py b/tests/agent_features/test_datastore_trace.py new file mode 100644 index 000000000..59995b0b5 --- /dev/null +++ b/tests/agent_features/test_datastore_trace.py @@ -0,0 +1,58 @@ +# 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.background_task import background_task +from newrelic.api.datastore_trace import DatastoreTrace, DatastoreTraceWrapper +from testing_support.validators.validate_datastore_trace_inputs import ( + validate_datastore_trace_inputs, +) + + +@validate_datastore_trace_inputs(operation='test_operation', target='test_target', host='test_host', port_path_or_id='test_port', database_name='test_db_name') +def test_dt_trace_all_args(): + with DatastoreTrace(product='Agent Features', target='test_target', operation='test_operation', host='test_host', port_path_or_id='test_port', database_name='test_db_name'): + pass + + +@validate_datastore_trace_inputs(operation=None, target=None, host=None, port_path_or_id=None, database_name=None) +def test_dt_trace_empty(): + with DatastoreTrace(product=None, target=None, operation=None): + pass + + +@background_task() +def test_dt_trace_callable_args(): + def product_callable(): + return "Agent Features" + + def target_callable(): + return 'test_target' + + def operation_callable(): + return "test_operation" + + def host_callable(): + return 'test_host' + + def port_path_id_callable(): + return 'test_port' + + def db_name_callable(): + return 'test_db_name' + + @validate_datastore_trace_inputs(operation='test_operation', target='test_target', host='test_host', port_path_or_id='test_port', database_name='test_db_name') + def _test(): + pass + wrapped_fn = DatastoreTraceWrapper(_test, product=product_callable, target=target_callable, operation=operation_callable, host=host_callable, port_path_or_id=port_path_id_callable, database_name=db_name_callable) + wrapped_fn() diff --git a/tests/testing_support/validators/validate_datastore_trace_inputs.py b/tests/testing_support/validators/validate_datastore_trace_inputs.py index ade4ebea6..6d6c79cf2 100644 --- a/tests/testing_support/validators/validate_datastore_trace_inputs.py +++ b/tests/testing_support/validators/validate_datastore_trace_inputs.py @@ -23,7 +23,7 @@ """ -def validate_datastore_trace_inputs(operation=None, target=None): +def validate_datastore_trace_inputs(operation=None, target=None, host=None, port_path_or_id=None, database_name=None): @transient_function_wrapper("newrelic.api.datastore_trace", "DatastoreTrace.__init__") @catch_background_exceptions def _validate_datastore_trace_inputs(wrapped, instance, args, kwargs): @@ -44,6 +44,13 @@ def _bind_params(product, target, operation, host=None, port_path_or_id=None, da assert captured_target == target, "%s didn't match expected %s" % (captured_target, target) if operation is not None: assert captured_operation == operation, "%s didn't match expected %s" % (captured_operation, operation) + if host is not None: + assert host == host, "%s didn't match expected %s" % (captured_host, host) + if port_path_or_id is not None: + assert captured_port_path_or_id == port_path_or_id, "%s didn't match expected %s" % (captured_port_path_or_id, port_path_or_id) + if database_name is not None: + assert captured_database_name == database_name, "%s didn't match expected %s" % (captured_database_name, database_name) + return wrapped(*args, **kwargs) From aa96d5fdb58436eb6b381b2e6aecca886e291e4c Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Tue, 1 Aug 2023 17:55:54 -0700 Subject: [PATCH 6/8] Add background task decorator. --- tests/agent_features/test_datastore_trace.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/agent_features/test_datastore_trace.py b/tests/agent_features/test_datastore_trace.py index 59995b0b5..eee82d8dc 100644 --- a/tests/agent_features/test_datastore_trace.py +++ b/tests/agent_features/test_datastore_trace.py @@ -20,12 +20,14 @@ @validate_datastore_trace_inputs(operation='test_operation', target='test_target', host='test_host', port_path_or_id='test_port', database_name='test_db_name') +@background_task() def test_dt_trace_all_args(): with DatastoreTrace(product='Agent Features', target='test_target', operation='test_operation', host='test_host', port_path_or_id='test_port', database_name='test_db_name'): pass @validate_datastore_trace_inputs(operation=None, target=None, host=None, port_path_or_id=None, database_name=None) +@background_task() def test_dt_trace_empty(): with DatastoreTrace(product=None, target=None, operation=None): pass From 9f10798eb5f2d9c586e245c00ae18fbb1d8f9608 Mon Sep 17 00:00:00 2001 From: umaannamalai Date: Wed, 2 Aug 2023 00:59:01 +0000 Subject: [PATCH 7/8] [Mega-Linter] Apply linters fixes --- tests/agent_features/test_datastore_trace.py | 49 +++++++++++++++---- .../validate_datastore_trace_inputs.py | 11 +++-- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/tests/agent_features/test_datastore_trace.py b/tests/agent_features/test_datastore_trace.py index eee82d8dc..08067e040 100644 --- a/tests/agent_features/test_datastore_trace.py +++ b/tests/agent_features/test_datastore_trace.py @@ -12,17 +12,31 @@ # See the License for the specific language governing permissions and # limitations under the License. -from newrelic.api.background_task import background_task -from newrelic.api.datastore_trace import DatastoreTrace, DatastoreTraceWrapper from testing_support.validators.validate_datastore_trace_inputs import ( validate_datastore_trace_inputs, ) +from newrelic.api.background_task import background_task +from newrelic.api.datastore_trace import DatastoreTrace, DatastoreTraceWrapper -@validate_datastore_trace_inputs(operation='test_operation', target='test_target', host='test_host', port_path_or_id='test_port', database_name='test_db_name') + +@validate_datastore_trace_inputs( + operation="test_operation", + target="test_target", + host="test_host", + port_path_or_id="test_port", + database_name="test_db_name", +) @background_task() def test_dt_trace_all_args(): - with DatastoreTrace(product='Agent Features', target='test_target', operation='test_operation', host='test_host', port_path_or_id='test_port', database_name='test_db_name'): + with DatastoreTrace( + product="Agent Features", + target="test_target", + operation="test_operation", + host="test_host", + port_path_or_id="test_port", + database_name="test_db_name", + ): pass @@ -39,22 +53,37 @@ def product_callable(): return "Agent Features" def target_callable(): - return 'test_target' + return "test_target" def operation_callable(): return "test_operation" def host_callable(): - return 'test_host' + return "test_host" def port_path_id_callable(): - return 'test_port' + return "test_port" def db_name_callable(): - return 'test_db_name' + return "test_db_name" - @validate_datastore_trace_inputs(operation='test_operation', target='test_target', host='test_host', port_path_or_id='test_port', database_name='test_db_name') + @validate_datastore_trace_inputs( + operation="test_operation", + target="test_target", + host="test_host", + port_path_or_id="test_port", + database_name="test_db_name", + ) def _test(): pass - wrapped_fn = DatastoreTraceWrapper(_test, product=product_callable, target=target_callable, operation=operation_callable, host=host_callable, port_path_or_id=port_path_id_callable, database_name=db_name_callable) + + wrapped_fn = DatastoreTraceWrapper( + _test, + product=product_callable, + target=target_callable, + operation=operation_callable, + host=host_callable, + port_path_or_id=port_path_id_callable, + database_name=db_name_callable, + ) wrapped_fn() diff --git a/tests/testing_support/validators/validate_datastore_trace_inputs.py b/tests/testing_support/validators/validate_datastore_trace_inputs.py index 6d6c79cf2..8e0841475 100644 --- a/tests/testing_support/validators/validate_datastore_trace_inputs.py +++ b/tests/testing_support/validators/validate_datastore_trace_inputs.py @@ -47,10 +47,15 @@ def _bind_params(product, target, operation, host=None, port_path_or_id=None, da if host is not None: assert host == host, "%s didn't match expected %s" % (captured_host, host) if port_path_or_id is not None: - assert captured_port_path_or_id == port_path_or_id, "%s didn't match expected %s" % (captured_port_path_or_id, port_path_or_id) + assert captured_port_path_or_id == port_path_or_id, "%s didn't match expected %s" % ( + captured_port_path_or_id, + port_path_or_id, + ) if database_name is not None: - assert captured_database_name == database_name, "%s didn't match expected %s" % (captured_database_name, database_name) - + assert captured_database_name == database_name, "%s didn't match expected %s" % ( + captured_database_name, + database_name, + ) return wrapped(*args, **kwargs) From 4d14bfd2e60828746c98a2ce6f974d0c4d48a9f8 Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Wed, 2 Aug 2023 09:46:21 -0700 Subject: [PATCH 8/8] Fix typo in validator. --- .../validators/validate_datastore_trace_inputs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testing_support/validators/validate_datastore_trace_inputs.py b/tests/testing_support/validators/validate_datastore_trace_inputs.py index 8e0841475..365a14ebd 100644 --- a/tests/testing_support/validators/validate_datastore_trace_inputs.py +++ b/tests/testing_support/validators/validate_datastore_trace_inputs.py @@ -45,7 +45,7 @@ def _bind_params(product, target, operation, host=None, port_path_or_id=None, da if operation is not None: assert captured_operation == operation, "%s didn't match expected %s" % (captured_operation, operation) if host is not None: - assert host == host, "%s didn't match expected %s" % (captured_host, host) + assert captured_host == host, "%s didn't match expected %s" % (captured_host, host) if port_path_or_id is not None: assert captured_port_path_or_id == port_path_or_id, "%s didn't match expected %s" % ( captured_port_path_or_id,