Skip to content

Commit

Permalink
Fix outstanding lint issues in instrumentation-aws-lambda
Browse files Browse the repository at this point in the history
  • Loading branch information
jbfenton committed Jun 21, 2024
1 parent 90b017b commit 4ad3d7e
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ def force_flush(self, timeout: int) -> None:
None
"""

...


class FlushableTracerProvider(TracerProvider):
"""
Expand All @@ -152,8 +150,6 @@ def force_flush(self, timeout: int) -> None:
None
"""

...


def provider_warning(provider_type: str):
"""
Expand All @@ -166,9 +162,10 @@ def provider_warning(provider_type: str):
"""

return lambda timeout: logger.warning(
f"{provider_type} was missing `force_flush` method. "
"%s was missing `force_flush` method. "
"This is necessary in case of a Lambda freeze and would exist in the "
"OTel SDK implementation."
"OTel SDK implementation.",
provider_type,
)


Expand Down Expand Up @@ -260,6 +257,10 @@ class EventWrapper:
General purpose wrapper for Lambda events.
"""

# This class is intended to be subclassed, so it has methods that are not, or partially implemented.
# Keeping the signatures consistent for the subclasses to implement.
# pylint:disable=R0201

def __init__(self, event, context):
self._event = event
self._context = context
Expand All @@ -274,8 +275,8 @@ def headers(self) -> dict:

if self._event and "headers" in self._event:
return self._event["headers"]
else:
return {}

return {}

@property
def span_kind(self) -> SpanKind:
Expand Down Expand Up @@ -357,12 +358,12 @@ def headers(self) -> dict:
headers: List[Tuple[bytes, bytes]] = []

if "multiValueHeaders" in self._event:
for k, v in self._event["multiValueHeaders"].items():
for inner_v in v:
headers.append((k.lower().encode(), inner_v.encode()))
for key, value in self._event["multiValueHeaders"].items():
for inner_v in value:
headers.append((key.lower().encode(), inner_v.encode()))
else:
for k, v in self._event["headers"].items():
headers.append((k.lower().encode(), v.encode()))
for key, value in self._event["headers"].items():
headers.append((key.lower().encode(), value.encode()))

# Unique headers. If there are duplicates, it will use the last defined.
uq_headers = {k.decode(): v.decode() for k, v in headers}
Expand Down Expand Up @@ -677,6 +678,8 @@ def get_event_wrapper(event: LambdaEvent, context: Any) -> EventWrapper:

# The logic behind this flow was adapted from work done by the Mangum project:
# https://github.com/jordaneremieff/mangum
# To reduce cognitive load, preserving if/elif structure.
# pylint:disable=R1705
if not event:
return default_wrapper
elif "requestContext" in event and "elb" in event["requestContext"]:
Expand Down Expand Up @@ -766,7 +769,6 @@ def flush(
logger.exception("MeterProvider failed to flush metrics")


# pylint: disable=too-many-statements
def _instrument(
wrapped_module_name,
wrapped_function_name,
Expand All @@ -775,8 +777,6 @@ def _instrument(
tracer_provider: FlushableTracerProvider,
meter_provider: FlushableMeterProvider,
):
# pylint: disable=too-many-locals
# pylint: disable=too-many-statements
def _instrumented_lambda_handler_call( # noqa pylint: disable=too-many-branches
call_wrapped, instance, args, kwargs
):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,20 @@
)


class TestDetermineFlushTimeout:
class TestDetermineFlushTimeout(TestCase):
default = 30000
timeout_var = OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT

def test_flush_timeout_default(self):
assert determine_flush_timeout() == self.default
self.assertEqual(determine_flush_timeout(), self.default)

@patch.dict("os.environ", {timeout_var: "1000"})
def test_flush_timeout_env_var(self):
assert determine_flush_timeout() == 1000
self.assertEqual(determine_flush_timeout(), 1000)

@patch.dict("os.environ", {timeout_var: "abc"})
def test_flush_timeout_env_var_invalid(self):
assert determine_flush_timeout() == self.default
self.assertEqual(determine_flush_timeout(), self.default)


class TestIsAWSContextPropagationDisabled(TestCase):
Expand Down Expand Up @@ -87,7 +87,6 @@ def test_env_var_not_set(self):


class TestFlush(TestCase):

@patch("time.time", return_value=0)
@patch(
"opentelemetry.instrumentation.aws_lambda.determine_flush_timeout",
Expand All @@ -101,7 +100,9 @@ def test_successful_flush(self, mock_time, mock_determine_flush_timeout):

flush(tracer_provider, meter_provider)

self.assertEqual(tracer_provider.force_flush.call_count, 1)
tracer_provider.force_flush.assert_called_once_with(1000)
self.assertEqual(meter_provider.force_flush.call_count, 1)
meter_provider.force_flush.assert_called_once_with(1000)

@patch("time.time", return_value=0)
Expand All @@ -119,7 +120,7 @@ def test_missing_force_flush_in_tracer_provider(

flush(tracer_provider, meter_provider)

assert "force_flush" not in dir(tracer_provider)
self.assertNotIn("force_flush", dir(tracer_provider))
meter_provider.force_flush.assert_called_once_with(1000)

@patch("time.time", return_value=0)
Expand All @@ -137,7 +138,7 @@ def test_missing_force_flush_in_meter_provider(

flush(tracer_provider, meter_provider)

assert "force_flush" not in dir(meter_provider)
self.assertNotIn("force_flush", dir(meter_provider))
tracer_provider.force_flush.assert_called_once_with(1000)

@patch("time.time", return_value=0)
Expand All @@ -157,6 +158,7 @@ def test_exception_in_force_flush_tracer_provider(

flush(tracer_provider, meter_provider)

self.assertEqual(meter_provider.force_flush.call_count, 1)
meter_provider.force_flush.assert_called_once_with(1000)

@patch("time.time", return_value=0)
Expand All @@ -176,4 +178,5 @@ def test_exception_in_force_flush_meter_provider(

flush(tracer_provider, meter_provider)

self.assertEqual(tracer_provider.force_flush.call_count, 1)
tracer_provider.force_flush.assert_called_once_with(1000)

0 comments on commit 4ad3d7e

Please sign in to comment.