Measure metric metadata coverage in CI#23814
Draft
nubtron wants to merge 1 commit into
Draft
Conversation
Contributor
|
Contributor
Validation ReportAll 21 validations passed. Show details
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Temporarily instruments
AggregatorStub.assert_metrics_using_metadataso each assertion can emit one metric metadata coverage JSON object during CI. The existing metadata assertion behavior is intentionally unchanged: submitted metrics that are not inmetadata.csv, type mismatches, and symmetric missing submissions still fail only in the same situations as before.When
DD_INTEGRATION_METRIC_COVERAGE=1is set, each assertion writes a JSON line toDD_INTEGRATION_METRIC_COVERAGE_FILE. In GitHub test-target CI, it also falls back to$TEST_RESULTS_DIR/metric-metadata-coverage.jsonlso reports are uploaded with the existing test-results artifact. Each report is also printed as a greppable line prefixed withDD_INTEGRATION_METRIC_COVERAGE. The PR enables this in the GitHub test-target unit/integration and E2E scripts and stores the JSONL file under the existing test-results artifact directory.Key fields:
integration/check_name: best-effort CI target/check context.pytest_nodeid: test node fromPYTEST_CURRENT_TESTwhen pytest provides it.submitted_count: unique submitted metric names considered by the assertion afterexcludeis applied.metadata_count: metric names present inmetadata.csvfor the assertion input.covered_count: metric names present in both submissions and metadata.coverage_percent:covered_count / metadata_count * 100.missing_metric_names: metadata rows not observed in this assertion.emitted_not_in_metadata: submitted metric names without metadata rows.type_mismatches: metrics whose observed type does not match metadata.excluded_metrics: metrics passed through the assertion'sexcludeparameter.Motivation
This is a draft PR experiment to measure integration metric metadata coverage in CI before deciding whether and where to add stricter gates. The output should make low-coverage tests and integrations visible without broadly failing on missing submitted coverage.
Interpretation guidance:
coverage_percentmeans the test only emitted a subset of metadata-defined metrics; this is expected for some narrow tests and useful for finding E2E gaps.emitted_not_in_metadatais already assertion-failing behavior today unless that metric is excluded.type_mismatchesis already assertion-failing behavior today when type checks are enabled.excluded_metricsshould be treated as intentional blind spots. Per-test exclusions should include a nearby code comment explaining why each metric is excluded, for example dynamic version behavior, optional backend features, or known fixture limitations. Avoid adding unexplained broad exclusions because they reduce the usefulness of coverage stats.Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged