refactor(test): update data plane tracing tests

[silvi-t]

PR description:

Description

• Update data plane tracing tests to reflect wasm-shim tracing improvements from Kuadrant/wasm-shim#375
• Adapt span name assertions to the new generic grpc/grpc_request/grpc_response naming scheme, using action and grpc_service tags to disambiguate
• Add new test cases for Limitador's ratelimit.limited and ratelimit.limit_name span tags

Changes

Refactoring

• test_spans_have_correct_policy_source_references: filter by operation_name == "grpc" with action tag instead of old unique operation names (auth/ratelimit)
• test_span_hierarchy: rewrite from flat dict-based hierarchy to tree-walking with assert_child helper, using action and grpc_service tags to distinguish grpc spans
• Replace assert_parent_child helper with assert_child that walks the tree via Trace.get_children() and supports tag filtering
• Add assertion for new headers span under kuadrant_filter

New Tests

• test_ratelimit_limited_tag: verify ratelimit.limited is False on 200 and True on 429
• test_ratelimit_limit_name_tag: verify ratelimit.limit_name matches the configured limit name on 429

Verification steps

Requires a cluster with the new wasm-shim (from Kuadrant/wasm-shim#375) deployed and tracing enabled.

poetry run pytest -vv testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py

Closes #1007

Summary by CodeRabbit

• Tests
  • Strengthened tracing checks for policy source references and span relationships.
  • Added validation for rate-limiting spans, including limited status and configured limit name.
  • Improved trace hierarchy assertions to verify direct parent/child span chains more precisely.

[coderabbitai]

📝 Walkthrough

Walkthrough

The tracing tests now validate policy-source spans with action-aware filtering, add ratelimit tag checks for limited and limit-name values, and assert a more detailed span hierarchy using a new direct-child helper.

Changes

Tracing span assertions

Layer / File(s)	Summary
Policy source span filters testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py	The policy-source span test is parameterised by action and policy fixture, and the span filter now requires grpc operation names, matching action tags, and matching sources tags.
Ratelimit tags and child helper testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py	The ratelimit assertions check ratelimit.limited and ratelimit.limit_name, and the span helper now matches direct children by operation name with optional tag filters.
Span hierarchy assertions testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py	The span hierarchy test uses the new child helper to assert the kuadrant_filter auth and ratelimit grpc/request/response/check chain and the headers child span.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Title: Refactor tracing tests to reflect wasm-shim rafactor #1007 — Updates the same data-plane tracing tests for the new wasm-shim tracing/span structure.

Possibly related PRs

• Kuadrant/testsuite#870 — Introduced the tracing test patterns this PR continues to use for span hierarchy and parent lookup.

Suggested reviewers

• averevki
• emmaaroche

Poem

A bunny hopped through spans so bright,
Checked grpc traces left and right.
A ratelimit blinked, then held its place,
While headers danced in rabbit grace.
Hop, hop — the tests all sing tonight 🐰

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows conventional commit format and clearly summarises the main tracing test refactor.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description follows the template with Description, Changes, and Verification sections and clearly explains the update and tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py (2)

206-213: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Make assert_child deterministic for duplicate matches.

assert_child currently returns the first match, which can hide ambiguous span structures; assert a single match to avoid order-dependent flakiness.

Proposed refactor

 def assert_child(trace, parent_span, child_op, **tags):
     """Assert that parent_span has a direct child with the given operation name and tags. Returns the child span."""
     children = trace.get_children(parent_span.span_id)
     matches = [c for c in children if c.operation_name == child_op]
     for key, value in tags.items():
         matches = [c for c in matches if c.has_tag(key, value)]
-    assert matches, f"No '{child_op}' child of '{parent_span.operation_name}' found (tags={tags})"
+    assert matches, f"No '{child_op}' child of '{parent_span.operation_name}' found (tags={tags})"
+    assert len(matches) == 1, (
+        f"Expected exactly one '{child_op}' child of '{parent_span.operation_name}' "
+        f"(tags={tags}), found {len(matches)}"
+    )
     return matches[0]

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py`
around lines 206 - 213, assert_child currently returns the first matching child
span, which can be order-dependent when multiple spans match the same operation
and tags. Update assert_child in test_kuadrant_tracing.py to verify there is
exactly one matching child after filtering by child_op and tags, and fail if
there are zero or multiple matches. Keep the existing helper signature and
locate the logic in assert_child using trace.get_children and the matches
filtering.

---

195-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid hard-coding the expected ratelimit.limit_name.

At Line 201, "testuser" is brittle; derive it from the policy fixture/config so this test stays aligned with fixture changes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py`
around lines 195 - 203, The test in test_ratelimit_limit_name_tag is hard-coding
the expected ratelimit.limit_name value, which makes it brittle. Update the
assertion in test_ratelimit_limit_name_tag to derive the expected limit name
from the existing policy fixture/config used by trace_429 rather than comparing
against the literal "testuser". Keep the span filter on should_rate_limit, but
reference the configured limit name from the same source that defines the
RateLimitPolicy so the test stays in sync with fixture changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py`:
- Line 224: The kuadrant_filter span lookup is indexing the filtered list
directly, which can raise an IndexError when no matching span exists. Update the
test around trace_200.filter_spans and kuadrant_filter to first assert that the
filtered result is non-empty, then assign the first span so failures are
explicit and easier to diagnose.

---

Nitpick comments:
In
`@testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py`:
- Around line 206-213: assert_child currently returns the first matching child
span, which can be order-dependent when multiple spans match the same operation
and tags. Update assert_child in test_kuadrant_tracing.py to verify there is
exactly one matching child after filtering by child_op and tags, and fail if
there are zero or multiple matches. Keep the existing helper signature and
locate the logic in assert_child using trace.get_children and the matches
filtering.
- Around line 195-203: The test in test_ratelimit_limit_name_tag is hard-coding
the expected ratelimit.limit_name value, which makes it brittle. Update the
assertion in test_ratelimit_limit_name_tag to derive the expected limit name
from the existing policy fixture/config used by trace_429 rather than comparing
against the literal "testuser". Keep the span filter on should_rate_limit, but
reference the configured limit name from the same source that defines the
RateLimitPolicy so the test stays in sync with fixture changes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7892b217-5916-4c68-89fc-438429ad8217

📥 Commits

Reviewing files that changed from the base of the PR and between 8fe7630 and cf62150.

📒 Files selected for processing (1)

• testsuite/tests/singlecluster/tracing/data_plane_tracing/test_kuadrant_tracing.py