Skip to content

Commit 07a4cb2

Browse files
authored
Fixed requests gap 2 by parsing HTTP_URL attrib (aws-observability#43)
### Description of changes: Updated the `_generate_remote_service` function to fallback to parsing the `_HTTP_URL` attribute for the host and port and generate remote_service from them. This will be the back up behavior until `_NET_PEER*` gets populated properly. GitHub issue: open-telemetry/opentelemetry-python-contrib#2138. ### Testing Tested it locally to see that the `aws_remote_service` is being populated properly. Tested by deploying the code changes to EKS and nodes were linking together properly instead of the vehicle-service pointing to UnknownRemoteService. ![Screenshot 2024-02-06 at 4 31 59 PM](https://github.com/aws-observability/aws-otel-python-instrumentation/assets/23292470/2636bad4-7eff-4be8-a98a-4457dfd3cd9d) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
1 parent 5b3ed74 commit 07a4cb2

File tree

3 files changed

+40
-13
lines changed

3 files changed

+40
-13
lines changed

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/_aws_metric_attribute_generator.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@ def _set_remote_service_and_operation(span: ReadableSpan, attributes: BoundedAtt
178178
* Attributes are confirmed to have low-cardinality values, based on code analysis.
179179
180180
if the selected attributes are still producing the UnknownRemoteService or UnknownRemoteOperation, `net.peer.name`,
181-
`net.peer.port`, `net.peer.sock.addr` and `net.peer.sock.port` will be used to derive the RemoteService. And
182-
`http.method` and `http.url` will be used to derive the RemoteOperation.
181+
`net.peer.port`, `net.peer.sock.addr`, `net.peer.sock.port`and 'http.url' will be used to derive the RemoteService.
182+
And `http.method` and `http.url` will be used to derive the RemoteOperation.
183183
"""
184184
remote_service: str = UNKNOWN_REMOTE_SERVICE
185185
remote_operation: str = UNKNOWN_REMOTE_OPERATION
@@ -252,6 +252,12 @@ def _generate_remote_service(span: ReadableSpan) -> str:
252252
if is_key_present(span, _NET_SOCK_PEER_PORT):
253253
port: str = str(span.attributes.get(_NET_SOCK_PEER_PORT))
254254
remote_service += ":" + port
255+
elif is_key_present(span, _HTTP_URL):
256+
http_url: str = span.attributes.get(_HTTP_URL)
257+
if http_url:
258+
url: ParseResult = urlparse(http_url)
259+
if url and url.netloc:
260+
remote_service = url.netloc
255261
else:
256262
_log_unknown_attribute(AWS_REMOTE_SERVICE, span)
257263

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attribute_generator.py

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -454,25 +454,49 @@ def test_remote_attributes_combinations(self):
454454
[SpanAttributes.NET_SOCK_PEER_ADDR, SpanAttributes.NET_SOCK_PEER_PORT], [None, None], keys, values
455455
)
456456

457-
# Validate behavior of Remote Operation from HttpTarget - with 1st api part, then remove it
457+
# Validate behavior of Remote Operation from HttpTarget - with 1st api part. Also validates that
458+
# RemoteService is extracted from http.url.
458459
keys, values = self._mock_attribute(
459460
[SpanAttributes.HTTP_URL], ["http://www.example.com/payment/123"], keys, values
460461
)
461-
self._validate_expected_remote_attributes(_UNKNOWN_REMOTE_SERVICE, "/payment")
462+
self._validate_expected_remote_attributes("www.example.com", "/payment")
462463
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [None], keys, values)
463464

464465
# Validate behavior of Remote Operation from HttpTarget - without 1st api part, then remove it
465466
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], ["http://www.example.com"], keys, values)
467+
self._validate_expected_remote_attributes("www.example.com", "/")
468+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [None], keys, values)
469+
470+
# Validate behaviour of extracting Remote Service from http.url. When url is None, it should default to
471+
# _UNKNOWN_REMOTE_SERVICE
472+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [None], keys, values)
473+
self._validate_expected_remote_attributes(_UNKNOWN_REMOTE_SERVICE, _UNKNOWN_REMOTE_OPERATION)
474+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [None], keys, values)
475+
476+
# Validate behaviour of extracting Remote Service from http.url. When url is empty, it should default to
477+
# _UNKNOWN_REMOTE_SERVICE
478+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [""], keys, values)
466479
self._validate_expected_remote_attributes(_UNKNOWN_REMOTE_SERVICE, "/")
467480
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [None], keys, values)
468481

469-
# Validate behavior of Remote Operation from HttpTarget - invalid url, then remove it
470-
# We designed to let Generator return a "/" rather than UNKNOWN OPERATION when receive invalid HTTP URL
471-
# We basically expect url to be well formed, both / or unknown is acceptable since it should never really happen
472-
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], ["abc"], keys, values)
482+
# Validate behaviour of extracting Remote Service from http.url. When url is invalid, it should default to
483+
# _UNKNOWN_REMOTE_SERVICE
484+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], ["invalid_url"], keys, values)
473485
self._validate_expected_remote_attributes(_UNKNOWN_REMOTE_SERVICE, "/")
474486
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [None], keys, values)
475487

488+
# Validate behaviour of extracting Remote Service from http.url. When url is a host name like
489+
# https://www.example.com, it should extract the netaddr name as www.example.com
490+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], ["https://www.example.com"], keys, values)
491+
self._validate_expected_remote_attributes("www.example.com", "/")
492+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [None], keys, values)
493+
494+
# Validate behaviour of extracting Remote Service from http.url. When url is an ip address with port like
495+
# http://192.168.1.1:1234, it should extract the netaddr name as 192.168.1.1:1234
496+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], ["http://192.168.1.1:1234"], keys, values)
497+
self._validate_expected_remote_attributes("192.168.1.1:1234", "/")
498+
keys, values = self._mock_attribute([SpanAttributes.HTTP_URL], [None], keys, values)
499+
476500
# Validate behaviour of Peer service attribute, then remove it.
477501
keys, values = self._mock_attribute([SpanAttributes.PEER_SERVICE], ["Peer service"], keys, values)
478502
self._validate_expected_remote_attributes("Peer service", _UNKNOWN_REMOTE_OPERATION)

contract-tests/tests/test/amazon/requests/requests_test.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,7 @@ def _assert_aws_attributes(self, attributes_list: List[KeyValue], method: str, e
101101
# InternalOperation as OTEL does not instrument the basic server we are using, so the client span is a local
102102
# root.
103103
self._assert_str_attribute(attributes_dict, AWS_LOCAL_OPERATION, "InternalOperation")
104-
# TODO: This should be "backend:8080", but isn't because requests instrumentation is not populating peer
105-
# attributes
106-
self._assert_str_attribute(attributes_dict, AWS_REMOTE_SERVICE, "UnknownRemoteService")
104+
self._assert_str_attribute(attributes_dict, AWS_REMOTE_SERVICE, "backend:8080")
107105
self._assert_str_attribute(attributes_dict, AWS_REMOTE_OPERATION, f"{method} /backend")
108106
# See comment above AWS_LOCAL_OPERATION
109107
self._assert_str_attribute(attributes_dict, AWS_SPAN_KIND, "LOCAL_ROOT")
@@ -181,8 +179,7 @@ def _assert_metric_attributes(
181179
self._assert_str_attribute(attribute_dict, AWS_LOCAL_SERVICE, self.get_application_otel_service_name())
182180
# See comment on AWS_LOCAL_OPERATION in _assert_aws_attributes
183181
self._assert_str_attribute(attribute_dict, AWS_LOCAL_OPERATION, "InternalOperation")
184-
# See comment on AWS_REMOTE_SERVICE in _assert_aws_attributes
185-
self._assert_str_attribute(attribute_dict, AWS_REMOTE_SERVICE, "UnknownRemoteService")
182+
self._assert_str_attribute(attribute_dict, AWS_REMOTE_SERVICE, "backend:8080")
186183
self._assert_str_attribute(attribute_dict, AWS_REMOTE_OPERATION, f"{method} /backend")
187184
self._assert_str_attribute(attribute_dict, AWS_SPAN_KIND, "CLIENT")
188185

0 commit comments

Comments
 (0)