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

Use semantic attributes in traces sampler for ASGI spans #3774

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
18103bc
Remove custom_sampling_context
sentrivana Nov 7, 2024
68402ff
confusing wording
sentrivana Nov 7, 2024
70d02cc
Merge branch 'potel-base' into ivana/sampling-context
sentrivana Nov 7, 2024
fa5d0ba
keep old format (why not)
sentrivana Nov 7, 2024
5df9ccc
.
sentrivana Nov 7, 2024
472ab5c
add attrs
sentrivana Nov 7, 2024
f893f12
.
sentrivana Nov 7, 2024
fff9f16
.
sentrivana Nov 7, 2024
ce815e8
more removals
sentrivana Nov 7, 2024
d307fe1
Merge branch 'potel-base' into ivana/sampling-context
sentrivana Nov 12, 2024
747ecc6
more readme
sentrivana Nov 12, 2024
7ae6745
more info
sentrivana Nov 12, 2024
09d247e
comment
sentrivana Nov 12, 2024
1aeeb68
small change
sentrivana Nov 12, 2024
786bb9e
.
sentrivana Nov 12, 2024
80427c6
dont do everything in this pr
sentrivana Nov 12, 2024
2acef08
get rid of none attributes
sentrivana Nov 12, 2024
eb2df70
dropped result too
sentrivana Nov 12, 2024
2cee373
Replace custom_sampling_context with attributes in ASGI
sentrivana Nov 12, 2024
f6fb9a9
add source too
sentrivana Nov 12, 2024
86c6373
Merge branch 'potel-base' into ivana/custom-sampling-context-integrat…
sentrivana Nov 12, 2024
4002f38
fix starlette tests
sentrivana Nov 12, 2024
dbc8ba2
fix fastapi
sentrivana Nov 12, 2024
52f918a
remove middleware spans toggle
sentrivana Nov 13, 2024
6b063cf
Merge branch 'potel-base' into ivana/custom-sampling-context-integrat…
sentrivana Nov 13, 2024
f69b800
better attrs
sentrivana Nov 13, 2024
2ed3860
rename
sentrivana Nov 13, 2024
a684a7c
Merge branch 'potel-base' into ivana/custom-sampling-context-integrat…
sentrivana Nov 14, 2024
f674be1
Merge branch 'potel-base' into ivana/custom-sampling-context-integrat…
sentrivana Nov 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,18 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
- clickhouse-driver integration: The query is now available under the `db.query.text` span attribute (only if `send_default_pii` is `True`).
- `sentry_sdk.init` now returns `None` instead of a context manager.
- The `sampling_context` argument of `traces_sampler` now additionally contains all span attributes known at span start.
- The `sampling_context` argument of `traces_sampler` doesn't contain the `asgi_scope` object anymore for ASGI frameworks. Instead, the individual properties, if available, are accessible as `asgi_scope.endpoint`, `asgi_scope.path`, `asgi_scope.root_path`, `asgi_scope.route`, `asgi_scope.scheme`, `asgi_scope.server` and `asgi_scope.type`.
- The `sampling_context` argument of `traces_sampler` doesn't contain the `asgi_scope` object anymore for ASGI frameworks. Instead, the individual properties on the scope, if available, are accessible as follows:

| Scope property | Sampling context key(s) |
| -------------- | ------------------------------- |
| `type` | `network.protocol.name` |
| `scheme` | `url.scheme` |
| `path` | `url.path` |
| `http_version` | `network.protocol.version` |
| `method` | `http.request.method` |
| `server` | `server.address`, `server.port` |
| `client` | `client.address`, `client.port` |
| full URL | `url.full` |

### Removed

Expand Down
4 changes: 2 additions & 2 deletions sentry_sdk/integrations/_asgi_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ def _get_headers(asgi_scope):
return headers


def _get_url(asgi_scope, default_scheme, host):
# type: (Dict[str, Any], Literal["ws", "http"], Optional[Union[AnnotatedValue, str]]) -> str
def _get_url(asgi_scope, default_scheme=None, host=None):
# type: (Dict[str, Any], Optional[Literal["ws", "http"]], Optional[Union[AnnotatedValue, str]]) -> str
"""
Extract URL from the ASGI scope, without also including the querystring.
"""
Expand Down
48 changes: 37 additions & 11 deletions sentry_sdk/integrations/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from sentry_sdk.integrations._asgi_common import (
_get_headers,
_get_query,
_get_request_data,
_get_url,
)
Expand Down Expand Up @@ -57,6 +58,14 @@

TRANSACTION_STYLE_VALUES = ("endpoint", "url")

ASGI_SCOPE_PROPERTY_TO_ATTRIBUTE = {
"http_version": "network.protocol.version",
"method": "http.request.method",
"path": "url.path",
"scheme": "url.scheme",
"type": "network.protocol.name",
}


def _capture_exception(exc, mechanism_type="asgi"):
# type: (Any, str) -> None
Expand Down Expand Up @@ -213,23 +222,21 @@ async def _run_app(self, scope, receive, send, asgi_version):
)
if should_trace
else nullcontext()
) as transaction:
if transaction is not None:
logger.debug(
"[ASGI] Started transaction: %s", transaction
)
transaction.set_tag("asgi.type", ty)
) as span:
if span is not None:
logger.debug("[ASGI] Started transaction: %s", span)
span.set_tag("asgi.type", ty)
try:

async def _sentry_wrapped_send(event):
# type: (Dict[str, Any]) -> Any
is_http_response = (
event.get("type") == "http.response.start"
and transaction is not None
and span is not None
and "status" in event
)
if is_http_response:
transaction.set_http_status(event["status"])
span.set_http_status(event["status"])

return await send(event)

Expand Down Expand Up @@ -328,12 +335,31 @@ def _get_transaction_name_and_source(self, transaction_style, asgi_scope):

def _prepopulate_attributes(scope):
# type: (Any) -> dict[str, Any]
"""Unpack asgi_scope into serializable attributes."""
"""Unpack ASGI scope into serializable OTel attributes."""
scope = scope or {}

attributes = {}
for attr in ("endpoint", "path", "root_path", "route", "scheme", "server", "type"):
for attr, key in ASGI_SCOPE_PROPERTY_TO_ATTRIBUTE.items():
if scope.get(attr):
attributes[f"asgi_scope.{attr}"] = scope[attr]
attributes[key] = scope[attr]

for attr in ("client", "server"):
if scope.get(attr):
try:
host, port = scope[attr]
attributes[f"{attr}.address"] = host
attributes[f"{attr}.port"] = port
except Exception:
pass

try:
full_url = _get_url(scope)
query = _get_query(scope)
if query:
full_url = f"{full_url}?{query}"

attributes["url.full"] = full_url
except Exception:
pass

return attributes
10 changes: 7 additions & 3 deletions tests/integrations/asgi/test_asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -726,8 +726,12 @@ async def test_custom_transaction_name(
@pytest.mark.asyncio
async def test_asgi_scope_in_traces_sampler(sentry_init, asgi3_app):
def dummy_traces_sampler(sampling_context):
assert sampling_context["asgi_scope.path"] == "/test"
assert sampling_context["asgi_scope.scheme"] == "http"
assert sampling_context["url.path"] == "/test"
assert sampling_context["url.scheme"] == "http"
assert sampling_context["url.full"] == "/test?hello=there"
assert sampling_context["http.request.method"] == "GET"
assert sampling_context["network.protocol.version"] == "1.1"
assert sampling_context["network.protocol.name"] == "http"

sentry_init(
traces_sampler=dummy_traces_sampler,
Expand All @@ -737,4 +741,4 @@ def dummy_traces_sampler(sampling_context):
app = SentryAsgiMiddleware(asgi3_app)

async with TestClient(app) as client:
await client.get("/test")
await client.get("/test?hello=there")
3 changes: 0 additions & 3 deletions tests/integrations/fastapi/test_fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ async def _error(request: Request):
assert event["request"]["headers"]["authorization"] == "[Filtered]"


@pytest.mark.asyncio
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pytest was complaining that these 3 test cases shouldn't have @pytest.mark.asyncio

def test_response_status_code_ok_in_transaction_context(sentry_init, capture_envelopes):
"""
Tests that the response status code is added to the transaction "response" context.
Expand Down Expand Up @@ -275,7 +274,6 @@ def test_response_status_code_ok_in_transaction_context(sentry_init, capture_env
assert transaction["contexts"]["response"]["status_code"] == 200


@pytest.mark.asyncio
def test_response_status_code_error_in_transaction_context(
sentry_init,
capture_envelopes,
Expand Down Expand Up @@ -312,7 +310,6 @@ def test_response_status_code_error_in_transaction_context(
assert transaction["contexts"]["response"]["status_code"] == 500


@pytest.mark.asyncio
def test_response_status_code_not_found_in_transaction_context(
sentry_init,
capture_envelopes,
Expand Down
Loading