pr comments: Address comments for tracing module

- Make newrelic import explicit with clear error message for missing dependency
- Remove tracing re-exports from agave.core to avoid forcing newrelic installation
- Use Optional and Union for Python 3.9 compatibility instead of PEP 604 syntax and future module
- Add specific type hints dict[str, str] for trace headers
- Add trailing newline to requirements-test.txt

Author: dpastranak

agave/core/__init__.py

@@ -1,21 +1,3 @@
-from .tracing import (
-    TRACE_HEADERS_KEY,
-    accept_trace_from_queue,
-    accept_trace_headers,
-    add_custom_attribute,
-    background_task,
-    get_trace_headers,
-    inject_trace_headers,
-    trace_attributes,
-)
-
-__all__ = [
-    'TRACE_HEADERS_KEY',
-    'accept_trace_from_queue',
-    'accept_trace_headers',
-    'add_custom_attribute',
-    'background_task',
-    'get_trace_headers',
-    'inject_trace_headers',
-    'trace_attributes',
-]
+# Tracing utilities live in agave.core.tracing.
+# We don't export them here because we didn't want to force everyone to install newrelic.
+# Just import directly when needed from agave.core.tracing import ...

agave/core/tracing.py

@@ -1,11 +1,15 @@
-from __future__ import annotations
-
 import inspect
 from contextlib import contextmanager
 from functools import wraps
-from typing import Any, Callable, Optional
-
-import newrelic.agent
+from typing import Any, Callable, Optional, Union
+
+try:
+    import newrelic.agent
+except ImportError:  # pragma: no cover
+    raise ImportError(
+        "You must install agave with [tracing] option.\n"
+        "You can install it with: pip install agave[tracing]"
+    )
 
 # trace headers key
 TRACE_HEADERS_KEY = "_nr_trace_headers"
@@ -15,7 +19,7 @@
 def background_task(
     name: str,
     group: str = "Task",
-    trace_headers: Optional[dict] = None,
+    trace_headers: Optional[dict[str, str]] = None,
 ):
     with newrelic.agent.BackgroundTask(
         application=newrelic.agent.application(),
@@ -27,14 +31,14 @@ def background_task(
         yield
 
 
-def get_trace_headers() -> dict:
+def get_trace_headers() -> dict[str, str]:
     headers_list: list = []
     newrelic.agent.insert_distributed_trace_headers(headers_list)
     return dict(headers_list)
 
 
 def accept_trace_headers(
-    headers: dict | None, transport_type: str = "HTTP"
+    headers: Optional[dict[str, str]], transport_type: str = "HTTP"
 ) -> None:
     """
     Accept incoming trace headers to continue a distributed trace.
@@ -136,7 +140,7 @@ def sync_wrapper(*args, **kwargs):
     return decorator
 
 
-def trace_attributes(**extractors: Callable | str):
+def trace_attributes(**extractors: Union[Callable, str]):
     """
     Decorator to add custom attributes to New Relic traces.
 

requirements-test.txt

@@ -12,4 +12,4 @@ moto[server]==5.0.26
 pytest-vcr==1.0.2
 pytest-asyncio==0.18.*
 typing_extensions==4.12.2
-newrelic==11.2.0
+newrelic==11.2.0

tests/core/test_tracing.py

@@ -1,6 +1,5 @@
-from __future__ import annotations
-
 import asyncio
+from typing import Optional
 from unittest.mock import patch
 
 from agave.core.tracing import (
@@ -131,7 +130,7 @@ async def my_task(data: dict):
 
 def test_inject_trace_headers_async_keyword_arg():
     @inject_trace_headers()
-    async def my_func(_url: str, trace_headers: dict | None = None):
+    async def my_func(_url: str, trace_headers: Optional[dict] = None):
         return trace_headers
 
     with patch(
@@ -144,7 +143,7 @@ async def my_func(_url: str, trace_headers: dict | None = None):
 
 def test_inject_trace_headers_merges_existing_headers_async():
     @inject_trace_headers()
-    async def my_func(_url: str, trace_headers: dict | None = None):
+    async def my_func(_url: str, trace_headers: Optional[dict] = None):
         return trace_headers
 
     with patch(
@@ -161,7 +160,7 @@ def test_inject_trace_headers_handles_positional_headers_async():
     """Test that positional args don't cause 'multiple values' error."""
 
     @inject_trace_headers("headers")
-    async def my_func(_url: str, headers: dict | None = None):
+    async def my_func(_url: str, headers: Optional[dict] = None):
         return headers
 
     with patch(
@@ -177,7 +176,7 @@ async def my_func(_url: str, headers: dict | None = None):
 
 def test_inject_trace_headers_sync_keyword_arg():
     @inject_trace_headers()
-    def my_func(_url: str, trace_headers: dict | None = None):
+    def my_func(_url: str, trace_headers: Optional[dict] = None):
         return trace_headers
 
     with patch(
@@ -190,7 +189,7 @@ def my_func(_url: str, trace_headers: dict | None = None):
 
 def test_inject_trace_headers_merges_existing_headers_sync():
     @inject_trace_headers()
-    def my_func(_url: str, trace_headers: dict | None = None):
+    def my_func(_url: str, trace_headers: Optional[dict] = None):
         return trace_headers
 
     with patch(
@@ -207,7 +206,7 @@ def test_inject_trace_headers_handles_positional_headers_sync():
     """Test that positional args don't cause 'multiple values' error."""
 
     @inject_trace_headers("headers")
-    def my_func(_url: str, headers: dict | None = None):
+    def my_func(_url: str, headers: Optional[dict] = None):
         return headers
 
     with patch(
@@ -221,7 +220,7 @@ def my_func(_url: str, headers: dict | None = None):
 
 def test_inject_trace_headers_custom_param_name():
     @inject_trace_headers("custom_headers")
-    def my_func(_url: str, custom_headers: dict | None = None):
+    def my_func(_url: str, custom_headers: Optional[dict] = None):
         return custom_headers
 
     with patch(