Skip to content

Add bounded workflow label metrics dimensions - #1226

Open
jiaenren wants to merge 4 commits into
mainfrom
jiaenr/osmo-6501-b8-label-metrics
Open

Add bounded workflow label metrics dimensions#1226
jiaenren wants to merge 4 commits into
mainfrom
jiaenr/osmo-6501-b8-label-metrics

Conversation

@jiaenren

@jiaenren jiaenren commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Issue - None

[OSMO-6501] Track B8 of the workflow-labels PR split.

  • osmo_tasks_count gains one workflow_label_<key> attribute per curated policy key (any enforcement mode), value-clamped to the allow-list plus <missing> / <other> sentinels so cardinality stays bounded at |allow_list|+2 per key.
  • Fixes the pre-existing counting defect: the query grouped by every column without COUNT(*), so each series read 1; it now selects and sums real counts.
  • Label keys are escaped into Prometheus-safe attribute names (-, ., /, _ collision-free).
  • The OSS workflow_resources_usage.json dashboard stays free of any deployment-specific label dimension, pinned by a new structural test; label-specific dashboards are provisioned per deployment.
  • The osmo_label_validation_total{key,outcome} submit-gate counter landed with Enforce workflow label policy at submission #1223 (it is emitted inside the admission gate).

Stacked on #1225 for chain simplicity; functionally depends only on the schema (#1219) and policy config (#1220).
Extracted from prototype #1194.

Testing

  • bazel test //src/service/core/workflow/tests:test_workflow_metrics (clamping matrix, sentinel values, attribute-name escaping, count aggregation, config-failure fallback)
  • bazel test //src/service/core/workflow/tests:test_helpers
  • bazel test //docs/deployment_guide/dashboards:test_dashboards

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added workflow-label dimensions to task metrics, including handling for missing, unexpected, and unsupported values.
    • Documented workflow metric labels, validation outcomes, and cardinality safeguards.
    • Added Grafana dashboard validation coverage for panel structure and deployment-neutral configuration.
  • Bug Fixes

    • Improved recent-task aggregation to report grouped task counts with workflow labels.
  • Tests

    • Added comprehensive coverage for workflow metrics, label policies, caching, disabled metrics, and dashboard configuration.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1e843ce4-0b6a-47b5-a4cb-06d1bd536125

📥 Commits

Reviewing files that changed from the base of the PR and between b8fe273 and f3bcad8.

📒 Files selected for processing (8)
  • docs/deployment_guide/dashboards/BUILD
  • docs/deployment_guide/dashboards/test_dashboards.py
  • docs/deployment_guide/references/configs_definitions/workflow.rst
  • src/service/core/workflow/helpers.py
  • src/service/core/workflow/tests/BUILD
  • src/service/core/workflow/tests/test_helpers.py
  • src/service/core/workflow/tests/test_workflow_metrics.py
  • src/service/core/workflow/workflow_metrics.py

📝 Walkthrough

Walkthrough

Workflow task metrics now include curated workflow labels with bounded Prometheus attributes. Task queries return grouped counts. Tests cover aggregation, caching, disabled metrics, registration, and dashboard structure. Documentation describes label dimensions and validation metrics.

Changes

Workflow metrics

Layer / File(s) Summary
Aggregated task-count query
src/service/core/workflow/helpers.py, src/service/core/workflow/tests/test_helpers.py
get_recent_tasks selects workflow labels and COUNT(*), then groups by workflow UUID, status, and labels.
Curated workflow-label metrics
src/service/core/workflow/workflow_metrics.py, src/service/core/workflow/tests/test_workflow_metrics.py, docs/deployment_guide/references/configs_definitions/workflow.rst, src/service/core/workflow/tests/BUILD
Workflow label policies are loaded and projected into bounded metric attributes. Tests cover fallback values, collisions, caching, disabled metrics, and registration metadata. Documentation defines the label dimensions and validation counter.
Dashboard and build validation
docs/deployment_guide/dashboards/BUILD, docs/deployment_guide/dashboards/test_dashboards.py
A Bazel test target validates dashboard panel structure, variables, queries, and Kubernetes pod metrics.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • NVIDIA/OSMO#1224: Both changes modify workflow helpers and workflow-label handling.
  • NVIDIA/OSMO#1225: This change adds metrics for workflow labels documented and configured by that PR.

Suggested reviewers: ryalinvidia, vvnpn-nv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding bounded workflow-label metric dimensions.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jiaenr/osmo-6501-b8-label-metrics

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

@jiaenren jiaenren mentioned this pull request Jul 22, 2026
3 tasks
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from b463ab3 to c300e64 Compare July 22, 2026 05:30
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from 5056561 to fc3b5d2 Compare July 22, 2026 22:00
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from fc3b5d2 to f9f2198 Compare July 22, 2026 22:08
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from f9f2198 to 328368a Compare July 23, 2026 22:16
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from 328368a to cb3b5ec Compare July 27, 2026 23:57
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from cb3b5ec to 709b4e9 Compare July 28, 2026 00:07
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from 709b4e9 to d1b9fae Compare July 28, 2026 00:38
@jiaenren jiaenren closed this Jul 28, 2026
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from d1b9fae to 0cfa33a Compare July 28, 2026 00:47
@jiaenren jiaenren reopened this Jul 28, 2026
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from 77c1694 to a8ba11c Compare July 28, 2026 20:29
@github-actions

Copy link
Copy Markdown

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.91%. Comparing base (b8fe273) to head (f3bcad8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1226      +/-   ##
==========================================
+ Coverage   66.76%   66.91%   +0.14%     
==========================================
  Files         202      202              
  Lines       26050    26076      +26     
  Branches     3938     3942       +4     
==========================================
+ Hits        17393    17448      +55     
+ Misses       7917     7887      -30     
- Partials      740      741       +1     
Flag Coverage Δ
backend 69.61% <100.00%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/service/core/workflow/helpers.py 99.66% <ø> (ø)
src/service/core/workflow/workflow_metrics.py 90.90% <100.00%> (+61.49%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from a8ba11c to a4f2ef4 Compare July 29, 2026 22:45
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from a4f2ef4 to 9f2e862 Compare July 30, 2026 18:22
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from 9f2e862 to 324664b Compare July 30, 2026 22:31
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from 324664b to fcf6b49 Compare July 30, 2026 23:47
Base automatically changed from jiaenr/osmo-6501-b7-label-cli to main July 31, 2026 21:59
jiaenren and others added 4 commits July 31, 2026 15:01
Add one prefixed attribute per curated policy key to osmo_tasks_count,
clamped to the allow-list plus <missing>/<other> sentinels so series
stay bounded in every enforcement mode. Fix the gauge's counting defect
by selecting COUNT(*) instead of emitting presence rows. Pin the OSS
Workflow Resources dashboard as deployment-neutral (no PPP binding)
with a structural test; PPP panels ship via internal provisioning.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop the unreachable JSON-text parsing branch (psycopg2 decodes JSONB
to dicts), hoist loop-invariant attribute names, consolidate the
triplicated patch scaffolding in the metrics tests, loosen
change-detector assertions, and replace the dashboard title pinning
with a deployment-neutrality scan. Re-home the osmo_tasks_count
label-dimension reference docs from the CLI PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Correct the aggregation comment (clamping can collapse distinct rows
today), explain the sentinel angle-bracket choice at both the code and
docs sites, refresh the stale get_recent_tasks docstring, and note the
Grafana schema quirk behind the query helper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the NVIDIA-internal 'PPP' example key with the generic 'project'
in the metrics test, dashboard-neutrality test, and the metrics docs, to
match merged #1220.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jiaenren
jiaenren force-pushed the jiaenr/osmo-6501-b8-label-metrics branch from fcf6b49 to f3bcad8 Compare July 31, 2026 22:02
@jiaenren
jiaenren marked this pull request as ready for review July 31, 2026 23:04
@jiaenren
jiaenren requested a review from a team as a code owner July 31, 2026 23:04
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.

1 participant