Improve local operation resolution (#113)

*Issue:*
In the
[`get_ingress_operation()`](https://github.com/aws-observability/aws-otel-python-instrumentation/blob/main/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/_aws_span_processing_util.py#L43)
method, we try to resolve the `aws.local.operation` by first using the
span name. If the span name is not meaningful, we [try to generate the
value](https://github.com/aws-observability/aws-otel-python-instrumentation/blob/main/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/_aws_span_processing_util.py#L162)
by combining `http.method` and `http.target` values from the span. If
the `http.target` value is not present, we set the `aws.local.operation`
to `UnknownOperation`.

In our testing, we found that quite often we get ingress requests where
the span name is not meaningful and the `http.target` is also not set by
the instrumentation. So we end up getting a lot of `UnknownOperation`
for such requests.

*Proposal:*
If the span name is not meaningful, and the `http.target` is not
present, then we try and parse the `http.url` if present to only pick
the first part of the url path (to minimize the probability of high
cardinality values) and combine it with the `http.method` to get the
local operation.
Similar logic is employed in [resolving the remote
operation](https://github.com/aws-observability/aws-otel-python-instrumentation/blob/566d1629b3ce4d26436c6dcad52f55f2b9e5873a/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/_aws_metric_attribute_generator.py#L302-L320).


By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

Author: srprash

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

@@ -4,6 +4,7 @@
 import json
 import os
 from typing import Dict, List
+from urllib.parse import ParseResult, urlparse
 
 from amazon.opentelemetry.distro._aws_attribute_keys import AWS_CONSUMER_PARENT_SPAN_KIND, AWS_LOCAL_OPERATION
 from opentelemetry.sdk.trace import InstrumentationScope, ReadableSpan
@@ -161,19 +162,25 @@ def _is_valid_operation(span: ReadableSpan, operation: str) -> bool:
 
 def _generate_ingress_operation(span: ReadableSpan) -> str:
     """
-    When span name is not meaningful(null, unknown or http_method value) as operation name for http use cases. Will try
-    to extract the operation name from http target string
+    When span name is not meaningful, this method is invoked to try to extract the operation name from either
+    `http.target`, if present, or from `http.url`, and combine with `http.method`.
     """
     operation: str = UNKNOWN_OPERATION
+    http_path: str = None
     if is_key_present(span, SpanAttributes.HTTP_TARGET):
-        http_target: str = span.attributes.get(SpanAttributes.HTTP_TARGET)
-        # get the first part from API path string as operation value
-        # the more levels/parts we get from API path the higher chance for getting high cardinality data
-        if http_target is not None:
-            operation = extract_api_path_value(http_target)
-            if is_key_present(span, SpanAttributes.HTTP_METHOD):
-                http_method: str = span.attributes.get(SpanAttributes.HTTP_METHOD)
-                if http_method is not None:
-                    operation = http_method + " " + operation
+        http_path = span.attributes.get(SpanAttributes.HTTP_TARGET)
+    elif is_key_present(span, SpanAttributes.HTTP_URL):
+        http_url = span.attributes.get(SpanAttributes.HTTP_URL)
+        url: ParseResult = urlparse(http_url)
+        http_path = url.path
+
+    # get the first part from API path string as operation value
+    # the more levels/parts we get from API path the higher chance for getting high cardinality data
+    if http_path is not None:
+        operation = extract_api_path_value(http_path)
+        if is_key_present(span, SpanAttributes.HTTP_METHOD):
+            http_method: str = span.attributes.get(SpanAttributes.HTTP_METHOD)
+            if http_method is not None:
+                operation = http_method + " " + operation
 
     return operation

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

@@ -1,5 +1,8 @@
 # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 # SPDX-License-Identifier: Apache-2.0
+
+# pylint: disable=too-many-lines
+
 from typing import Dict, List, Optional
 from unittest import TestCase
 from unittest.mock import MagicMock
@@ -303,6 +306,101 @@ def test_server_span_with_span_name_with_http_target(self):
         self._mock_attribute(SpanAttributes.HTTP_METHOD, None)
         self._mock_attribute(SpanAttributes.HTTP_TARGET, None)
 
+    def test_server_span_with_span_name_with_target_and_url(self):
+        # when http.target & http.url are present, the local operation should be derived from the http.target
+        self._update_resource_with_service_name()
+        self.span_mock.name = "POST"
+        self._mock_attribute(
+            [SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_TARGET, SpanAttributes.HTTP_URL],
+            ["POST", "/my-target/09876", "http://127.0.0.1:8000/payment/123"],
+        )
+
+        expected_attributes: Attributes = {
+            AWS_SPAN_KIND: SpanKind.SERVER.name,
+            AWS_LOCAL_SERVICE: _SERVICE_NAME_VALUE,
+            AWS_LOCAL_OPERATION: "POST /my-target",
+        }
+        self._validate_attributes_produced_for_non_local_root_span_of_kind(expected_attributes, SpanKind.SERVER)
+        self._mock_attribute(SpanAttributes.HTTP_METHOD, None)
+        self._mock_attribute(SpanAttributes.HTTP_TARGET, None)
+        self._mock_attribute(SpanAttributes.HTTP_URL, None)
+
+    def test_server_span_with_span_name_with_http_url(self):
+        self._update_resource_with_service_name()
+        self.span_mock.name = "POST"
+        self._mock_attribute(
+            [SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_URL], ["POST", "http://127.0.0.1:8000/payment/123"]
+        )
+
+        expected_attributes: Attributes = {
+            AWS_SPAN_KIND: SpanKind.SERVER.name,
+            AWS_LOCAL_SERVICE: _SERVICE_NAME_VALUE,
+            AWS_LOCAL_OPERATION: "POST /payment",
+        }
+        self._validate_attributes_produced_for_non_local_root_span_of_kind(expected_attributes, SpanKind.SERVER)
+        self._mock_attribute(SpanAttributes.HTTP_METHOD, None)
+        self._mock_attribute(SpanAttributes.HTTP_URL, None)
+
+    def test_server_span_with_http_url_with_no_path(self):
+        # http.url with no path should result in local operation to be "POST /"
+        self._update_resource_with_service_name()
+        self.span_mock.name = "POST"
+        self._mock_attribute([SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_URL], ["POST", "http://www.example.com"])
+
+        expected_attributes: Attributes = {
+            AWS_SPAN_KIND: SpanKind.SERVER.name,
+            AWS_LOCAL_SERVICE: _SERVICE_NAME_VALUE,
+            AWS_LOCAL_OPERATION: "POST /",
+        }
+        self._validate_attributes_produced_for_non_local_root_span_of_kind(expected_attributes, SpanKind.SERVER)
+        self._mock_attribute(SpanAttributes.HTTP_METHOD, None)
+        self._mock_attribute(SpanAttributes.HTTP_URL, None)
+
+    def test_server_span_with_http_url_as_none(self):
+        # if http.url is none, local operation should default to UnknownOperation
+        self._update_resource_with_service_name()
+        self.span_mock.name = "POST"
+        self._mock_attribute([SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_URL], ["POST", None])
+
+        expected_attributes: Attributes = {
+            AWS_SPAN_KIND: SpanKind.SERVER.name,
+            AWS_LOCAL_SERVICE: _SERVICE_NAME_VALUE,
+            AWS_LOCAL_OPERATION: _UNKNOWN_OPERATION,
+        }
+        self._validate_attributes_produced_for_non_local_root_span_of_kind(expected_attributes, SpanKind.SERVER)
+        self._mock_attribute(SpanAttributes.HTTP_METHOD, None)
+        self._mock_attribute(SpanAttributes.HTTP_URL, None)
+
+    def test_server_span_with_http_url_as_empty(self):
+        # if http.url is empty, local operation should default to "POST /"
+        self._update_resource_with_service_name()
+        self.span_mock.name = "POST"
+        self._mock_attribute([SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_URL], ["POST", ""])
+
+        expected_attributes: Attributes = {
+            AWS_SPAN_KIND: SpanKind.SERVER.name,
+            AWS_LOCAL_SERVICE: _SERVICE_NAME_VALUE,
+            AWS_LOCAL_OPERATION: "POST /",
+        }
+        self._validate_attributes_produced_for_non_local_root_span_of_kind(expected_attributes, SpanKind.SERVER)
+        self._mock_attribute(SpanAttributes.HTTP_METHOD, None)
+        self._mock_attribute(SpanAttributes.HTTP_URL, None)
+
+    def test_server_span_with_http_url_as_invalid(self):
+        # if http.url is invalid, local operation should default to "POST /"
+        self._update_resource_with_service_name()
+        self.span_mock.name = "POST"
+        self._mock_attribute([SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_URL], ["POST", "invalid_url"])
+
+        expected_attributes: Attributes = {
+            AWS_SPAN_KIND: SpanKind.SERVER.name,
+            AWS_LOCAL_SERVICE: _SERVICE_NAME_VALUE,
+            AWS_LOCAL_OPERATION: "POST /",
+        }
+        self._validate_attributes_produced_for_non_local_root_span_of_kind(expected_attributes, SpanKind.SERVER)
+        self._mock_attribute(SpanAttributes.HTTP_METHOD, None)
+        self._mock_attribute(SpanAttributes.HTTP_URL, None)
+
     def test_producer_span_with_attributes(self):
         self._update_resource_with_service_name()
         self._mock_attribute(