chore(tests): refactor E2E tracer to ease maintenance, writing tests and parallelization

[heitorlessa]

Issue number: #1440

Summary

This PR refactors Tracer E2E test following the new Infrastructure and explicit tests. Unrelated to Tracer, it also breaks up the various helpers into their own respective modules (data_fetcher/traces.py and data_builder/metrics.py).

Tasks

• Refactor tracer fetching recursive logic into TraceFetcher
• Update get_traces factory to return TraceFetcher
• Self-document trace segments, trace documents, etc. to ease maintenance
• Create TracerStack and use new infrastructure
• Pass POWERTOOLS_SERVICE_NAME as Env Var; Tracer needs changing to allow dynamic service injection
• Test capture lambda handler and cold start annotation
• Test methods (sync, async)
• Break data builders into their own module
• Ensure minimum number of traces are found before returning to address flaky tests

Changes

Please provide a summary of what's being changed

User experience

Please share what the user experience looks like before and after this change

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• Meet tenets criteria
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented
• PR title follows conventional commit semantics

Is this a breaking change?

RFC issue number:

Checklist:

• Migration process documented
• Implement warnings (if it can live side by side)

Acknowledgment

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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[heitorlessa]

Test differences

Besides data builders, I've encapsulated trace fetching and manipulation of different views in its own class. The logic is largely the same, what changed is that we have more flexibility in the type of trace we want (origin, segment name, etc.) and have getters to ease assertion.

Before

def test_basic_lambda_async_trace_visible(execute_lambda: conftest.InfrastructureOutput, config: conftest.LambdaConfig):
    # GIVEN
    lambda_name = execute_lambda.get_lambda_function_name(cf_output_name="basichandlerarn")
    start_date = execute_lambda.get_lambda_execution_time()
    end_date = start_date + datetime.timedelta(minutes=5)
    trace_filter_exporession = f'service("{lambda_name}")'

    # WHEN
    trace = helpers.get_traces(
        start_date=start_date,
        end_date=end_date,
        filter_expression=trace_filter_exporession,
        xray_client=boto3.client("xray"),
    )

    # THEN
    info = helpers.find_trace_additional_info(trace=trace)
    print(info)
    handler_trace_segment = [trace_segment for trace_segment in info if trace_segment.name == "## lambda_handler"][0]
    collect_payment_trace_segment = [
        trace_segment for trace_segment in info if trace_segment.name == "## collect_payment"
    ][0]

    annotation_key = config["environment_variables"]["ANNOTATION_KEY"]
    expected_value = config["environment_variables"]["ANNOTATION_VALUE"]
    expected_async_value = config["environment_variables"]["ANNOTATION_ASYNC_VALUE"]

    assert handler_trace_segment.annotations["Service"] == "e2e-tests-app"
    assert handler_trace_segment.metadata["e2e-tests-app"][annotation_key] == expected_value
    assert collect_payment_trace_segment.metadata["e2e-tests-app"][annotation_key] == expected_async_value

After

def test_lambda_handler_trace_is_visible(basic_handler_fn_arn: str, basic_handler_fn: str):
    # GIVEN
    handler_name = basic_handler.lambda_handler.__name__
    handler_subsegment = f"## {handler_name}"
    handler_metadata_key = f"{handler_name} response"
    trace_query = data_builder.build_trace_default_query(function_name=basic_handler_fn)

    # WHEN
    _, execution_time = helpers.trigger_lambda(lambda_arn=basic_handler_fn_arn)
    helpers.trigger_lambda(lambda_arn=basic_handler_fn_arn)

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

    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_subsegment(name=handler_subsegment)) == 2

[leandrodamascena]

Huge improvement in readability and code quality @heitorlessa. Too easy to understand now 🎸

[codecov-commenter]

Codecov Report

Merging #1457 (92df461) into develop (83fb6cc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #1457   +/-   ##
========================================
  Coverage    99.89%   99.89%           
========================================
  Files          121      121           
  Lines         5479     5479           
  Branches       627      627           
========================================
  Hits          5473     5473           
  Misses           2        2           
  Partials         4        4           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[heitorlessa]

@mploski @rubenfonseca could either of you review before I start Logger and complete the refactoring?

Thanks a lot!

[rubenfonseca]

Just two tiny comments :)

[heitorlessa]

All feedback addressed @mploski - when you're ready

[heitorlessa]

I'm gonna simplify get_metrics return to return the Values so tests are easier to write too ...... but in another PR.

[heitorlessa]

Thanks a lot for the review and suggested changes @mploski @rubenfonseca ! Merging.