Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added option to disable middleware spans in Starlette #3052

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions sentry_sdk/integrations/starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,15 @@ class StarletteIntegration(Integration):

transaction_style = ""

def __init__(self, transaction_style="url"):
# type: (str) -> None
def __init__(self, transaction_style="url", middleware_spans=True):
# type: (str, bool) -> None
if transaction_style not in TRANSACTION_STYLE_VALUES:
raise ValueError(
"Invalid value for transaction_style: %s (must be in %s)"
% (transaction_style, TRANSACTION_STYLE_VALUES)
)
self.transaction_style = transaction_style
self.middleware_spans = middleware_spans

@staticmethod
def setup_once():
Expand All @@ -105,7 +106,7 @@ def _enable_span_for_middleware(middleware_class):
async def _create_span_call(app, scope, receive, send, **kwargs):
# type: (Any, Dict[str, Any], Callable[[], Awaitable[Dict[str, Any]]], Callable[[Dict[str, Any]], Awaitable[None]], Any) -> None
integration = sentry_sdk.get_client().get_integration(StarletteIntegration)
if integration is None:
if integration is None or not integration.middleware_spans:
return await old_call(app, scope, receive, send, **kwargs)

middleware_name = app.__class__.__name__
Expand Down
37 changes: 33 additions & 4 deletions tests/integrations/starlette/test_starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,20 +636,49 @@ def test_middleware_spans(sentry_init, capture_events):

(_, transaction_event) = events

expected = [
expected_middleware_spans = [
"ServerErrorMiddleware",
"AuthenticationMiddleware",
"ExceptionMiddleware",
"AuthenticationMiddleware", # 'op': 'middleware.starlette.send'
"ServerErrorMiddleware", # 'op': 'middleware.starlette.send'
"AuthenticationMiddleware", # 'op': 'middleware.starlette.send'
"ServerErrorMiddleware", # 'op': 'middleware.starlette.send'
]

assert len(transaction_event["spans"]) == len(expected_middleware_spans)

idx = 0
for span in transaction_event["spans"]:
if span["op"] == "middleware.starlette":
assert span["description"] == expected[idx]
assert span["tags"]["starlette.middleware_name"] == expected[idx]
if span["op"].startswith("middleware.starlette"):
assert (
span["tags"]["starlette.middleware_name"]
== expected_middleware_spans[idx]
)
idx += 1
Comment on lines 651 to 658
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this idx stuff was already here, but maybe we can consider changing this for loop to use zip or enumerate instead, so we can get rid of it?



def test_middleware_spans_disabled(sentry_init, capture_events):
sentry_init(
traces_sample_rate=1.0,
integrations=[StarletteIntegration(middleware_spans=False)],
)
starlette_app = starlette_app_factory(
middleware=[Middleware(AuthenticationMiddleware, backend=BasicAuthBackend())]
)
events = capture_events()

client = TestClient(starlette_app, raise_server_exceptions=False)
try:
client.get("/message", auth=("Gabriela", "hello123"))
except Exception:
pass

(_, transaction_event) = events

assert len(transaction_event["spans"]) == 0


def test_middleware_callback_spans(sentry_init, capture_events):
sentry_init(
traces_sample_rate=1.0,
Expand Down