Skip to content

Commit

Permalink
fix(instrumentation-asgi): remove high cardinal path from span name
Browse files Browse the repository at this point in the history
  • Loading branch information
@srikanthccv
srikanthccv committed Jul 2, 2024
1 parent 529178d commit 076144c
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 23 deletions.
10 changes: 3 additions & 7 deletions ...ion/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,13 +403,9 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]:
Returns:
a tuple of the span name, and any attributes to attach to the span.
"""
path = scope.get("path", "").strip()
method = scope.get("method", "").strip()
if method and path: # http
return f"{method} {path}", {}
if path: # websocket
return path, {}
return method, {} # http with no path
if scope.get("type") == "http":
return scope.get("method", ""), {}
return scope.get("type", ""), {}


def _collect_target_attribute(
Expand Down
136 changes: 120 additions & 16 deletions instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,53 @@ async def error_asgi(scope, receive, send):
await send({"type": "http.response.body", "body": b"*"})


# New ASGI app for user update
async def user_update_app(scope, receive, send):
assert scope["type"] == "http"
assert scope["method"] == "PUT"
assert scope["path"].startswith("/api/v3/io/users/")

user_id = scope["path"].split("/")[-1]

message = await receive()
if message["type"] == "http.request":
await send(
{
"type": "http.response.start",
"status": 200,
"headers": [
[b"Content-Type", b"application/json"],
],
}
)
await send(
{
"type": "http.response.body",
"body": f'{{"status": "updated", "user_id": "{user_id}"}}'.encode(),
}
)


# New ASGI app for WebSocket
async def websocket_session_app(scope, receive, send):
assert scope["type"] == "websocket"
assert scope["path"].startswith("/ws/")

session_id = scope["path"].split("/")[-1]

await send({"type": "websocket.accept"})

while True:
event = await receive()
if event["type"] == "websocket.disconnect":
break
if event["type"] == "websocket.receive":
if event.get("text") == "ping":
await send(
{"type": "websocket.send", "text": f"pong:{session_id}"}
)


# pylint: disable=too-many-public-methods
class TestAsgiApplication(AsgiTestBase):
def validate_outputs(self, outputs, error=None, modifiers=None):
Expand Down Expand Up @@ -266,25 +313,25 @@ def validate_outputs(self, outputs, error=None, modifiers=None):
span_list = self.memory_exporter.get_finished_spans()
expected = [
{
"name": "GET / http receive",
"name": "GET http receive",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"asgi.event.type": "http.request"},
},
{
"name": "GET / http send",
"name": "GET http send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {
SpanAttributes.HTTP_STATUS_CODE: 200,
"asgi.event.type": "http.response.start",
},
},
{
"name": "GET / http send",
"name": "GET http send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"asgi.event.type": "http.response.body"},
},
{
"name": "GET /",
"name": "GET",
"kind": trace_api.SpanKind.SERVER,
"attributes": {
SpanAttributes.HTTP_METHOD: "GET",
Expand Down Expand Up @@ -360,7 +407,7 @@ def test_long_response(self):

def add_more_body_spans(expected: list):
more_body_span = {
"name": "GET / http send",
"name": "GET http send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"asgi.event.type": "http.response.body"},
}
Expand Down Expand Up @@ -398,12 +445,12 @@ def test_trailers(self):

def add_body_and_trailer_span(expected: list):
body_span = {
"name": "GET / http send",
"name": "GET http send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"asgi.event.type": "http.response.body"},
}
trailer_span = {
"name": "GET / http send",
"name": "GET http send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"asgi.event.type": "http.response.trailers"},
}
Expand Down Expand Up @@ -434,7 +481,7 @@ def update_expected_span_name(expected):
entry["name"] = span_name
else:
entry["name"] = " ".join(
[span_name] + entry["name"].split(" ")[2:]
[span_name] + entry["name"].split(" ")[1:]
)
return expected

Expand Down Expand Up @@ -584,38 +631,38 @@ def test_websocket(self):
self.assertEqual(len(span_list), 6)
expected = [
{
"name": "/ websocket receive",
"name": "websocket websocket receive",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"asgi.event.type": "websocket.connect"},
},
{
"name": "/ websocket send",
"name": "websocket websocket send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"asgi.event.type": "websocket.accept"},
},
{
"name": "/ websocket receive",
"name": "websocket websocket receive",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {
"asgi.event.type": "websocket.receive",
SpanAttributes.HTTP_STATUS_CODE: 200,
},
},
{
"name": "/ websocket send",
"name": "websocket websocket send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {
"asgi.event.type": "websocket.send",
SpanAttributes.HTTP_STATUS_CODE: 200,
},
},
{
"name": "/ websocket receive",
"name": "websocket websocket receive",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"asgi.event.type": "websocket.disconnect"},
},
{
"name": "/",
"name": "websocket",
"kind": trace_api.SpanKind.SERVER,
"attributes": {
SpanAttributes.HTTP_SCHEME: self.scope["scheme"],
Expand Down Expand Up @@ -697,9 +744,9 @@ def update_expected_hook_results(expected):
for entry in expected:
if entry["kind"] == trace_api.SpanKind.SERVER:
entry["name"] = "name from server hook"
elif entry["name"] == "GET / http receive":
elif entry["name"] == "GET http receive":
entry["name"] = "name from client request hook"
elif entry["name"] == "GET / http send":
elif entry["name"] == "GET http send":
entry["attributes"].update({"attr-from-hook": "value"})
return expected

Expand Down Expand Up @@ -851,6 +898,63 @@ def test_no_metric_for_websockets(self):
self.get_all_output()
self.assertIsNone(self.memory_metrics_reader.get_metrics_data())

def test_put_request_with_user_id(self):
app = otel_asgi.OpenTelemetryMiddleware(user_update_app)
self.seed_app(app)
self.scope["method"] = "PUT"
self.scope["path"] = "/api/v3/io/users/123"
self.send_input(
{"type": "http.request", "body": b'{"name": "John Doe"}'}
)

outputs = self.get_all_output()
self.assertEqual(len(outputs), 2)
self.assertEqual(outputs[0]["type"], "http.response.start")
self.assertEqual(outputs[0]["status"], 200)
self.assertEqual(outputs[1]["type"], "http.response.body")
self.assertEqual(
outputs[1]["body"], b'{"status": "updated", "user_id": "123"}'
)

span_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(span_list), 4) # 3 internal spans + 1 server span
server_span = span_list[-1]
self.assertEqual(server_span.name, "PUT")
self.assertEqual(server_span.kind, trace_api.SpanKind.SERVER)
self.assertEqual(
server_span.attributes[SpanAttributes.HTTP_METHOD], "PUT"
)
self.assertEqual(
server_span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200
)

def skip_test_websocket_connection_with_session_id(self):
app = otel_asgi.OpenTelemetryMiddleware(websocket_session_app)
self.seed_app(app)
self.scope["type"] = "websocket"
self.scope["path"] = "/ws/05b55f3f66aa31cbe6a25e7027f7c2cc"

self.send_input({"type": "websocket.connect"})
self.send_input({"type": "websocket.receive", "text": "ping"})
self.send_input({"type": "websocket.disconnect"})

outputs = self.get_all_output()
self.assertEqual(len(outputs), 2)
self.assertEqual(outputs[0]["type"], "websocket.accept")
self.assertEqual(outputs[1]["type"], "websocket.send")
self.assertEqual(
outputs[1]["text"], "pong:05b55f3f66aa31cbe6a25e7027f7c2cc"
)

span_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(span_list), 6) # 5 internal spans + 1 server span
server_span = span_list[-1]
self.assertEqual(server_span.name, "websocket")
self.assertEqual(server_span.kind, trace_api.SpanKind.SERVER)
self.assertEqual(
server_span.attributes[SpanAttributes.HTTP_SCHEME], "ws"
)


class TestAsgiAttributes(unittest.TestCase):
def setUp(self):
Expand Down

0 comments on commit 076144c

Please sign in to comment.