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

Issue #1757 - Update HTTP server/client instrumentation span names #1759

Merged
merged 30 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e67218a
Update ASGI implementation
macieyng Apr 15, 2023
22422e8
Update WSGI implementation
macieyng Apr 16, 2023
e94660a
Refactor ASGI and WSGI default span name handlers
macieyng Apr 16, 2023
ef94b36
Update tests for instrumentations that use otel_wsgi
macieyng Apr 16, 2023
a749e77
update Tornado implementation
macieyng Apr 16, 2023
2c056d9
update docstrings
macieyng Apr 16, 2023
51e950c
Update FastAPI implementation
macieyng Apr 16, 2023
5581bd5
Update Django implementation
macieyng Apr 19, 2023
d0b37c4
Update aiohttp implementation
macieyng Apr 19, 2023
836c2c7
Update httpx implementation
macieyng Apr 19, 2023
1ea0970
Update requests implementation
macieyng Apr 19, 2023
a0159a8
Update urllib implementation
macieyng Apr 19, 2023
a88b5a0
Update urllib3 implementation
macieyng Apr 19, 2023
f7b3c53
Merge branch 'main' into issue-1757
macieyng Apr 19, 2023
28428ef
Format with black
macieyng Apr 20, 2023
1513461
Add changelog entry
macieyng Apr 20, 2023
370c656
make django1 compliant with specification
macieyng Apr 20, 2023
2d4abcd
fix django1 test
macieyng Apr 21, 2023
c408058
Update Starlette instrumentation
macieyng Apr 21, 2023
c73f7de
Merge branch 'main' into issue-1757
shalevr Apr 24, 2023
b413d78
Merge branch 'main' into issue-1757
shalevr Apr 25, 2023
d9e39da
Merge branch 'main' into issue-1757
srikanthccv Apr 28, 2023
3355b99
Merge branch 'main' into issue-1757
shalevr Apr 29, 2023
253707d
Merge branch 'main' into issue-1757
srikanthccv May 3, 2023
adb73a7
fix starlette default span details
macieyng May 6, 2023
1a27e12
Merge branch 'open-telemetry:main' into issue-1757
macieyng May 13, 2023
42b38c1
fix starlette finally
macieyng May 13, 2023
e445641
Merge branch 'main' into issue-1757
shalevr May 17, 2023
7bb2d73
Merge branch 'main' into issue-1757
macieyng Jun 7, 2023
fea749f
Merge branch 'main' into issue-1757
shalevr Jun 15, 2023
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
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
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
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
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
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
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
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
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
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