Skip to content

Commit 0bf0163

Browse files
authored
Add unit tests and fix bug (#128)
When enabling test coverage checks, we observed that `aws_opentelementry_configurator.py` had low (75%) coverage. We decided to add unit test coverage as this class contains simple but critical code. In adding these tests, we identified a bug in `is_app_signals_enabled`, namely that it returned true if any string value is supplied (e.g. "true", "false", "blue"). In [Java](https://github.com/aws-observability/aws-otel-java-instrumentation/blob/main/awsagentprovider/src/main/java/software/amazon/opentelemetry/javaagent/providers/AwsAppSignalsCustomizerProvider.java#L70-L71), we use [`ConfigProperties.getBoolean`](https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/ConfigProperties.java#L44), which only returns true if "true" is supplied. 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 c7de72c commit 0bf0163

File tree

3 files changed

+66
-8
lines changed

3 files changed

+66
-8
lines changed

.coveragerc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
[report]
22
include_namespace_packages = true
3-
fail_under = 93.00
3+
fail_under = 95.00
44
precision = 2

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,19 +203,19 @@ def _custom_import_sampler(sampler_name: str, resource: Resource) -> Sampler:
203203

204204

205205
def _customize_sampler(sampler: Sampler) -> Sampler:
206-
if not is_app_signals_enabled():
206+
if not _is_app_signals_enabled():
207207
return sampler
208208
return AlwaysRecordSampler(sampler)
209209

210210

211211
def _customize_exporter(span_exporter: SpanExporter, resource: Resource) -> SpanExporter:
212-
if not is_app_signals_enabled():
212+
if not _is_app_signals_enabled():
213213
return span_exporter
214214
return AwsMetricAttributesSpanExporterBuilder(span_exporter, resource).build()
215215

216216

217217
def _customize_span_processors(provider: TracerProvider, resource: Resource) -> None:
218-
if not is_app_signals_enabled():
218+
if not _is_app_signals_enabled():
219219
return
220220

221221
# Construct and set local and remote attributes span processor
@@ -247,8 +247,8 @@ def _customize_versions(auto_resource: Dict[str, any]) -> Dict[str, any]:
247247
return auto_resource
248248

249249

250-
def is_app_signals_enabled():
251-
return os.environ.get(OTEL_AWS_APP_SIGNALS_ENABLED, False)
250+
def _is_app_signals_enabled():
251+
return os.environ.get(OTEL_AWS_APP_SIGNALS_ENABLED, "false").lower() == "true"
252252

253253

254254
class AppSignalsExporterProvider:

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

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,28 @@
33
import os
44
import time
55
from unittest import TestCase
6-
from unittest.mock import patch
6+
from unittest.mock import MagicMock, patch
77

8+
from amazon.opentelemetry.distro.always_record_sampler import AlwaysRecordSampler
9+
from amazon.opentelemetry.distro.attribute_propagating_span_processor import AttributePropagatingSpanProcessor
10+
from amazon.opentelemetry.distro.aws_metric_attributes_span_exporter import AwsMetricAttributesSpanExporter
811
from amazon.opentelemetry.distro.aws_opentelemetry_configurator import (
912
AwsOpenTelemetryConfigurator,
1013
_custom_import_sampler,
14+
_customize_exporter,
15+
_customize_sampler,
16+
_customize_span_processors,
17+
_is_app_signals_enabled,
1118
)
1219
from amazon.opentelemetry.distro.aws_opentelemetry_distro import AwsOpenTelemetryDistro
20+
from amazon.opentelemetry.distro.aws_span_metrics_processor import AwsSpanMetricsProcessor
1321
from amazon.opentelemetry.distro.sampler._aws_xray_sampling_client import _AwsXRaySamplingClient
1422
from amazon.opentelemetry.distro.sampler.aws_xray_remote_sampler import AwsXRayRemoteSampler
1523
from opentelemetry.environment_variables import OTEL_LOGS_EXPORTER, OTEL_METRICS_EXPORTER, OTEL_TRACES_EXPORTER
1624
from opentelemetry.sdk.environment_variables import OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG
17-
from opentelemetry.sdk.trace import Span, Tracer, TracerProvider
25+
from opentelemetry.sdk.resources import Resource
26+
from opentelemetry.sdk.trace import Span, SpanProcessor, Tracer, TracerProvider
27+
from opentelemetry.sdk.trace.export import SpanExporter
1828
from opentelemetry.sdk.trace.sampling import Sampler
1929
from opentelemetry.trace import get_tracer_provider
2030

@@ -183,3 +193,51 @@ def test_not_using_xray_sampler_does_not_modify_url_exclusion_env_vars(self):
183193
_: Sampler = _custom_import_sampler("traceidratio", resource=None)
184194
self.assertEqual(os.environ.get("OTEL_PYTHON_REQUESTS_EXCLUDED_URLS", None), ",,,target_A,target_B,,,")
185195
self.assertEqual(os.environ.get("OTEL_PYTHON_URLLIB3_EXCLUDED_URLS", None), "target_C,target_D")
196+
197+
def test_is_app_signals_enabled(self):
198+
os.environ.setdefault("OTEL_AWS_APP_SIGNALS_ENABLED", "True")
199+
self.assertTrue(_is_app_signals_enabled())
200+
os.environ.pop("OTEL_AWS_APP_SIGNALS_ENABLED", None)
201+
202+
os.environ.setdefault("OTEL_AWS_APP_SIGNALS_ENABLED", "False")
203+
self.assertFalse(_is_app_signals_enabled())
204+
os.environ.pop("OTEL_AWS_APP_SIGNALS_ENABLED", None)
205+
self.assertFalse(_is_app_signals_enabled())
206+
207+
def test_customize_sampler(self):
208+
mock_sampler: Sampler = MagicMock()
209+
customized_sampler: Sampler = _customize_sampler(mock_sampler)
210+
self.assertEqual(mock_sampler, customized_sampler)
211+
212+
os.environ.setdefault("OTEL_AWS_APP_SIGNALS_ENABLED", "True")
213+
customized_sampler = _customize_sampler(mock_sampler)
214+
self.assertNotEqual(mock_sampler, customized_sampler)
215+
self.assertIsInstance(customized_sampler, AlwaysRecordSampler)
216+
self.assertEqual(mock_sampler, customized_sampler._root_sampler)
217+
os.environ.pop("OTEL_AWS_APP_SIGNALS_ENABLED", None)
218+
219+
def test_customize_exporter(self):
220+
mock_exporter: SpanExporter = MagicMock()
221+
customized_exporter: SpanExporter = _customize_exporter(mock_exporter, Resource.get_empty())
222+
self.assertEqual(mock_exporter, customized_exporter)
223+
224+
os.environ.setdefault("OTEL_AWS_APP_SIGNALS_ENABLED", "True")
225+
customized_exporter = _customize_exporter(mock_exporter, Resource.get_empty())
226+
self.assertNotEqual(mock_exporter, customized_exporter)
227+
self.assertIsInstance(customized_exporter, AwsMetricAttributesSpanExporter)
228+
self.assertEqual(mock_exporter, customized_exporter._delegate)
229+
os.environ.pop("OTEL_AWS_APP_SIGNALS_ENABLED", None)
230+
231+
def test_customize_span_processors(self):
232+
mock_tracer_provider: TracerProvider = MagicMock()
233+
_customize_span_processors(mock_tracer_provider, Resource.get_empty())
234+
self.assertEqual(mock_tracer_provider.add_span_processor.call_count, 0)
235+
236+
os.environ.setdefault("OTEL_AWS_APP_SIGNALS_ENABLED", "True")
237+
_customize_span_processors(mock_tracer_provider, Resource.get_empty())
238+
self.assertEqual(mock_tracer_provider.add_span_processor.call_count, 2)
239+
first_processor: SpanProcessor = mock_tracer_provider.add_span_processor.call_args_list[0].args[0]
240+
self.assertIsInstance(first_processor, AttributePropagatingSpanProcessor)
241+
second_processor: SpanProcessor = mock_tracer_provider.add_span_processor.call_args_list[1].args[0]
242+
self.assertIsInstance(second_processor, AwsSpanMetricsProcessor)
243+
os.environ.pop("OTEL_AWS_APP_SIGNALS_ENABLED", None)

0 commit comments

Comments
 (0)