Skip to content

Commit

Permalink
Issue open-telemetry#1757 - Update HTTP server/client instrumentation…
Browse files Browse the repository at this point in the history 
… span names (open-telemetry#1759)

Co-authored-by: Shalev Roda <65566801+shalevr@users.noreply.github.com>
Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>
  • Loading branch information
@macieyng @shalevr @srikanthccv
3 people committed Jun 15, 2023
1 parent a5ed4da commit 743ac64
Show file tree
Hide file tree
Showing 30 changed files with 194 additions and 118 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix Flask instrumentation to only close the span if it was created by the same request context.
([#1692](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1692))

### Changed
- Update HTTP server/client instrumentation span names to comply with spec
([#1759](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1759)

## Version 1.17.0/0.38b0 (2023-03-22)

### Added
Expand Down
2 changes: 1 addition & 1 deletion ...strumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ async def on_request_start(
return

http_method = params.method.upper()
request_span_name = f"HTTP {http_method}"
request_span_name = f"{http_method}"
request_url = (
remove_url_credentials(trace_config_ctx.url_filter(params.url))
if callable(trace_config_ctx.url_filter)
Expand Down
13 changes: 7 additions & 6 deletions ...ion/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def test_status_codes(self):
self.assert_spans(
[
(
"HTTP GET",
"GET",
(span_status, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand Down Expand Up @@ -213,7 +213,7 @@ def strip_query_params(url: yarl.URL) -> str:
self.assert_spans(
[
(
"HTTP GET",
"GET",
(StatusCode.UNSET, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand Down Expand Up @@ -247,7 +247,7 @@ async def do_request(url):
self.assert_spans(
[
(
"HTTP GET",
"GET",
(expected_status, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand All @@ -274,7 +274,7 @@ async def request_handler(request):
self.assert_spans(
[
(
"HTTP GET",
"GET",
(StatusCode.ERROR, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand All @@ -301,7 +301,7 @@ async def request_handler(request):
self.assert_spans(
[
(
"HTTP GET",
"GET",
(StatusCode.ERROR, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand Down Expand Up @@ -338,7 +338,7 @@ async def do_request(url):
self.assert_spans(
[
(
"HTTP GET",
"GET",
(StatusCode.UNSET, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand Down Expand Up @@ -391,6 +391,7 @@ def test_instrument(self):
self.get_default_request(), self.URL, self.default_handler
)
span = self.assert_spans(1)
self.assertEqual("GET", span.name)
self.assertEqual("GET", span.attributes[SpanAttributes.HTTP_METHOD])
self.assertEqual(
f"http://{host}:{port}/test-path",
Expand Down
19 changes: 12 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 @@ -415,18 +415,23 @@ def set_status_code(span, status_code):


def get_default_span_details(scope: dict) -> Tuple[str, dict]:
"""Default implementation for get_default_span_details
"""
Default span name is the HTTP method and URL path, or just the method.
https://github.com/open-telemetry/opentelemetry-specification/pull/3165
https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#name
Args:
scope: the ASGI scope dictionary
Returns:
a tuple of the span name, and any attributes to attach to the span.
"""
span_name = (
scope.get("path", "").strip()
or f"HTTP {scope.get('method', '').strip()}"
)

return span_name, {}
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


def _collect_target_attribute(
Expand Down
14 changes: 7 additions & 7 deletions instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,25 +142,25 @@ def validate_outputs(self, outputs, error=None, modifiers=None):
self.assertEqual(len(span_list), 4)
expected = [
{
"name": "/ http receive",
"name": "GET / http receive",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"type": "http.request"},
},
{
"name": "/ http send",
"name": "GET / http send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {
SpanAttributes.HTTP_STATUS_CODE: 200,
"type": "http.response.start",
},
},
{
"name": "/ http send",
"name": "GET / http send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {"type": "http.response.body"},
},
{
"name": "/",
"name": "GET /",
"kind": trace_api.SpanKind.SERVER,
"attributes": {
SpanAttributes.HTTP_METHOD: "GET",
Expand Down Expand Up @@ -231,7 +231,7 @@ def update_expected_span_name(expected):
entry["name"] = span_name
else:
entry["name"] = " ".join(
[span_name] + entry["name"].split(" ")[1:]
[span_name] + entry["name"].split(" ")[2:]
)
return expected

Expand Down Expand Up @@ -493,9 +493,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"] == "/ http receive":
elif entry["name"] == "GET / http receive":
entry["name"] = "name from client request hook"
elif entry["name"] == "/ http send":
elif entry["name"] == "GET / http send":
entry["attributes"].update({"attr-from-hook": "value"})
return expected

Expand Down
12 changes: 3 additions & 9 deletions ...rumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,18 +173,12 @@ def _get_span_name(request):
match = resolve(request.path)

if hasattr(match, "route"):
return match.route
return f"{request.method} {match.route}"

# Instead of using `view_name`, better to use `_func_name` as some applications can use similar
# view names in different modules
if hasattr(match, "_func_name"):
return match._func_name # pylint: disable=protected-access

# Fallback for safety as `_func_name` private field
return match.view_name
return request.method

except Resolver404:
return f"HTTP {request.method}"
return request.method

# pylint: disable=too-many-locals
def process_request(self, request):
Expand Down
26 changes: 8 additions & 18 deletions instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,9 @@ def test_templated_route_get(self):

self.assertEqual(
span.name,
"^route/(?P<year>[0-9]{4})/template/$"
"GET ^route/(?P<year>[0-9]{4})/template/$"
if DJANGO_2_2
else "tests.views.traced_template",
else "GET",
)
self.assertEqual(span.kind, SpanKind.SERVER)
self.assertEqual(span.status.status_code, StatusCode.UNSET)
Expand All @@ -177,9 +177,7 @@ def test_traced_get(self):

span = spans[0]

self.assertEqual(
span.name, "^traced/" if DJANGO_2_2 else "tests.views.traced"
)
self.assertEqual(span.name, "GET ^traced/" if DJANGO_2_2 else "GET")
self.assertEqual(span.kind, SpanKind.SERVER)
self.assertEqual(span.status.status_code, StatusCode.UNSET)
self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET")
Expand Down Expand Up @@ -215,9 +213,7 @@ def test_traced_post(self):

span = spans[0]

self.assertEqual(
span.name, "^traced/" if DJANGO_2_2 else "tests.views.traced"
)
self.assertEqual(span.name, "POST ^traced/" if DJANGO_2_2 else "POST")
self.assertEqual(span.kind, SpanKind.SERVER)
self.assertEqual(span.status.status_code, StatusCode.UNSET)
self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST")
Expand All @@ -241,9 +237,7 @@ def test_error(self):

span = spans[0]

self.assertEqual(
span.name, "^error/" if DJANGO_2_2 else "tests.views.error"
)
self.assertEqual(span.name, "GET ^error/" if DJANGO_2_2 else "GET")
self.assertEqual(span.kind, SpanKind.SERVER)
self.assertEqual(span.status.status_code, StatusCode.ERROR)
self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET")
Expand Down Expand Up @@ -307,9 +301,7 @@ def test_span_name(self):
span = span_list[0]
self.assertEqual(
span.name,
"^span_name/([0-9]{4})/$"
if DJANGO_2_2
else "tests.views.route_span_name",
"GET ^span_name/([0-9]{4})/$" if DJANGO_2_2 else "GET",
)

def test_span_name_for_query_string(self):
Expand All @@ -323,9 +315,7 @@ def test_span_name_for_query_string(self):
span = span_list[0]
self.assertEqual(
span.name,
"^span_name/([0-9]{4})/$"
if DJANGO_2_2
else "tests.views.route_span_name",
"GET ^span_name/([0-9]{4})/$" if DJANGO_2_2 else "GET",
)

def test_span_name_404(self):
Expand All @@ -334,7 +324,7 @@ def test_span_name_404(self):
self.assertEqual(len(span_list), 1)

span = span_list[0]
self.assertEqual(span.name, "HTTP GET")
self.assertEqual(span.name, "GET")

def test_traced_request_attrs(self):
Client().get("/span_name/1234/", CONTENT_TYPE="test/ct")
Expand Down
14 changes: 7 additions & 7 deletions instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async def test_templated_route_get(self):

span = spans[0]

self.assertEqual(span.name, "^route/(?P<year>[0-9]{4})/template/$")
self.assertEqual(span.name, "GET ^route/(?P<year>[0-9]{4})/template/$")
self.assertEqual(span.kind, SpanKind.SERVER)
self.assertEqual(span.status.status_code, StatusCode.UNSET)
self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET")
Expand All @@ -160,7 +160,7 @@ async def test_traced_get(self):

span = spans[0]

self.assertEqual(span.name, "^traced/")
self.assertEqual(span.name, "GET ^traced/")
self.assertEqual(span.kind, SpanKind.SERVER)
self.assertEqual(span.status.status_code, StatusCode.UNSET)
self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET")
Expand Down Expand Up @@ -195,7 +195,7 @@ async def test_traced_post(self):

span = spans[0]

self.assertEqual(span.name, "^traced/")
self.assertEqual(span.name, "POST ^traced/")
self.assertEqual(span.kind, SpanKind.SERVER)
self.assertEqual(span.status.status_code, StatusCode.UNSET)
self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST")
Expand All @@ -218,7 +218,7 @@ async def test_error(self):

span = spans[0]

self.assertEqual(span.name, "^error/")
self.assertEqual(span.name, "GET ^error/")
self.assertEqual(span.kind, SpanKind.SERVER)
self.assertEqual(span.status.status_code, StatusCode.ERROR)
self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET")
Expand Down Expand Up @@ -264,7 +264,7 @@ async def test_span_name(self):
self.assertEqual(len(span_list), 1)

span = span_list[0]
self.assertEqual(span.name, "^span_name/([0-9]{4})/$")
self.assertEqual(span.name, "GET ^span_name/([0-9]{4})/$")

async def test_span_name_for_query_string(self):
"""
Expand All @@ -275,15 +275,15 @@ async def test_span_name_for_query_string(self):
self.assertEqual(len(span_list), 1)

span = span_list[0]
self.assertEqual(span.name, "^span_name/([0-9]{4})/$")
self.assertEqual(span.name, "GET ^span_name/([0-9]{4})/$")

async def test_span_name_404(self):
await self.async_client.get("/span_name/1234567890/")
span_list = self.memory_exporter.get_finished_spans()
self.assertEqual(len(span_list), 1)

span = span_list[0]
self.assertEqual(span.name, "HTTP GET")
self.assertEqual(span.name, "GET")

async def test_traced_request_attrs(self):
await self.async_client.get("/span_name/1234/", CONTENT_TYPE="test/ct")
Expand Down
2 changes: 1 addition & 1 deletion instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def test_404(self):
spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1)
span = spans[0]
self.assertEqual(span.name, "HTTP GET")
self.assertEqual(span.name, "GET /does-not-exist")
self.assertEqual(span.status.status_code, StatusCode.UNSET)
self.assertSpanHasAttributes(
span,
Expand Down
Loading

0 comments on commit 743ac64

Please sign in to comment.