Skip to content

Commit 0a5a4de

Browse files
authored
Remove set_http_status logic from AwsMetricAttributeGenerator (#51)
In this commit, we are removing the `set_http_status` logic from AwsMetricAttributeGenerator. This logic was originally in place for parity with Java. In Java, the OTEL instrumentation for AWS SDK does not populate the `http.status_code` attribute for spans, so we had to add logic to extract status code from the exception embedded within the events of the span. However, in Python, we have verified that this is not a problem; `http.status_code` is reliably set and so we do not need this logic. `set_http_status` logic is no longer needed, so this commit seeks to clean up the code by removing it and all code depending on it. 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 9d6a0a5 commit 0a5a4de

File tree

4 files changed

+0
-57
lines changed

4 files changed

+0
-57
lines changed

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
_FAAS_TRIGGER: str = SpanAttributes.FAAS_TRIGGER
4444
_GRAPHQL_OPERATION_TYPE: str = SpanAttributes.GRAPHQL_OPERATION_TYPE
4545
_HTTP_METHOD: str = SpanAttributes.HTTP_METHOD
46-
_HTTP_STATUS_CODE: str = SpanAttributes.HTTP_STATUS_CODE
4746
_HTTP_URL: str = SpanAttributes.HTTP_URL
4847
_MESSAGING_OPERATION: str = SpanAttributes.MESSAGING_OPERATION
4948
_MESSAGING_SYSTEM: str = SpanAttributes.MESSAGING_SYSTEM
@@ -92,7 +91,6 @@ def _generate_service_metric_attributes(span: ReadableSpan, resource: Resource)
9291
_set_service(resource, span, attributes)
9392
_set_ingress_operation(span, attributes)
9493
_set_span_kind_for_service(span, attributes)
95-
_set_http_status(span, attributes)
9694
return attributes
9795

9896

@@ -103,7 +101,6 @@ def _generate_dependency_metric_attributes(span: ReadableSpan, resource: Resourc
103101
_set_remote_service_and_operation(span, attributes)
104102
_set_remote_target(span, attributes)
105103
_set_span_kind_for_dependency(span, attributes)
106-
_set_http_status(span, attributes)
107104
return attributes
108105

109106

@@ -140,22 +137,6 @@ def _set_span_kind_for_service(span: ReadableSpan, attributes: BoundedAttributes
140137
attributes[AWS_SPAN_KIND] = span_kind
141138

142139

143-
def _set_http_status(span: ReadableSpan, attributes: BoundedAttributes) -> None:
144-
if is_key_present(span, _HTTP_STATUS_CODE):
145-
return
146-
147-
status_code: Optional[int] = _get_aws_status_code(span)
148-
if status_code is not None:
149-
attributes[_HTTP_STATUS_CODE] = status_code
150-
151-
152-
def _get_aws_status_code(span: ReadableSpan) -> Optional[int]:
153-
"""
154-
TODO: We need to determine if the same status code issue in Java AWS SDK instrumentation is present in Python.
155-
"""
156-
return None
157-
158-
159140
def _set_egress_operation(span: ReadableSpan, attributes: BoundedAttributes) -> None:
160141
"""
161142
Egress operation (i.e. operation for Client and Producer spans) is always derived from a special span attribute,

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,6 @@ def _record_error_or_fault(self, span: ReadableSpan, attributes: BoundedAttribut
9494
http_status_code: int = span.attributes.get(_HTTP_STATUS_CODE)
9595
status_code: StatusCode = span.status.status_code
9696

97-
if http_status_code is None:
98-
http_status_code = attributes.get(_HTTP_STATUS_CODE)
99-
10097
if _is_not_error_or_fault(http_status_code):
10198
if StatusCode.ERROR == status_code:
10299
self._error_histogram.record(0, attributes)

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,8 +500,6 @@ def test_peer_service_does_not_override_aws_remote_service(self):
500500
).get(DEPENDENCY_METRIC)
501501
self.assertEqual(actual_attributes.get(AWS_REMOTE_SERVICE), "TestString")
502502

503-
# TODO: add HTTP_STATUS_CODE based test when http-status-code related implementation ready
504-
505503
def test_no_metric_when_consumer_process_with_consumer_parent(self):
506504
self._mock_attribute(
507505
[AWS_CONSUMER_PARENT_SPAN_KIND, SpanAttributes.MESSAGING_OPERATION],

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

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -198,18 +198,6 @@ def test_on_end_metrics_generation_with_latency(self):
198198
self.fault_histogram_mock.assert_has_calls([call.record(0, metric_attributes_dict.get(SERVICE_METRIC))])
199199
self.latency_histogram_mock.assert_has_calls([call.record(5.5, metric_attributes_dict.get(SERVICE_METRIC))])
200200

201-
def test_on_end_metrics_generation_with_aws_status_codes(self):
202-
# INVALID HTTP STATUS CODE
203-
self._validate_metrics_generated_for_attributes_status_code(None, self.ExpectedStatusMetric.NEITHER)
204-
205-
# VALID HTTP STATUS CODE
206-
self._validate_metrics_generated_for_attributes_status_code(399, self.ExpectedStatusMetric.NEITHER)
207-
self._validate_metrics_generated_for_attributes_status_code(400, self.ExpectedStatusMetric.ERROR)
208-
self._validate_metrics_generated_for_attributes_status_code(499, self.ExpectedStatusMetric.ERROR)
209-
self._validate_metrics_generated_for_attributes_status_code(500, self.ExpectedStatusMetric.FAULT)
210-
self._validate_metrics_generated_for_attributes_status_code(599, self.ExpectedStatusMetric.FAULT)
211-
self._validate_metrics_generated_for_attributes_status_code(600, self.ExpectedStatusMetric.NEITHER)
212-
213201
def test_on_end_metrics_generation_with_http_status_codes(self):
214202
# INVALID HTTP STATUS CODE
215203
self._validate_metrics_generated_for_http_status_code(None, self.ExpectedStatusMetric.NEITHER)
@@ -317,27 +305,6 @@ def _validate_metrics_generated_for_http_status_code(
317305
self.aws_span_metrics_processor.on_end(span)
318306
self._valid_metrics(metric_attributes_dict, expected_status_metric)
319307

320-
def _validate_metrics_generated_for_attributes_status_code(
321-
self, aws_status_code, expected_status_metric: ExpectedStatusMetric
322-
):
323-
attributes: Attributes = {"new key": "new value"}
324-
span: ReadableSpan = _build_readable_span_mock(attributes, SpanKind.PRODUCER)
325-
metric_attributes_dict = _build_metric_attributes(_CONTAINS_ATTRIBUTES, span)
326-
if aws_status_code is not None:
327-
attr_temp_service = {
328-
"new service key": "new service value",
329-
SpanAttributes.HTTP_STATUS_CODE: aws_status_code,
330-
}
331-
metric_attributes_dict[SERVICE_METRIC] = attr_temp_service
332-
attr_temp_dependency = {
333-
"new dependency key": "new dependency value",
334-
SpanAttributes.HTTP_STATUS_CODE: aws_status_code,
335-
}
336-
metric_attributes_dict[DEPENDENCY_METRIC] = attr_temp_dependency
337-
self._configure_mock_for_on_end(span, metric_attributes_dict)
338-
self.aws_span_metrics_processor.on_end(span)
339-
self._valid_metrics(metric_attributes_dict, expected_status_metric)
340-
341308
def _valid_metrics(self, metric_attributes_dict, expected_status_metric: ExpectedStatusMetric):
342309

343310
if expected_status_metric == self.ExpectedStatusMetric.ERROR:

0 commit comments

Comments
 (0)