Skip to content

Commit 3a87f17

Browse files
floelsconstantinius
authored andcommitted
fix(test): Fix flaky organization trace test sorting (#97831)
# Goal The goal of this PR is to fix the flakiness reported in GH-97781. The unit test `test_uptime_root_tree_with_orphaned_spans` in `tests/snuba/api/endpoints/test_organization_trace.py` is randomly failing in the CI with this error: ``` AssertionError: Span 0 differs (excluding children) assert {'additional_...06400.15, ...} == {'additional_...906400.3, ...} Omitting 15 identical items, use -vv to show Differing items: {'duration': 150.0} != {'duration': 300.0} {'description': 'Uptime Check [success] - https://www.example.com/ (200)'} != {'description': 'Uptime Check [success] - https://example.com/ (301)'} ... ``` # Explanation of the flakiness The failing assertion is: https://github.com/getsentry/sentry/blob/3fa39f0740f17dab4c934b352525dbb5978e8f6e/tests/snuba/api/endpoints/test_organization_trace.py#L669-L673 `assert_expected_results` is a helper method defined in the test class which asserts that the API response matches expected results from input trace items. To do this, it sorts both the API response and the input trace item with a `(guid, request_sequence)` custom sort key: https://github.com/getsentry/sentry/blob/3fa39f0740f17dab4c934b352525dbb5978e8f6e/tests/snuba/api/endpoints/test_organization_trace.py#L465-L476 The issue is that the API response items don't have an `attributes` key but only an `additional_attributes` key. In the `sort_key` method, their `guid` will therefore default to their `event_id`, which is a randomly-generated UUID. While input trace items (`[redirect_result, final_result]`) will be ordered deterministically (based on their `request_sequence`, since they have the same `guid` of `check-123`), API response items will be ordered randomly based on their random UUID, making the assertion fail randomly (because the assertion doesn't compare related items). You can easily reproduce the failure locally by running the test 10 times in a row: ``` for i in {1..10}; do pytest tests/snuba/api/endpoints/test_organization_trace.py::OrganizationEventsTraceEndpointTest::test_with_uptime_results || exit 1; done ``` It should fail at least once. # Fix The fix is simple: rely on `additional_attributes["guid"]` to sort the API response items. Incidentally, it's exactly what is done to get the sequence number: https://github.com/getsentry/sentry/blob/3fa39f0740f17dab4c934b352525dbb5978e8f6e/tests/snuba/api/endpoints/test_organization_trace.py#L471-L475 We just need to use the same syntax. # Reproduction Run the test 10 times in a row locally (same command as above) and see that it now passes consistently. # Legal Boilerplate Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
1 parent 4503825 commit 3a87f17

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

tests/snuba/api/endpoints/test_organization_trace.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ def sort_key(item):
466466
guid = (
467467
item.attributes.get("guid", ProtoAttributeValue(val_str="")).string_value
468468
if hasattr(item, "attributes")
469-
else item.get("event_id", "")
469+
else item.get("additional_attributes", {}).get("guid", "")
470470
)
471471
seq = (
472472
item.attributes.get("request_sequence", ProtoAttributeValue(val_int=0)).int_value

0 commit comments

Comments
 (0)