Skip to content

Commit cc52bd2

Browse files
emdnetoemdneto
authored
Fix http clients method attribute in case of non standard http methods (#2726)
1 parent 910d5ec commit cc52bd2

File tree

7 files changed

+209
-15
lines changed

7 files changed

+209
-15
lines changed

‎CHANGELOG.md

+4-1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5252
([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682))
5353
- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `django` middleware
5454
([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714))
55+
- `opentelemetry-instrumentation-httpx`, `opentelemetry-instrumentation-aiohttp-client`,
56+
`opentelemetry-instrumentation-requests` Populate `{method}` as `HTTP` on `_OTHER` methods
57+
([#2726](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2726))
5558

5659
### Fixed
5760
- Handle `redis.exceptions.WatchError` as a non-error event in redis instrumentation
@@ -79,7 +82,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
7982
([#2153](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2153))
8083
- `opentelemetry-instrumentation-asgi` Removed `NET_HOST_NAME` AND `NET_HOST_PORT` from active requests count attribute
8184
([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610))
82-
- `opentelemetry-instrumentation-asgi` Bugfix: Middleware did not set status code attribute on duration metrics for non-recording spans.
85+
- `opentelemetry-instrumentation-asgi` Bugfix: Middleware did not set status code attribute on duration metrics for non-recording spans.
8386
([#2627](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2627))
8487

8588

‎instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py

+5-6
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,9 @@ def response_hook(span: Span, params: typing.Union[
132132

133133

134134
def _get_span_name(method: str) -> str:
135-
method = sanitize_method(method.upper().strip())
135+
method = sanitize_method(method.strip())
136136
if method == "_OTHER":
137137
method = "HTTP"
138-
139138
return method
140139

141140

@@ -230,8 +229,8 @@ async def on_request_start(
230229
trace_config_ctx.span = None
231230
return
232231

233-
http_method = params.method
234-
request_span_name = _get_span_name(http_method)
232+
method = params.method
233+
request_span_name = _get_span_name(method)
235234
request_url = (
236235
remove_url_credentials(trace_config_ctx.url_filter(params.url))
237236
if callable(trace_config_ctx.url_filter)
@@ -241,8 +240,8 @@ async def on_request_start(
241240
span_attributes = {}
242241
_set_http_method(
243242
span_attributes,
244-
http_method,
245-
request_span_name,
243+
method,
244+
sanitize_method(method),
246245
sem_conv_opt_in_mode,
247246
)
248247
_set_http_url(span_attributes, request_url, sem_conv_opt_in_mode)

‎instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py

+87
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
4141
from opentelemetry.semconv.attributes.http_attributes import (
4242
HTTP_REQUEST_METHOD,
43+
HTTP_REQUEST_METHOD_ORIGINAL,
4344
HTTP_RESPONSE_STATUS_CODE,
4445
)
4546
from opentelemetry.semconv.attributes.url_attributes import URL_FULL
@@ -503,6 +504,92 @@ async def request_handler(request):
503504
]
504505
)
505506

507+
def test_nonstandard_http_method(self):
508+
trace_configs = [aiohttp_client.create_trace_config()]
509+
app = HttpServerMock("nonstandard_method")
510+
511+
@app.route("/status/200", methods=["NONSTANDARD"])
512+
def index():
513+
return ("", 405, {})
514+
515+
url = "http://localhost:5000/status/200"
516+
517+
with app.run("localhost", 5000):
518+
with self.subTest(url=url):
519+
520+
async def do_request(url):
521+
async with aiohttp.ClientSession(
522+
trace_configs=trace_configs,
523+
) as session:
524+
async with session.request("NONSTANDARD", url):
525+
pass
526+
527+
loop = asyncio.get_event_loop()
528+
loop.run_until_complete(do_request(url))
529+
530+
self.assert_spans(
531+
[
532+
(
533+
"HTTP",
534+
(StatusCode.ERROR, None),
535+
{
536+
SpanAttributes.HTTP_METHOD: "_OTHER",
537+
SpanAttributes.HTTP_URL: url,
538+
SpanAttributes.HTTP_STATUS_CODE: int(
539+
HTTPStatus.METHOD_NOT_ALLOWED
540+
),
541+
},
542+
)
543+
]
544+
)
545+
self.memory_exporter.clear()
546+
547+
def test_nonstandard_http_method_new_semconv(self):
548+
trace_configs = [
549+
aiohttp_client.create_trace_config(
550+
sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP
551+
)
552+
]
553+
app = HttpServerMock("nonstandard_method")
554+
555+
@app.route("/status/200", methods=["NONSTANDARD"])
556+
def index():
557+
return ("", 405, {})
558+
559+
url = "http://localhost:5000/status/200"
560+
561+
with app.run("localhost", 5000):
562+
with self.subTest(url=url):
563+
564+
async def do_request(url):
565+
async with aiohttp.ClientSession(
566+
trace_configs=trace_configs,
567+
) as session:
568+
async with session.request("NONSTANDARD", url):
569+
pass
570+
571+
loop = asyncio.get_event_loop()
572+
loop.run_until_complete(do_request(url))
573+
574+
self.assert_spans(
575+
[
576+
(
577+
"HTTP",
578+
(StatusCode.ERROR, None),
579+
{
580+
HTTP_REQUEST_METHOD: "_OTHER",
581+
URL_FULL: url,
582+
HTTP_RESPONSE_STATUS_CODE: int(
583+
HTTPStatus.METHOD_NOT_ALLOWED
584+
),
585+
HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD",
586+
ERROR_TYPE: "405",
587+
},
588+
)
589+
]
590+
)
591+
self.memory_exporter.clear()
592+
506593
def test_credential_removal(self):
507594
trace_configs = [aiohttp_client.create_trace_config()]
508595

‎instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py

+7-5
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ class ResponseInfo(typing.NamedTuple):
259259

260260

261261
def _get_default_span_name(method: str) -> str:
262-
method = sanitize_method(method.upper().strip())
262+
method = sanitize_method(method.strip())
263263
if method == "_OTHER":
264264
method = "HTTP"
265265

@@ -326,12 +326,16 @@ def _apply_request_client_attributes_to_span(
326326
span_attributes: dict,
327327
url: typing.Union[str, URL, httpx.URL],
328328
method_original: str,
329-
span_name: str,
330329
semconv: _HTTPStabilityMode,
331330
):
332331
url = httpx.URL(url)
333332
# http semconv transition: http.method -> http.request.method
334-
_set_http_method(span_attributes, method_original, span_name, semconv)
333+
_set_http_method(
334+
span_attributes,
335+
method_original,
336+
sanitize_method(method_original),
337+
semconv,
338+
)
335339
# http semconv transition: http.url -> url.full
336340
_set_http_url(span_attributes, str(url), semconv)
337341

@@ -450,7 +454,6 @@ def handle_request(
450454
span_attributes,
451455
url,
452456
method_original,
453-
span_name,
454457
self._sem_conv_opt_in_mode,
455458
)
456459

@@ -572,7 +575,6 @@ async def handle_async_request(self, *args, **kwargs) -> typing.Union[
572575
span_attributes,
573576
url,
574577
method_original,
575-
span_name,
576578
self._sem_conv_opt_in_mode,
577579
)
578580

‎instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py

+54
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
4040
from opentelemetry.semconv.attributes.http_attributes import (
4141
HTTP_REQUEST_METHOD,
42+
HTTP_REQUEST_METHOD_ORIGINAL,
4243
HTTP_RESPONSE_STATUS_CODE,
4344
)
4445
from opentelemetry.semconv.attributes.network_attributes import (
@@ -217,6 +218,59 @@ def test_basic(self):
217218
span, opentelemetry.instrumentation.httpx
218219
)
219220

221+
def test_nonstandard_http_method(self):
222+
respx.route(method="NONSTANDARD").mock(
223+
return_value=httpx.Response(405)
224+
)
225+
self.perform_request(self.URL, method="NONSTANDARD")
226+
span = self.assert_span()
227+
228+
self.assertIs(span.kind, trace.SpanKind.CLIENT)
229+
self.assertEqual(span.name, "HTTP")
230+
self.assertEqual(
231+
span.attributes,
232+
{
233+
SpanAttributes.HTTP_METHOD: "_OTHER",
234+
SpanAttributes.HTTP_URL: self.URL,
235+
SpanAttributes.HTTP_STATUS_CODE: 405,
236+
},
237+
)
238+
239+
self.assertIs(span.status.status_code, trace.StatusCode.ERROR)
240+
241+
self.assertEqualSpanInstrumentationInfo(
242+
span, opentelemetry.instrumentation.httpx
243+
)
244+
245+
def test_nonstandard_http_method_new_semconv(self):
246+
respx.route(method="NONSTANDARD").mock(
247+
return_value=httpx.Response(405)
248+
)
249+
self.perform_request(self.URL, method="NONSTANDARD")
250+
span = self.assert_span()
251+
252+
self.assertIs(span.kind, trace.SpanKind.CLIENT)
253+
self.assertEqual(span.name, "HTTP")
254+
self.assertEqual(
255+
span.attributes,
256+
{
257+
HTTP_REQUEST_METHOD: "_OTHER",
258+
URL_FULL: self.URL,
259+
SERVER_ADDRESS: "mock",
260+
NETWORK_PEER_ADDRESS: "mock",
261+
HTTP_RESPONSE_STATUS_CODE: 405,
262+
NETWORK_PROTOCOL_VERSION: "1.1",
263+
ERROR_TYPE: "405",
264+
HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD",
265+
},
266+
)
267+
268+
self.assertIs(span.status.status_code, trace.StatusCode.ERROR)
269+
270+
self.assertEqualSpanInstrumentationInfo(
271+
span, opentelemetry.instrumentation.httpx
272+
)
273+
220274
def test_basic_new_semconv(self):
221275
url = "http://mock:8080/status/200"
222276
respx.get(url).mock(

‎instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py

+9-3
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,19 @@ def get_or_create_headers():
188188

189189
span_attributes = {}
190190
_set_http_method(
191-
span_attributes, method, span_name, sem_conv_opt_in_mode
191+
span_attributes,
192+
method,
193+
sanitize_method(method),
194+
sem_conv_opt_in_mode,
192195
)
193196
_set_http_url(span_attributes, url, sem_conv_opt_in_mode)
194197

195198
metric_labels = {}
196199
_set_http_method(
197-
metric_labels, method, span_name, sem_conv_opt_in_mode
200+
metric_labels,
201+
method,
202+
sanitize_method(method),
203+
sem_conv_opt_in_mode,
198204
)
199205

200206
try:
@@ -365,7 +371,7 @@ def get_default_span_name(method):
365371
Returns:
366372
span name
367373
"""
368-
method = sanitize_method(method.upper().strip())
374+
method = sanitize_method(method.strip())
369375
if method == "_OTHER":
370376
return "HTTP"
371377
return method

‎instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py

+43
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
3737
from opentelemetry.semconv.attributes.http_attributes import (
3838
HTTP_REQUEST_METHOD,
39+
HTTP_REQUEST_METHOD_ORIGINAL,
3940
HTTP_RESPONSE_STATUS_CODE,
4041
)
4142
from opentelemetry.semconv.attributes.network_attributes import (
@@ -247,6 +248,48 @@ def test_basic_both_semconv(self):
247248
span, opentelemetry.instrumentation.requests
248249
)
249250

251+
@mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",))
252+
def test_nonstandard_http_method(self):
253+
httpretty.register_uri("NONSTANDARD", self.URL, status=405)
254+
session = requests.Session()
255+
session.request("NONSTANDARD", self.URL)
256+
span = self.assert_span()
257+
self.assertIs(span.kind, trace.SpanKind.CLIENT)
258+
self.assertEqual(span.name, "HTTP")
259+
self.assertEqual(
260+
span.attributes,
261+
{
262+
SpanAttributes.HTTP_METHOD: "_OTHER",
263+
SpanAttributes.HTTP_URL: self.URL,
264+
SpanAttributes.HTTP_STATUS_CODE: 405,
265+
},
266+
)
267+
268+
self.assertIs(span.status.status_code, trace.StatusCode.ERROR)
269+
270+
@mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",))
271+
def test_nonstandard_http_method_new_semconv(self):
272+
httpretty.register_uri("NONSTANDARD", self.URL, status=405)
273+
session = requests.Session()
274+
session.request("NONSTANDARD", self.URL)
275+
span = self.assert_span()
276+
self.assertIs(span.kind, trace.SpanKind.CLIENT)
277+
self.assertEqual(span.name, "HTTP")
278+
self.assertEqual(
279+
span.attributes,
280+
{
281+
HTTP_REQUEST_METHOD: "_OTHER",
282+
URL_FULL: self.URL,
283+
SERVER_ADDRESS: "mock",
284+
NETWORK_PEER_ADDRESS: "mock",
285+
HTTP_RESPONSE_STATUS_CODE: 405,
286+
NETWORK_PROTOCOL_VERSION: "1.1",
287+
ERROR_TYPE: "405",
288+
HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD",
289+
},
290+
)
291+
self.assertIs(span.status.status_code, trace.StatusCode.ERROR)
292+
250293
def test_hooks(self):
251294
def request_hook(span, request_obj):
252295
span.update_name("name set from hook")

0 commit comments

Comments
 (0)