Skip to content

Commit

Permalink
feat(tracer): support methods with the same name (ABCs) by including …
Browse files Browse the repository at this point in the history
…fully qualified name in v2 (#1486)

Co-authored-by: Heitor Lessa <[email protected]>
Co-authored-by: Ruben Fonseca <[email protected]>
  • Loading branch information
3 people authored Sep 8, 2022
1 parent 64513d9 commit 3690943
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 19 deletions.
6 changes: 4 additions & 2 deletions aws_lambda_powertools/tracing/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ def capture_method(
"""Decorator to create subsegment for arbitrary functions
It also captures both response and exceptions as metadata
and creates a subsegment named `## <method_name>`
and creates a subsegment named `## <method_module.method_qualifiedname>`
# see here: [Qualified name for classes and functions](https://peps.python.org/pep-3155/)
When running [async functions concurrently](https://docs.python.org/3/library/asyncio-task.html#id6),
methods may impact each others subsegment, and can trigger
Expand Down Expand Up @@ -508,7 +509,8 @@ async def async_tasks():
functools.partial(self.capture_method, capture_response=capture_response, capture_error=capture_error),
)

method_name = f"{method.__name__}"
# Example: app.ClassA.get_all # noqa E800
method_name = f"{method.__module__}.{method.__qualname__}"

capture_response = resolve_truthy_env_var_choice(
env=os.getenv(constants.TRACER_CAPTURE_RESPONSE_ENV, "true"), choice=capture_response
Expand Down
Binary file modified docs/media/tracer_utility_showcase.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
33 changes: 33 additions & 0 deletions tests/e2e/tracer/handlers/same_function_name.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
from abc import ABC, abstractmethod
from uuid import uuid4

from aws_lambda_powertools import Tracer
from aws_lambda_powertools.utilities.typing import LambdaContext

tracer = Tracer()


class MainAbstractClass(ABC):
@abstractmethod
def get_all(self):
raise NotImplementedError


class Comments(MainAbstractClass):
@tracer.capture_method
def get_all(self):
return [{"id": f"{uuid4()}", "completed": False} for _ in range(5)]


class Todos(MainAbstractClass):
@tracer.capture_method
def get_all(self):
return [{"id": f"{uuid4()}", "completed": False} for _ in range(5)]


def lambda_handler(event: dict, context: LambdaContext):

todos = Todos()
comments = Comments()

return {"todos": todos.get_all(), "comments": comments.get_all()}
42 changes: 38 additions & 4 deletions tests/e2e/tracer/test_tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ def basic_handler_fn(infrastructure: dict) -> str:
return infrastructure.get("BasicHandler", "")


@pytest.fixture
def same_function_name_fn(infrastructure: dict) -> str:
return infrastructure.get("SameFunctionName", "")


@pytest.fixture
def same_function_name_arn(infrastructure: dict) -> str:
return infrastructure.get("SameFunctionNameArn", "")


@pytest.fixture
def async_fn_arn(infrastructure: dict) -> str:
return infrastructure.get("AsyncCaptureArn", "")
Expand All @@ -31,9 +41,9 @@ def test_lambda_handler_trace_is_visible(basic_handler_fn_arn: str, basic_handle
handler_subsegment = f"## {handler_name}"
handler_metadata_key = f"{handler_name} response"

method_name = basic_handler.get_todos.__name__
method_name = f"basic_handler.{basic_handler.get_todos.__name__}"
method_subsegment = f"## {method_name}"
handler_metadata_key = f"{method_name} response"
method_metadata_key = f"{method_name} response"

trace_query = data_builder.build_trace_default_query(function_name=basic_handler_fn)

Expand All @@ -46,14 +56,38 @@ def test_lambda_handler_trace_is_visible(basic_handler_fn_arn: str, basic_handle

assert len(trace.get_annotation(key="ColdStart", value=True)) == 1
assert len(trace.get_metadata(key=handler_metadata_key, namespace=TracerStack.SERVICE_NAME)) == 2
assert len(trace.get_metadata(key=handler_metadata_key, namespace=TracerStack.SERVICE_NAME)) == 2
assert len(trace.get_metadata(key=method_metadata_key, namespace=TracerStack.SERVICE_NAME)) == 2
assert len(trace.get_subsegment(name=handler_subsegment)) == 2
assert len(trace.get_subsegment(name=method_subsegment)) == 2


def test_lambda_handler_trace_multiple_functions_same_name(same_function_name_arn: str, same_function_name_fn: str):
# GIVEN
method_name_todos = "same_function_name.Todos.get_all"
method_subsegment_todos = f"## {method_name_todos}"
method_metadata_key_todos = f"{method_name_todos} response"

method_name_comments = "same_function_name.Comments.get_all"
method_subsegment_comments = f"## {method_name_comments}"
method_metadata_key_comments = f"{method_name_comments} response"

trace_query = data_builder.build_trace_default_query(function_name=same_function_name_fn)

# WHEN
_, execution_time = data_fetcher.get_lambda_response(lambda_arn=same_function_name_arn)

# THEN
trace = data_fetcher.get_traces(start_date=execution_time, filter_expression=trace_query)

assert len(trace.get_metadata(key=method_metadata_key_todos, namespace=TracerStack.SERVICE_NAME)) == 1
assert len(trace.get_metadata(key=method_metadata_key_comments, namespace=TracerStack.SERVICE_NAME)) == 1
assert len(trace.get_subsegment(name=method_subsegment_todos)) == 1
assert len(trace.get_subsegment(name=method_subsegment_comments)) == 1


def test_async_trace_is_visible(async_fn_arn: str, async_fn: str):
# GIVEN
async_fn_name = async_capture.async_get_users.__name__
async_fn_name = f"async_capture.{async_capture.async_get_users.__name__}"
async_fn_name_subsegment = f"## {async_fn_name}"
async_fn_name_metadata_key = f"{async_fn_name} response"

Expand Down
58 changes: 45 additions & 13 deletions tests/unit/test_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

# Maintenance: This should move to Functional tests and use Fake over mocks.

MODULE_PREFIX = "unit.test_tracing"


@pytest.fixture
def dummy_response():
Expand Down Expand Up @@ -125,9 +127,13 @@ def greeting(name, message):
# and add its response as trace metadata
# and use service name as a metadata namespace
assert in_subsegment_mock.in_subsegment.call_count == 1
assert in_subsegment_mock.in_subsegment.call_args == mocker.call(name="## greeting")
assert in_subsegment_mock.in_subsegment.call_args == mocker.call(
name=f"## {MODULE_PREFIX}.test_tracer_method.<locals>.greeting"
)
assert in_subsegment_mock.put_metadata.call_args == mocker.call(
key="greeting response", value=dummy_response, namespace="booking"
key=f"{MODULE_PREFIX}.test_tracer_method.<locals>.greeting response",
value=dummy_response,
namespace="booking",
)


Expand Down Expand Up @@ -253,7 +259,10 @@ def greeting(name, message):
# THEN we should add the exception using method name as key plus error
# and their service name as the namespace
put_metadata_mock_args = in_subsegment_mock.put_metadata.call_args[1]
assert put_metadata_mock_args["key"] == "greeting error"
assert (
put_metadata_mock_args["key"]
== f"{MODULE_PREFIX}.test_tracer_method_exception_metadata.<locals>.greeting error"
)
assert put_metadata_mock_args["namespace"] == "booking"


Expand Down Expand Up @@ -305,15 +314,23 @@ async def greeting(name, message):

# THEN we should add metadata for each response like we would for a sync decorated method
assert in_subsegment_mock.in_subsegment.call_count == 2
assert in_subsegment_greeting_call_args == mocker.call(name="## greeting")
assert in_subsegment_greeting2_call_args == mocker.call(name="## greeting_2")
assert in_subsegment_greeting_call_args == mocker.call(
name=f"## {MODULE_PREFIX}.test_tracer_method_nested_async.<locals>.greeting"
)
assert in_subsegment_greeting2_call_args == mocker.call(
name=f"## {MODULE_PREFIX}.test_tracer_method_nested_async.<locals>.greeting_2"
)

assert in_subsegment_mock.put_metadata.call_count == 2
assert put_metadata_greeting2_call_args == mocker.call(
key="greeting_2 response", value=dummy_response, namespace="booking"
key=f"{MODULE_PREFIX}.test_tracer_method_nested_async.<locals>.greeting_2 response",
value=dummy_response,
namespace="booking",
)
assert put_metadata_greeting_call_args == mocker.call(
key="greeting response", value=dummy_response, namespace="booking"
key=f"{MODULE_PREFIX}.test_tracer_method_nested_async.<locals>.greeting response",
value=dummy_response,
namespace="booking",
)


Expand Down Expand Up @@ -355,7 +372,10 @@ async def greeting(name, message):
# THEN we should add the exception using method name as key plus error
# and their service name as the namespace
put_metadata_mock_args = in_subsegment_mock.put_metadata.call_args[1]
assert put_metadata_mock_args["key"] == "greeting error"
assert (
put_metadata_mock_args["key"]
== f"{MODULE_PREFIX}.test_tracer_method_exception_metadata_async.<locals>.greeting error"
)
assert put_metadata_mock_args["namespace"] == "booking"


Expand Down Expand Up @@ -387,7 +407,9 @@ def handler(event, context):
assert "test result" in in_subsegment_mock.put_metadata.call_args[1]["value"]
assert in_subsegment_mock.in_subsegment.call_count == 2
assert handler_trace == mocker.call(name="## handler")
assert yield_function_trace == mocker.call(name="## yield_with_capture")
assert yield_function_trace == mocker.call(
name=f"## {MODULE_PREFIX}.test_tracer_yield_from_context_manager.<locals>.yield_with_capture"
)
assert "test result" in result


Expand All @@ -411,7 +433,10 @@ def yield_with_capture():
# THEN we should add the exception using method name as key plus error
# and their service name as the namespace
put_metadata_mock_args = in_subsegment_mock.put_metadata.call_args[1]
assert put_metadata_mock_args["key"] == "yield_with_capture error"
assert (
put_metadata_mock_args["key"]
== f"{MODULE_PREFIX}.test_tracer_yield_from_context_manager_exception_metadata.<locals>.yield_with_capture error" # noqa E501
)
assert isinstance(put_metadata_mock_args["value"], ValueError)
assert put_metadata_mock_args["namespace"] == "booking"

Expand Down Expand Up @@ -453,7 +478,9 @@ def handler(event, context):
assert "test result" in in_subsegment_mock.put_metadata.call_args[1]["value"]
assert in_subsegment_mock.in_subsegment.call_count == 2
assert handler_trace == mocker.call(name="## handler")
assert yield_function_trace == mocker.call(name="## yield_with_capture")
assert yield_function_trace == mocker.call(
name=f"## {MODULE_PREFIX}.test_tracer_yield_from_nested_context_manager.<locals>.yield_with_capture"
)
assert "test result" in result


Expand Down Expand Up @@ -483,7 +510,9 @@ def handler(event, context):
assert "test result" in in_subsegment_mock.put_metadata.call_args[1]["value"]
assert in_subsegment_mock.in_subsegment.call_count == 2
assert handler_trace == mocker.call(name="## handler")
assert generator_fn_trace == mocker.call(name="## generator_fn")
assert generator_fn_trace == mocker.call(
name=f"## {MODULE_PREFIX}.test_tracer_yield_from_generator.<locals>.generator_fn"
)
assert "test result" in result


Expand All @@ -506,7 +535,10 @@ def generator_fn():
# THEN we should add the exception using method name as key plus error
# and their service name as the namespace
put_metadata_mock_args = in_subsegment_mock.put_metadata.call_args[1]
assert put_metadata_mock_args["key"] == "generator_fn error"
assert (
put_metadata_mock_args["key"]
== f"{MODULE_PREFIX}.test_tracer_yield_from_generator_exception_metadata.<locals>.generator_fn error"
)
assert put_metadata_mock_args["namespace"] == "booking"
assert isinstance(put_metadata_mock_args["value"], ValueError)
assert str(put_metadata_mock_args["value"]) == "test"
Expand Down

0 comments on commit 3690943

Please sign in to comment.