Update Starlette transaction naming hierarchy.

Author: newrelic-opensource

newrelic/core/config.py

@@ -90,6 +90,21 @@ def create_settings(nested):
     return type('Settings', (Settings,), {'nested': nested})()
 
 
+class TopLevelSettings(Settings):
+    _host = None
+
+    @property
+    def host(self):
+        if self._host:
+            return self._host
+        else:
+            return default_host(self.license_key)
+
+    @host.setter
+    def host(self, value):
+        self._host = value
+
+
 class AttributesSettings(Settings):
     pass
 
@@ -303,7 +318,7 @@ class EventHarvestConfigHarvestLimitSettings(Settings):
     nested = True
 
 
-_settings = Settings()
+_settings = TopLevelSettings()
 _settings.attributes = AttributesSettings()
 _settings.thread_profiler = ThreadProfilerSettings()
 _settings.transaction_tracer = TransactionTracerSettings()
@@ -502,8 +517,7 @@ def default_host(license_key):
 
 _settings.ssl = _environ_as_bool('NEW_RELIC_SSL', True)
 
-_settings.host = os.environ.get('NEW_RELIC_HOST',
-        default_host(_settings.license_key))
+_settings.host = os.environ.get('NEW_RELIC_HOST')
 _settings.port = int(os.environ.get('NEW_RELIC_PORT', '0'))
 
 _settings.agent_run_id = None

newrelic/core/environment.py

@@ -185,16 +185,14 @@ def environment_settings():
 
     plugins = []
 
-    # Using six to create create a snapshot of sys.modules can occassionally
+    # Using any iterable to create a snapshot of sys.modules can occassionally
     # fail in a rare case when modules are imported in parallel by different
-    # threads. This is because list(six.iteritems(sys.modules)) results in
-    # list(iter(sys.modules.iteritems())), which means sys.modules could change
-    # between the time when the iterable is handed over from the iter() to
-    # list().
+    # threads.
     #
-    # TL;DR: Do NOT use six module for the following iteration.
+    # TL;DR: Do NOT use an iterable on the original sys.modules to generate the
+    # list
 
-    for name, module in list(sys.modules.items()):
+    for name, module in sys.modules.copy().items():
         # If the module isn't actually loaded (such as failed relative imports
         # in Python 2.7), the module will be None and should not be reported.
         if module is None:

newrelic/hooks/framework_starlette.py

@@ -51,7 +51,7 @@ def route_naming_wrapper(wrapped, instance, args, kwargs):
     with RequestContext(bind_request(*args, **kwargs)):
         transaction = current_transaction()
         if transaction:
-            transaction.set_transaction_name(callable_name(wrapped), priority=3)
+            transaction.set_transaction_name(callable_name(wrapped), priority=2)
         return wrapped(*args, **kwargs)
 
 
@@ -83,14 +83,21 @@ def wrap_background_method(wrapped, instance, args, kwargs):
     return wrapped(*args, **kwargs)
 
 
-@function_wrapper
-def wrap_middleware(wrapped, instance, args, kwargs):
-    result = wrapped(*args, **kwargs)
+async def middleware_wrapper(wrapped, instance, args, kwargs):
+    transaction = current_transaction()
+    if transaction:
+        transaction.set_transaction_name(callable_name(wrapped), priority=1)
 
-    dispatch_func = getattr(result, "dispatch_func", None)
+    dispatch_func = getattr(wrapped, "dispatch_func", None)
     name = dispatch_func and callable_name(dispatch_func)
 
-    return FunctionTraceWrapper(result, name=name)
+    return await FunctionTraceWrapper(wrapped, name=name)(*args, **kwargs)
+
+
+@function_wrapper
+def wrap_middleware(wrapped, instance, args, kwargs):
+    result = wrapped(*args, **kwargs)
+    return FunctionWrapper(result, middleware_wrapper)
 
 
 def bind_middleware(middleware_class, *args, **kwargs):
@@ -159,6 +166,14 @@ def wrap_add_exception_handler(wrapped, instance, args, kwargs):
     return wrapped(exc_class_or_status_code, handler, *args, **kwargs)
 
 
+def error_middleware_wrapper(wrapped, instance, args, kwargs):
+    transaction = current_transaction()
+    if transaction:
+        transaction.set_transaction_name(callable_name(wrapped), priority=1)
+
+    return FunctionTraceWrapper(wrapped)(*args, **kwargs)
+
+
 def instrument_starlette_applications(module):
     framework = framework_details()
     version_info = tuple(int(v) for v in framework[1].split(".", 3)[:3])
@@ -178,7 +193,7 @@ def instrument_starlette_requests(module):
 
 
 def instrument_starlette_middleware_errors(module):
-    wrap_function_trace(module, "ServerErrorMiddleware.__call__")
+    wrap_function_wrapper(module, "ServerErrorMiddleware.__call__", error_middleware_wrapper)
 
     wrap_function_wrapper(module, "ServerErrorMiddleware.__init__", wrap_server_error_handler)
 
@@ -188,7 +203,7 @@ def instrument_starlette_middleware_errors(module):
 
 
 def instrument_starlette_exceptions(module):
-    wrap_function_trace(module, "ExceptionMiddleware.__call__")
+    wrap_function_wrapper(module, "ExceptionMiddleware.__call__", error_middleware_wrapper)
 
     wrap_function_wrapper(module, "ExceptionMiddleware.http_exception",
         wrap_exception_handler)

tests/agent_unittests/test_environment.py

@@ -43,6 +43,37 @@ def test_plugin_list():
     assert "newrelic.hooks.newrelic" not in plugin_list
 
 
+class NoIteratorDict(object):
+    def __init__(self, d):
+        self.d = d
+
+    def copy(self):
+        return self.d.copy()
+
+    def get(self, *args, **kwargs):
+        return self.d.get(*args, **kwargs)
+
+    def __getitem__(self, *args, **kwargs):
+        return self.d.__getitem__(*args, **kwargs)
+
+    def __contains__(self, *args, **kwargs):
+        return self.d.__contains__(*args, **kwargs)
+
+
+def test_plugin_list_uses_no_sys_modules_iterator(monkeypatch):
+    modules = NoIteratorDict(sys.modules)
+    monkeypatch.setattr(sys, 'modules', modules)
+
+    # If environment_settings iterates over sys.modules, an attribute error will be generated
+    environment_info = environment_settings()
+
+    for key, plugin_list in environment_info:
+        if key == "Plugin List":
+            break
+    else:
+        assert False, "'Plugin List' not found"
+
+
 @pytest.mark.parametrize(
     "loaded_modules,dispatcher,dispatcher_version,worker_version",
     (

tests/agent_unittests/test_region_aware_settings.py

@@ -81,3 +81,19 @@ def test_region_aware_license_keys(ini, env, expected_host,
     settings = global_settings()
     assert settings.host == expected_host
     assert settings.license_key == expected_license_key
+
+
+@pytest.mark.parametrize("ini,license_key,host_override,expected_host", (
+    (INI_FILE_WITHOUT_LICENSE_KEY, NO_REGION_KEY, None, "collector.newrelic.com"),
+    (INI_FILE_WITHOUT_LICENSE_KEY, EU01_KEY, None, "collector.eu01.nr-data.net"),
+    (INI_FILE_WITHOUT_LICENSE_KEY, NO_REGION_KEY, "foo.newrelic.com", "foo.newrelic.com"),
+))
+def test_region_aware_global_settings(ini, license_key, host_override,
+        expected_host, global_settings):
+    settings = global_settings()
+
+    if host_override:
+        settings.host = host_override
+
+    settings.license_key = license_key
+    assert settings.host == expected_host