Repository: scoutapp/scout_apm_python (python)

Issue:
Require Celery app reference to read configuration
We had a customer whose Celery tasks weren't reporting whilst their Django views were. It turns out they had configured in the Django settings file, which isn't applied when Celery runs. This is because it doesn't run "under" Django through `manage.py`, but separately through `celery worker`.

The django pattern is to use [Celery's `app.config_from_object`](https://docs.celeryproject.org/en/latest/reference/celery.html#celery.Celery.config_from_object) to read the Django settings. If we then read out of there for the scout settings, we would again allow shared configuration between the two.

This would need changing the Celery install process to take an `app` argument:

```python
app = celery.Celery(..)
...
scout_apm.celery.install(app)
```

We should work without this for backwards compatibility reasons, but throw a warninng when it's not passed as I predict this issue will appear repeatedly if we don't encourage users this way.

Pull request description:
Copy configuration from Celery app
Fixes #477.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 23ef32a..1bbaeed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,11 +2,20 @@
 
 ## Pending
 
+### Added
+
+- Added ability to read configuration from your Celery app. This can help share
+  configuration between Django and Celery. Pass your application as `app` to
+  have the configuration copied: `scout_apm.celery.install(app=app)`
+  ([Issue #477](https://github.com/scoutapp/scout_apm_python/issues/477)).
+
 ### Fixed
 
 - Made installation of the objtrace C extension optional, so that if it fails
   due to your C compiler being old, Scout can still install
   ([Issue #488](https://github.com/scoutapp/scout_apm_python/issues/488)).
+- Removed warning for setting the `SCOUT_LOG_LEVEL` setting, since this is set
+  by default by the Heroku add-on.
 
 ## [2.11.0] 2020-02-17
 
diff --git a/src/scout_apm/celery.py b/src/scout_apm/celery.py
index e65aac1..e52c1c8 100644
--- a/src/scout_apm/celery.py
+++ b/src/scout_apm/celery.py
@@ -7,15 +7,16 @@ from celery.signals import before_task_publish, task_postrun, task_prerun
 
 import scout_apm.core
 from scout_apm.compat import datetime_to_timestamp
+from scout_apm.core.config import scout_config
 from scout_apm.core.tracked_request import TrackedRequest
 
 
-def before_publish_callback(headers=None, properties=None, **kwargs):
+def before_task_publish_callback(headers=None, properties=None, **kwargs):
     if "scout_task_start" not in headers:
         headers["scout_task_start"] = datetime_to_timestamp(dt.datetime.utcnow())
 
 
-def prerun_callback(task=None, **kwargs):
+def task_prerun_callback(task=None, **kwargs):
     tracked_request = TrackedRequest.instance()
     tracked_request.is_real_request = True
 
@@ -45,22 +46,39 @@ def prerun_callback(task=None, **kwargs):
     tracked_request.start_span(operation=("Job/" + task.name))
 
 
-def postrun_callback(task=None, **kwargs):
+def task_postrun_callback(task=None, **kwargs):
     tracked_request = TrackedRequest.instance()
     tracked_request.stop_span()
 
 
-def install():
+def install(app=None):
+    if app is not None:
+        copy_configuration(app)
+
     installed = scout_apm.core.install()
     if not installed:
         return
 
-    before_task_publish.connect(before_publish_callback)
-    task_prerun.connect(prerun_callback)
-    task_postrun.connect(postrun_callback)
+    before_task_publish.connect(before_task_publish_callback)
+    task_prerun.connect(task_prerun_callback)
+    task_postrun.connect(task_postrun_callback)
+
+
+def copy_configuration(app):
+    prefix = "scout_"
+    prefix_len = len(prefix)
+
+    to_set = {}
+    for key, value in app.conf.items():
+        key_lower = key.lower()
+        if key_lower.startswith(prefix) and len(key_lower) > prefix_len:
+            scout_key = key_lower[prefix_len:]
+            to_set[scout_key] = value
+
+    scout_config.set(**to_set)
 
 
 def uninstall():
-    before_task_publish.disconnect(before_publish_callback)
-    task_prerun.disconnect(prerun_callback)
-    task_postrun.disconnect(postrun_callback)
+    before_task_publish.disconnect(before_task_publish_callback)
+    task_prerun.disconnect(task_prerun_callback)
+    task_postrun.disconnect(task_postrun_callback)
diff --git a/src/scout_apm/core/config.py b/src/scout_apm/core/config.py
index a2f73a2..69c5321 100644
--- a/src/scout_apm/core/config.py
+++ b/src/scout_apm/core/config.py
@@ -218,7 +218,7 @@ class Defaults(object):
             "hostname": None,
             "key": "",
             "monitor": False,
-            "name": "",
+            "name": "Python App",
             "revision_sha": self._git_revision_sha(),
             "scm_subdirectory": "",
             "shutdown_timeout_seconds": 2.0,
diff --git a/src/scout_apm/core/core_agent_manager.py b/src/scout_apm/core/core_agent_manager.py
index df0eb50..56f2c5d 100644
--- a/src/scout_apm/core/core_agent_manager.py
+++ b/src/scout_apm/core/core_agent_manager.py
@@ -8,7 +8,6 @@ import os
 import subprocess
 import tarfile
 import time
-import warnings
 
 from urllib3.exceptions import HTTPError
 
@@ -89,15 +88,7 @@ class CoreAgentManager(object):
     def log_level(self):
         # Old deprecated name "log_level"
         log_level = scout_config.value("log_level")
-        if log_level is not None:
-            warnings.warn(
-                "The config name 'log_level' is deprecated - "
-                + "please use the new name 'core_agent_log_level' instead. "
-                + "This might be configured in your environment variables or "
-                + "framework settings as SCOUT_LOG_LEVEL.",
-                DeprecationWarning,
-            )
-        else:
+        if log_level is None:
             log_level = scout_config.value("core_agent_log_level")
         return ["--log-level", log_level]
 


Test patch:
diff --git a/tests/compat.py b/tests/compat.py
index 80e72a3..7b9c199 100644
--- a/tests/compat.py
+++ b/tests/compat.py
@@ -3,6 +3,16 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 
 import sys
 
+if sys.version_info >= (3, 0):
+    from types import SimpleNamespace
+else:
+
+    class SimpleNamespace(object):
+        def __init__(self, **kwargs):
+            for key, value in kwargs.items():
+                setattr(self, key, value)
+
+
 if sys.version_info >= (3, 0):
     from unittest import mock
 else:
diff --git a/tests/integration/core/test_core_agent_manager.py b/tests/integration/core/test_core_agent_manager.py
index e44a6a1..4f4772d 100644
--- a/tests/integration/core/test_core_agent_manager.py
+++ b/tests/integration/core/test_core_agent_manager.py
@@ -100,25 +100,17 @@ def test_launch_error(caplog, core_agent_manager):
     assert caplog.records[0].exc_info[1] is exception
 
 
-def test_log_level(caplog, core_agent_manager):
+def test_log_level(core_agent_manager):
     scout_config.set(core_agent_log_level="foo")
 
     result = core_agent_manager.log_level()
 
     assert result == ["--log-level", "foo"]
-    assert caplog.record_tuples == []
 
 
-def test_log_level_deprecated(core_agent_manager, recwarn):
+def test_log_level_old_name_takes_precedence(core_agent_manager):
     scout_config.set(log_level="foo", core_agent_log_level="bar")
 
     result = core_agent_manager.log_level()
 
     assert result == ["--log-level", "foo"]
-    assert len(recwarn) == 1
-    warning = recwarn.pop(DeprecationWarning)
-    assert str(warning.message) == (
-        "The config name 'log_level' is deprecated - please use the new name "
-        + "'core_agent_log_level' instead. This might be configured in your "
-        + "environment variables or framework settings as SCOUT_LOG_LEVEL."
-    )
diff --git a/tests/integration/test_celery.py b/tests/integration/test_celery.py
index c2dcb53..2004de9 100644
--- a/tests/integration/test_celery.py
+++ b/tests/integration/test_celery.py
@@ -10,6 +10,8 @@ from celery.signals import setup_logging
 import scout_apm.celery
 from scout_apm.api import Config
 from scout_apm.compat import kwargs_only
+from scout_apm.core.config import scout_config
+from tests.compat import SimpleNamespace
 
 # http://docs.celeryproject.org/en/latest/userguide/testing.html#py-test
 skip_unless_celery_4_plus = pytest.mark.skipif(
@@ -27,13 +29,16 @@ def do_nothing(**kwargs):
 
 @contextmanager
 @kwargs_only
-def app_with_scout(app=None, config=None):
+def app_with_scout(celery_config=None, app=None, config=None):
     """
     Context manager that configures a Celery app with Scout installed.
     """
     if app is None:
         app = celery.Celery("tasks", broker="memory://")
 
+    if celery_config is not None:
+        app.config_from_object(celery_config)
+
     # Enable Scout by default in tests.
     if config is None:
         config = {"monitor": True}
@@ -47,7 +52,7 @@ def app_with_scout(app=None, config=None):
 
     # Setup according to https://docs.scoutapm.com/#celery
     Config.set(**config)
-    scout_apm.celery.install()
+    scout_apm.celery.install(app)
 
     try:
         yield app
@@ -57,6 +62,12 @@ def app_with_scout(app=None, config=None):
         Config.reset_all()
 
 
+def test_configuration_copied():
+    celery_config = SimpleNamespace(SCOUT_IGNORE=["/foobar/"])
+    with app_with_scout(celery_config=celery_config):
+        assert scout_config.value("ignore") == ["/foobar/"]
+
+
 def test_hello_eager(tracked_requests):
     with app_with_scout() as app:
         result = app.tasks["tests.integration.test_celery.hello"].apply()