From 693111a532478ed2ab723e8d0fcb47c6e11a418c Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 20 Nov 2019 17:48:41 +0100 Subject: [PATCH 1/6] added auto-instrumentation of Django management commands fixes #627 --- docs/configuration.asciidoc | 10 +++ docs/django.asciidoc | 16 +++- elasticapm/conf/__init__.py | 1 + .../packages/django/commands.py | 64 ++++++++++++++++ elasticapm/instrumentation/register.py | 1 + tests/contrib/django/test_commands.py | 75 +++++++++++++++++++ .../django/testapp/management/__init__.py | 29 +++++++ .../testapp/management/commands/__init__.py | 29 +++++++ .../management/commands/eapm_test_command.py | 51 +++++++++++++ 9 files changed, 275 insertions(+), 1 deletion(-) create mode 100644 elasticapm/instrumentation/packages/django/commands.py create mode 100644 tests/contrib/django/test_commands.py create mode 100644 tests/contrib/django/testapp/management/__init__.py create mode 100644 tests/contrib/django/testapp/management/commands/__init__.py create mode 100644 tests/contrib/django/testapp/management/commands/eapm_test_command.py diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index 07e3ae863..cd036212d 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -750,6 +750,16 @@ To trace Django requests, the agent uses a middleware, `elasticapm.contrib.djang By default, this middleware is inserted automatically as the first item in `settings.MIDDLEWARES`. To disable the automatic insertion of the middleware, change this setting to `False`. +[float] +[[config-django-instrument-commands]] +==== `django_instrument_commands` +|============ +| Environment | Django | Default +| `ELASTIC_APM_DJANGO_INSTRUMENT_COMMANDS` | `DJANGO_INSTRUMENT_COMMANDS` | `True` +|============ + +By default, Elastic APM instruments Django management commands. +You can disable this by setting this setting to `False`. [float] [[config-generic-environment]] diff --git a/docs/django.asciidoc b/docs/django.asciidoc index f4bd0992e..98edd981f 100644 --- a/docs/django.asciidoc +++ b/docs/django.asciidoc @@ -83,7 +83,7 @@ In order to collect performance metrics, the agent automatically inserts a middleware at the top of your middleware list (`settings.MIDDLEWARE` in current versions of Django, `settings.MIDDLEWARE_CLASSES` in some older versions). To disable the automatic insertion of the middleware, -see <>. +see <>. NOTE: For automatic insertion to work, your list of middlewares (`settings.MIDDLEWARE` or `settings.MIDDLEWARE_CLASSES`) must be of type `list` or `tuple`. @@ -93,6 +93,20 @@ the agent also collects fine grained metrics on template rendering, database queries, HTTP requests, etc. You can find more information on what we instrument in the <> section. +Lastly, the agent will also collect performance data for Django management commands. +To disable this, use the <> setting. + +[NOTE] +==== +The agent also collects command line arguments as additional meta data for transactions. +If you run a command that contains sensitive data, like tokens or passwords, +we recommend to prefix the call with `ELASTIC_APM_DJANGO_INSTRUMENT_COMMANDS=False`, e.g. +[source,bash] +---- +ELASTIC_APM_DJANGO_INSTRUMENT_COMMANDS=False ./manage.py upload_to_service --token FCXNk23PHFy9A6q764BJ +---- +==== + [float] [[django-instrumenting-custom-python-code]] ===== Instrumenting custom Python code diff --git a/elasticapm/conf/__init__.py b/elasticapm/conf/__init__.py index 84bbe8f17..c670df83e 100644 --- a/elasticapm/conf/__init__.py +++ b/elasticapm/conf/__init__.py @@ -322,6 +322,7 @@ class Config(_ConfigBase): capture_body = _ConfigValue("CAPTURE_BODY", default="off") async_mode = _BoolConfigValue("ASYNC_MODE", default=True) instrument_django_middleware = _BoolConfigValue("INSTRUMENT_DJANGO_MIDDLEWARE", default=True) + instrument_django_commands = _BoolConfigValue("INSTRUMENT_DJANGO_COMMANDS", default=True) autoinsert_django_middleware = _BoolConfigValue("AUTOINSERT_DJANGO_MIDDLEWARE", default=True) transactions_ignore_patterns = _ListConfigValue("TRANSACTIONS_IGNORE_PATTERNS", default=[]) service_version = _ConfigValue("SERVICE_VERSION") diff --git a/elasticapm/instrumentation/packages/django/commands.py b/elasticapm/instrumentation/packages/django/commands.py new file mode 100644 index 000000000..2a4734522 --- /dev/null +++ b/elasticapm/instrumentation/packages/django/commands.py @@ -0,0 +1,64 @@ +# BSD 3-Clause License +# +# Copyright (c) 2019, Elasticsearch BV +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +from django.apps import apps + +from elasticapm.instrumentation.packages.base import AbstractInstrumentedModule +from elasticapm.utils import compat + + +class DjangoCommandInstrumentation(AbstractInstrumentedModule): + name = "django_command" + + instrument_list = [("django.core.management", "BaseCommand.execute")] + + def call_if_sampling(self, module, method, wrapped, instance, args, kwargs): + app = apps.get_app_config("elasticapm.contrib.django") + client = getattr(app, "client", None) + if not client or not client.config.instrument_django_commands: + return wrapped(*args, **kwargs) + name = compat.text_type(instance.__module__) + transaction = client.begin_transaction("django_command") + transaction.is_sampled = True # always sample transactions + status = "ok" + try: + return wrapped(*args, **kwargs) + except Exception as e: + status = "failed" + client.capture_exception() + if compat.PY3: + raise e # Python 3 + else: + import sys + + transaction, v, tb = sys.exc_info() + raise transaction(v).with_traceback(tb) + finally: + client.end_transaction(name, status) diff --git a/elasticapm/instrumentation/register.py b/elasticapm/instrumentation/register.py index defa97df9..8cf3c087d 100644 --- a/elasticapm/instrumentation/register.py +++ b/elasticapm/instrumentation/register.py @@ -56,6 +56,7 @@ "elasticapm.instrumentation.packages.pyodbc.PyODBCInstrumentation", "elasticapm.instrumentation.packages.django.template.DjangoTemplateInstrumentation", "elasticapm.instrumentation.packages.django.template.DjangoTemplateSourceInstrumentation", + "elasticapm.instrumentation.packages.django.commands.DjangoCommandInstrumentation", "elasticapm.instrumentation.packages.urllib.UrllibInstrumentation", } diff --git a/tests/contrib/django/test_commands.py b/tests/contrib/django/test_commands.py new file mode 100644 index 000000000..5e2e54a31 --- /dev/null +++ b/tests/contrib/django/test_commands.py @@ -0,0 +1,75 @@ +# BSD 3-Clause License +# +# Copyright (c) 2019, Elasticsearch BV +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +from django.core.management import CommandError, call_command + +import pytest + +from elasticapm.conf import constants + + +def test_management_command(django_elasticapm_client): + call_command("eapm_test_command") + transaction = django_elasticapm_client.events[constants.TRANSACTION][0] + assert transaction["type"] == "django_command" + assert transaction["name"] == "tests.contrib.django.testapp.management.commands.eapm_test_command" + assert transaction["result"] == "ok" + + spans = django_elasticapm_client.spans_for_transaction(transaction) + assert len(spans) == 1 + assert spans[0]["name"] == "yay" + + +def test_management_command_command_error(django_elasticapm_client): + with pytest.raises(CommandError): + call_command("eapm_test_command", explode="yes") + transaction = django_elasticapm_client.events[constants.TRANSACTION][0] + assert transaction["type"] == "django_command" + assert transaction["name"] == "tests.contrib.django.testapp.management.commands.eapm_test_command" + assert transaction["result"] == "failed" + + exception = django_elasticapm_client.events[constants.ERROR][0] + assert exception["culprit"] == "tests.contrib.django.testapp.management.commands.eapm_test_command.handle" + assert exception["exception"]["message"] == "CommandError: oh no" + assert exception["transaction_id"] == transaction["id"] + + +def test_management_command_other_error(django_elasticapm_client): + with pytest.raises(ZeroDivisionError): + call_command("eapm_test_command", explode="yes, really") + transaction = django_elasticapm_client.events[constants.TRANSACTION][0] + assert transaction["type"] == "django_command" + assert transaction["name"] == "tests.contrib.django.testapp.management.commands.eapm_test_command" + assert transaction["result"] == "failed" + + exception = django_elasticapm_client.events[constants.ERROR][0] + assert exception["culprit"] == "tests.contrib.django.testapp.management.commands.eapm_test_command.handle" + assert exception["exception"]["message"] == "ZeroDivisionError: division by zero" + assert exception["transaction_id"] == transaction["id"] diff --git a/tests/contrib/django/testapp/management/__init__.py b/tests/contrib/django/testapp/management/__init__.py new file mode 100644 index 000000000..7e2b340e6 --- /dev/null +++ b/tests/contrib/django/testapp/management/__init__.py @@ -0,0 +1,29 @@ +# BSD 3-Clause License +# +# Copyright (c) 2019, Elasticsearch BV +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/tests/contrib/django/testapp/management/commands/__init__.py b/tests/contrib/django/testapp/management/commands/__init__.py new file mode 100644 index 000000000..7e2b340e6 --- /dev/null +++ b/tests/contrib/django/testapp/management/commands/__init__.py @@ -0,0 +1,29 @@ +# BSD 3-Clause License +# +# Copyright (c) 2019, Elasticsearch BV +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/tests/contrib/django/testapp/management/commands/eapm_test_command.py b/tests/contrib/django/testapp/management/commands/eapm_test_command.py new file mode 100644 index 000000000..e7d8c6354 --- /dev/null +++ b/tests/contrib/django/testapp/management/commands/eapm_test_command.py @@ -0,0 +1,51 @@ +# BSD 3-Clause License +# +# Copyright (c) 2019, Elasticsearch BV +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +from django.core.management.base import BaseCommand, CommandError + +from elasticapm import capture_span + + +class Command(BaseCommand): + help = "Just a test" + + def add_arguments(self, parser): + parser.add_argument("--explode", default="no", action="store") + + def handle(self, *args, **options): + + with capture_span("yay"): + pass + + if options["explode"] == "yes": + raise CommandError("oh no") + + elif options["explode"] == "yes, really": + nan = 1 / 0 From 12040e27b8ce5afa7b65e1ca810ff3690a5bc71b Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 20 Nov 2019 22:49:58 +0100 Subject: [PATCH 2/6] use a list of commands to exclude instead of a true/false flag --- docs/configuration.asciidoc | 11 ++++--- docs/django.asciidoc | 17 ++++------ elasticapm/conf/__init__.py | 6 +++- .../packages/django/commands.py | 8 +++-- tests/contrib/django/test_commands.py | 33 +++++++++++++++++++ 5 files changed, 55 insertions(+), 20 deletions(-) diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index cd036212d..0cd274f36 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -751,15 +751,16 @@ By default, this middleware is inserted automatically as the first item in `sett To disable the automatic insertion of the middleware, change this setting to `False`. [float] -[[config-django-instrument-commands]] -==== `django_instrument_commands` +[[config-django-commands-exclude]] +==== `django_commands_exclude` |============ -| Environment | Django | Default -| `ELASTIC_APM_DJANGO_INSTRUMENT_COMMANDS` | `DJANGO_INSTRUMENT_COMMANDS` | `True` +| Environment | Django | Default +| `ELASTIC_APM_DJANGO_COMMANDS_EXCLUDE` | `DJANGO_COMMANDS_EXCLUDE` | `runserver*,migrate,createsuperuser,\*shell*,testserver` |============ By default, Elastic APM instruments Django management commands. -You can disable this by setting this setting to `False`. +You can supply a list of commands that should not be instrumented. +To disable instrumenting of management commands, set it to `*`. [float] [[config-generic-environment]] diff --git a/docs/django.asciidoc b/docs/django.asciidoc index 98edd981f..cf1b50f3d 100644 --- a/docs/django.asciidoc +++ b/docs/django.asciidoc @@ -94,18 +94,13 @@ database queries, HTTP requests, etc. You can find more information on what we instrument in the <> section. Lastly, the agent will also collect performance data for Django management commands. -To disable this, use the <> setting. +You can disable instrumenting for certain commands using the +<> setting. +Transactions for management commands can be accessed in the APM app in Kibana by choosing `django_command` in the "transaction type" filter. -[NOTE] -==== -The agent also collects command line arguments as additional meta data for transactions. -If you run a command that contains sensitive data, like tokens or passwords, -we recommend to prefix the call with `ELASTIC_APM_DJANGO_INSTRUMENT_COMMANDS=False`, e.g. -[source,bash] ----- -ELASTIC_APM_DJANGO_INSTRUMENT_COMMANDS=False ./manage.py upload_to_service --token FCXNk23PHFy9A6q764BJ ----- -==== +NOTE: The agent collects command line arguments as additional meta data for transactions. +If you run a command that contains sensitive data on the command line, like tokens or passwords, +we recommend to exclude that command from instrumentation. [float] [[django-instrumenting-custom-python-code]] diff --git a/elasticapm/conf/__init__.py b/elasticapm/conf/__init__.py index c670df83e..71f4e65f5 100644 --- a/elasticapm/conf/__init__.py +++ b/elasticapm/conf/__init__.py @@ -322,7 +322,11 @@ class Config(_ConfigBase): capture_body = _ConfigValue("CAPTURE_BODY", default="off") async_mode = _BoolConfigValue("ASYNC_MODE", default=True) instrument_django_middleware = _BoolConfigValue("INSTRUMENT_DJANGO_MIDDLEWARE", default=True) - instrument_django_commands = _BoolConfigValue("INSTRUMENT_DJANGO_COMMANDS", default=True) + django_commands_exclude = _ListConfigValue( + "DJANGO_COMMANDS_EXCLUDE", + type=starmatch_to_regex, + default=list(map(starmatch_to_regex, ["runserver*", "migrate", "createsuperuser", "*shell*", "testserver"])), + ) autoinsert_django_middleware = _BoolConfigValue("AUTOINSERT_DJANGO_MIDDLEWARE", default=True) transactions_ignore_patterns = _ListConfigValue("TRANSACTIONS_IGNORE_PATTERNS", default=[]) service_version = _ConfigValue("SERVICE_VERSION") diff --git a/elasticapm/instrumentation/packages/django/commands.py b/elasticapm/instrumentation/packages/django/commands.py index 2a4734522..a1b06eb6d 100644 --- a/elasticapm/instrumentation/packages/django/commands.py +++ b/elasticapm/instrumentation/packages/django/commands.py @@ -42,9 +42,11 @@ class DjangoCommandInstrumentation(AbstractInstrumentedModule): def call_if_sampling(self, module, method, wrapped, instance, args, kwargs): app = apps.get_app_config("elasticapm.contrib.django") client = getattr(app, "client", None) - if not client or not client.config.instrument_django_commands: + full_name = compat.text_type(instance.__module__) + name = full_name.rsplit(".", 1)[-1] + if not client or any(pattern.match(name) for pattern in client.config.django_commands_exclude): return wrapped(*args, **kwargs) - name = compat.text_type(instance.__module__) + transaction = client.begin_transaction("django_command") transaction.is_sampled = True # always sample transactions status = "ok" @@ -61,4 +63,4 @@ def call_if_sampling(self, module, method, wrapped, instance, args, kwargs): transaction, v, tb = sys.exc_info() raise transaction(v).with_traceback(tb) finally: - client.end_transaction(name, status) + client.end_transaction(full_name, status) diff --git a/tests/contrib/django/test_commands.py b/tests/contrib/django/test_commands.py index 5e2e54a31..d2eb7910a 100644 --- a/tests/contrib/django/test_commands.py +++ b/tests/contrib/django/test_commands.py @@ -27,6 +27,9 @@ # CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +import pytest # isort:skip + +django = pytest.importorskip("django") # isort:skip from django.core.management import CommandError, call_command @@ -73,3 +76,33 @@ def test_management_command_other_error(django_elasticapm_client): assert exception["culprit"] == "tests.contrib.django.testapp.management.commands.eapm_test_command.handle" assert exception["exception"]["message"] == "ZeroDivisionError: division by zero" assert exception["transaction_id"] == transaction["id"] + + +@pytest.mark.parametrize("django_elasticapm_client", [{"django_commands_exclude": "*"}], indirect=True) +def test_management_command_ignore_all(django_elasticapm_client): + call_command("eapm_test_command") + assert len(django_elasticapm_client.events[constants.TRANSACTION]) == 0 + + +@pytest.mark.parametrize( + "django_elasticapm_client", [{"django_commands_exclude": "eapm_test_command,other_command"}], indirect=True +) +def test_management_command_ignore_exact(django_elasticapm_client): + call_command("eapm_test_command") + assert len(django_elasticapm_client.events[constants.TRANSACTION]) == 0 + + +@pytest.mark.parametrize( + "django_elasticapm_client", [{"django_commands_exclude": "eapm_test_command,other_command"}], indirect=True +) +def test_management_command_ignore_exact(django_elasticapm_client): + call_command("eapm_test_command") + assert len(django_elasticapm_client.events[constants.TRANSACTION]) == 0 + + +@pytest.mark.parametrize( + "django_elasticapm_client", [{"django_commands_exclude": "this_command,other_command"}], indirect=True +) +def test_management_command_ignore_no_match(django_elasticapm_client): + call_command("eapm_test_command") + assert len(django_elasticapm_client.events[constants.TRANSACTION]) == 1 From d6c9cce572fa1ed285db475284944dc011ff1087 Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 20 Nov 2019 23:15:11 +0100 Subject: [PATCH 3/6] fix import error --- elasticapm/instrumentation/packages/django/commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/elasticapm/instrumentation/packages/django/commands.py b/elasticapm/instrumentation/packages/django/commands.py index a1b06eb6d..f8d0e6491 100644 --- a/elasticapm/instrumentation/packages/django/commands.py +++ b/elasticapm/instrumentation/packages/django/commands.py @@ -28,8 +28,6 @@ # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from django.apps import apps - from elasticapm.instrumentation.packages.base import AbstractInstrumentedModule from elasticapm.utils import compat @@ -40,6 +38,8 @@ class DjangoCommandInstrumentation(AbstractInstrumentedModule): instrument_list = [("django.core.management", "BaseCommand.execute")] def call_if_sampling(self, module, method, wrapped, instance, args, kwargs): + from django.apps import apps # import at top level fails if Django is not installed + app = apps.get_app_config("elasticapm.contrib.django") client = getattr(app, "client", None) full_name = compat.text_type(instance.__module__) From e975b459e9e319e22d954381fd1620902358c152 Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Wed, 20 Nov 2019 23:45:48 +0100 Subject: [PATCH 4/6] copied six.reraise into our compat module, and use it in command instrumentation six is MIT-licensed, a copyright notice is already included in our compat module --- .../packages/django/commands.py | 11 ++----- elasticapm/utils/compat.py | 32 ++++++++++++++++++- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/elasticapm/instrumentation/packages/django/commands.py b/elasticapm/instrumentation/packages/django/commands.py index f8d0e6491..fabb66b2a 100644 --- a/elasticapm/instrumentation/packages/django/commands.py +++ b/elasticapm/instrumentation/packages/django/commands.py @@ -27,6 +27,7 @@ # CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +import sys from elasticapm.instrumentation.packages.base import AbstractInstrumentedModule from elasticapm.utils import compat @@ -52,15 +53,9 @@ def call_if_sampling(self, module, method, wrapped, instance, args, kwargs): status = "ok" try: return wrapped(*args, **kwargs) - except Exception as e: + except Exception: status = "failed" client.capture_exception() - if compat.PY3: - raise e # Python 3 - else: - import sys - - transaction, v, tb = sys.exc_info() - raise transaction(v).with_traceback(tb) + compat.reraise(*sys.exc_info()) finally: client.end_transaction(full_name, status) diff --git a/elasticapm/utils/compat.py b/elasticapm/utils/compat.py index d617ad7c0..af7be1ab7 100644 --- a/elasticapm/utils/compat.py +++ b/elasticapm/utils/compat.py @@ -107,7 +107,26 @@ def iteritems(d, **kwargs): def iterlists(d, **kw): return d.iterlists(**kw) - + def exec_(_code_, _globs_=None, _locs_=None): + """Execute code in a namespace.""" + if _globs_ is None: + frame = sys._getframe(1) + _globs_ = frame.f_globals + if _locs_ is None: + _locs_ = frame.f_locals + del frame + elif _locs_ is None: + _locs_ = _globs_ + exec("""exec _code_ in _globs_, _locs_""") + + exec_( + """def reraise(tp, value, tb=None): + try: + raise tp, value, tb + finally: + tb = None +""" + ) else: import io import queue # noqa F401 @@ -140,6 +159,17 @@ def iteritems(d, **kwargs): def iterlists(d, **kw): return iter(d.lists(**kw)) + def reraise(tp, value, tb=None): + try: + if value is None: + value = tp() + if value.__traceback__ is not tb: + raise value.with_traceback(tb) + raise value + finally: + value = None + tb = None + def get_default_library_patters(): """ From ebbf500bf0a7be94eaef795b7432867091acead3 Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Thu, 21 Nov 2019 00:06:18 +0100 Subject: [PATCH 5/6] ZeroDivisionError is different in Python 2 and Python 3 --- tests/contrib/django/test_commands.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/contrib/django/test_commands.py b/tests/contrib/django/test_commands.py index d2eb7910a..5b9814403 100644 --- a/tests/contrib/django/test_commands.py +++ b/tests/contrib/django/test_commands.py @@ -36,6 +36,7 @@ import pytest from elasticapm.conf import constants +from elasticapm.utils import compat def test_management_command(django_elasticapm_client): @@ -74,7 +75,7 @@ def test_management_command_other_error(django_elasticapm_client): exception = django_elasticapm_client.events[constants.ERROR][0] assert exception["culprit"] == "tests.contrib.django.testapp.management.commands.eapm_test_command.handle" - assert exception["exception"]["message"] == "ZeroDivisionError: division by zero" + assert exception["exception"]["message"].startswith("ZeroDivisionError:") assert exception["transaction_id"] == transaction["id"] From 45c81b853bd5ae9ce8db310352e267007eb5e85a Mon Sep 17 00:00:00 2001 From: Benjamin Wohlwend Date: Fri, 22 Nov 2019 15:08:50 +0100 Subject: [PATCH 6/6] Apply doc fixes from code review Co-Authored-By: Brandon Morelli --- docs/django.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/django.asciidoc b/docs/django.asciidoc index cf1b50f3d..2921a552f 100644 --- a/docs/django.asciidoc +++ b/docs/django.asciidoc @@ -94,11 +94,11 @@ database queries, HTTP requests, etc. You can find more information on what we instrument in the <> section. Lastly, the agent will also collect performance data for Django management commands. -You can disable instrumenting for certain commands using the +You can disable instrumentation for certain commands using the <> setting. Transactions for management commands can be accessed in the APM app in Kibana by choosing `django_command` in the "transaction type" filter. -NOTE: The agent collects command line arguments as additional meta data for transactions. +NOTE: The agent collects command line arguments as additional metadata for transactions. If you run a command that contains sensitive data on the command line, like tokens or passwords, we recommend to exclude that command from instrumentation.