Skip to content

refactor(gapic-generator): align telemetry compliance matrix and test engine with tidy data - #18852

Merged
chalmerlowe merged 9 commits into
mainfrom
refactor/tidy-telemetry-matrix
Oct 26, 2026
Merged

chalmerlowe merged 9 commits into
mainfrom
refactor/tidy-telemetry-matrix

Conversation

@chalmerlowe

Copy link
Copy Markdown
Contributor

Problem

In the OpenTelemetry system compliance suite (packages/gapic-generator/tests/system/), the requirements matrix (telemetry_requirements_matrix.csv) previously grouped an entire remote procedure call (RPC) scenario into a single row, even when that call produced multiple spans across different layers or retry attempts.

Because a single row had to describe up to three spans at once, individual cells packed multiple values together using custom formatting rules:

  • Combining outer client operation spans (Tier 3, or T3) and inner network attempt spans (Tier 4, or T4) in the same cell (for example, T3: CLIENT | T4: CLIENT).
  • Combining sequential retry attempts in the same cell (for example, 503 | 200 or ERROR | OK).
  • Using algebraic formulas for span counts in retry exhaustion scenarios (N + 1).

To read these combined cells, test_span_compliance.py had to maintain a custom string parser (resolve_expected_value) and pass tier and attempt indices through every assertion helper. In addition, retry exhaustion tests relied on a short wall-clock timer racing against random retry sleep intervals, which meant the number of spans produced could vary from run to run.

Solution

This pull request refactors the requirements matrix and test engine to follow tidy data principles, where each row represents a single test for one specific span and each column holds a single value:

  1. One Row per Span Assertion (telemetry_requirements_matrix.csv):

    • Added an Attempt column (0 or 1 for active spans, N/A when tracing is disabled) to identify which span is being tested.
    • Split multi-span retry scenarios (F1.5, F1.10, and F3.1 through F3.4) into separate row variants (for example, F3.1a for the T3 root span, F3.1b for the first T4 network attempt, and F3.1c for the second T4 network attempt), expanding the matrix from 22 compound rows to 32 single-span rows.
    • Replaced all N + 1 expressions with exact integer counts in Span Count (0, 1, or 2).
  2. Simplified Compliance Test Engine (test_span_compliance.py):

    • Removed the resolve_expected_value parser and simplified all attribute assertion helpers to compare each span attribute directly against its cell value.
    • Added _assert_parent_span_id so the parent_span_id column (None (Root) or T3.span_id) is validated directly from the CSV matrix.
    • Updated run_sequence_retry_call to make retry exhaustion scenarios deterministic: the test configures two UNAVAILABLE responses on the Showcase server and uses the retry policy's on_error callback to exceed the retry deadline on the second failure. This guarantees that google.api_core.retry.Retry raises RetryError after exactly two network attempts without timing flakiness.

Notes for Reviewers

  • Scope: This change is strictly a test matrix and test runner refactor in packages/gapic-generator/tests/system/. No runtime library or code generation templates are modified.
  • Cross-Tier Parentage Checks: Even though T3 and T4 assertions now live in separate rows, executing a T4 row still runs the full client call and captures both the T3 root span and T4 child spans in memory. This allows T4 rows with parent_span_id = T3.span_id to continue verifying that every child attempt span links to the parent T3 span ID.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the telemetry requirements matrix and span compliance tests to adhere to Tidy Data guidelines. The requirements matrix has been restructured into atomic rows, and the test engine has been simplified by removing the complex cell grammar parser in favor of direct assertions on target spans. Additionally, retry exhaustion tests now use a deterministic attempt count and an error hook. Feedback is provided regarding a potential ValueError in the resend count assertion if the expected value is 'N/A'.

@chalmerlowe
chalmerlowe marked this pull request as ready for review October 26, 2026 16:11
@chalmerlowe
chalmerlowe requested a review from a team as a code owner October 26, 2026 16:11
@chalmerlowe
chalmerlowe merged commit d54a6e0 into main Oct 26, 2026
125 checks passed
@chalmerlowe
chalmerlowe deleted the refactor/tidy-telemetry-matrix branch October 26, 2026 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants