Skip to content

Commit

Permalink
Merge branch 'main' into emptyroute
Browse files Browse the repository at this point in the history
  • Loading branch information
shalevr authored Jun 16, 2023
2 parents c28c672 + f9f7b01 commit 1b53e15
Show file tree
Hide file tree
Showing 51 changed files with 474 additions and 381 deletions.
15 changes: 13 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- `opentelemetry-instrumentation-django` Fix empty span name when using
`path("", ...)` ([#1788](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1788)
- Fix falcon instrumentation's usage of Span Status to only set the description if the status code is ERROR.
([#1840](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1840))
- Instrument all httpx versions >= 0.18. ([#1748](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1748))

## Version 1.18.0/0.39b0 (2023-05-10)

Expand All @@ -26,9 +29,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#1778](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1778))
- Add `excluded_urls` functionality to `urllib` and `urllib3` instrumentations
([#1733](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1733))
- Make Django request span attributes available for `start_span`.
- Make Django request span attributes available for `start_span`.
([#1730](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1730))
- Make ASGI request span attributes available for `start_span`.
- Make ASGI request span attributes available for `start_span`.
([#1762](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1762))
- `opentelemetry-instrumentation-celery` Add support for anonymous tasks.
([#1407](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1407))
Expand All @@ -43,12 +46,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Fix redis db.statements to be sanitized by default
([#1778](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1778))
- Fix elasticsearch db.statement attribute to be sanitized by default
([#1758](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1758))
- Fix `AttributeError` when AWS Lambda handler receives a list event
([#1738](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1738))
- Fix `None does not implement middleware` error when there are no middlewares registered
([#1766](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1766))
- 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)

Expand Down
2 changes: 1 addition & 1 deletion dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ bleach==4.1.0 # transient dependency for readme-renderer
grpcio-tools==1.29.0
mypy-protobuf>=1.23
protobuf~=3.13
markupsafe==2.0.1
markupsafe>=2.0.1
codespell==2.1.0
requests==2.28.1
ruamel.yaml==0.17.21
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ instruments = [
]
test = [
"opentelemetry-instrumentation-aiohttp-client[instruments]",
"http-server-mock"
]

[project.entry-points.opentelemetry_instrumentor]
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 @@ -23,6 +23,7 @@
import aiohttp
import aiohttp.test_utils
import yarl
from http_server_mock import HttpServerMock
from pkg_resources import iter_entry_points

from opentelemetry import context
Expand Down Expand Up @@ -118,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 @@ -212,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 @@ -246,7 +247,7 @@ async def do_request(url):
self.assert_spans(
[
(
"HTTP GET",
"GET",
(expected_status, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand All @@ -273,7 +274,7 @@ async def request_handler(request):
self.assert_spans(
[
(
"HTTP GET",
"GET",
(StatusCode.ERROR, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand All @@ -300,7 +301,7 @@ async def request_handler(request):
self.assert_spans(
[
(
"HTTP GET",
"GET",
(StatusCode.ERROR, None),
{
SpanAttributes.HTTP_METHOD: "GET",
Expand All @@ -313,27 +314,37 @@ async def request_handler(request):
def test_credential_removal(self):
trace_configs = [aiohttp_client.create_trace_config()]

url = "http://username:password@httpbin.org/status/200"
with self.subTest(url=url):
app = HttpServerMock("test_credential_removal")

async def do_request(url):
async with aiohttp.ClientSession(
trace_configs=trace_configs,
) as session:
async with session.get(url):
pass
@app.route("/status/200")
def index():
return "hello"

loop = asyncio.get_event_loop()
loop.run_until_complete(do_request(url))
url = "http://username:password@localhost:5000/status/200"

with app.run("localhost", 5000):
with self.subTest(url=url):

async def do_request(url):
async with aiohttp.ClientSession(
trace_configs=trace_configs,
) as session:
async with session.get(url):
pass

loop = asyncio.get_event_loop()
loop.run_until_complete(do_request(url))

self.assert_spans(
[
(
"HTTP GET",
"GET",
(StatusCode.UNSET, None),
{
SpanAttributes.HTTP_METHOD: "GET",
SpanAttributes.HTTP_URL: "http://httpbin.org/status/200",
SpanAttributes.HTTP_URL: (
"http://localhost:5000/status/200"
),
SpanAttributes.HTTP_STATUS_CODE: int(HTTPStatus.OK),
},
)
Expand Down Expand Up @@ -380,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 Expand Up @@ -705,11 +705,11 @@ def test_response_attributes_invalid_status_code(self):
self.assertEqual(self.span.set_status.call_count, 1)

def test_credential_removal(self):
self.scope["server"] = ("username:password@httpbin.org", 80)
self.scope["server"] = ("username:password@mock", 80)
self.scope["path"] = "/status/200"
attrs = otel_asgi.collect_request_attributes(self.scope)
self.assertEqual(
attrs[SpanAttributes.HTTP_URL], "http://httpbin.org/status/200"
attrs[SpanAttributes.HTTP_URL], "http://mock/status/200"
)

def test_collect_target_attribute_missing(self):
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") and 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 @@ -154,9 +154,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 @@ -181,9 +181,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 @@ -229,9 +227,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 @@ -255,9 +251,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 @@ -321,9 +315,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 @@ -337,9 +329,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 @@ -348,7 +338,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
Loading

0 comments on commit 1b53e15

Please sign in to comment.