OSAC-3149: PRD: Metering for Network Bandwidth - #160
Conversation
Add PRD for network bandwidth metering covering per-tenant ingress/egress GiB transferred with consumption-based metering model dependent on networking vendor integration. Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Moti Asayag <masayag@redhat.com> Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Moti Asayag <masayag@redhat.com>
|
@masayag: This pull request references OSAC-3149 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughChangesNetwork bandwidth metering
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
AI EP Review: EP-160Score: 10/10 | Verdict: PASS
Verdict: A strong, tightly-scoped PRD that clearly describes network bandwidth metering as a user-facing capability, with concrete business justification, four well-differentiated persona stories, and testable acceptance criteria — minor gaps are limited to the UI dimension not being addressed. Feedback: State whether UI console views for bandwidth data are in scope, API/CLI-only, or deferred to a later milestone — the PRD says users can 'view' bandwidth but doesn't specify through which surface, and osac-dimensions.md requires this declaration. Consider restating the specific Part 1 cross-cutting acceptance criteria in AC #4 rather than referencing them by name, so a tester can verify bandwidth meters without reading the Part 1 PRD. Critical (0)None. Important (1)
Suggestions (2)
Review costModel: claude-opus-4-6 |
|
|
||
| OSAC provides usage data. The provider applies their own price schedule to generate charges. This section defines the metering units and formulas for bandwidth, extending the charge calculation model from [Part 1](/enhancements/metering-and-usage-tracking/prd.md). | ||
|
|
||
| Bandwidth is a consumption meter. Unlike the resource-based allocation meters in sibling PRDs, it is driven by traffic volume rather than time. |
There was a problem hiding this comment.
I guess it's similar to MaaS (token volume)
There was a problem hiding this comment.
Yes, exactly — bandwidth is a consumption meter driven by volume (GiB transferred) rather than time, similar to how MaaS meters token counts. The sibling PRDs (BMaaS, Storage, Networking resources) use allocation meters tied to resource existence duration.
| ### 10.1 Bandwidth data source unidentified | ||
|
|
||
| - **Owner:** OSAC platform team / Networking team | ||
| - **Mitigation:** No networking vendor has been selected to provide per-tenant ingress/egress traffic counters. Without a data source, bandwidth metering cannot be implemented. Engage Netris and OVN-Kubernetes teams during design to evaluate options. Bandwidth metering may ship after other Part 2 meters if the vendor integration is not ready. |
There was a problem hiding this comment.
What does "No networking vendor has been selected to provide per-tenant ingress/egress traffic counters" mean?
Do they not support these counters?
There was a problem hiding this comment.
Good question — the wording was ambiguous. The risk is not that no vendor has been selected, but that it has not been confirmed whether the vendors already integrated with OSAC (Netris, OVN-Kubernetes) expose per-tenant traffic counter APIs that OSAC can consume for metering. Updated Risk 10.1 in the latest push to clarify this.
Apply the same terminology cleanup as sibling metering PRDs: fix broken Part 1 links to OSAC-985 renamed path, replace pricing/costing language with metering equivalents, rename "Charge Calculation Model" to "Usage Calculation Model" with pure accumulation rules (no dollar amounts), inline Part 1 cross-cutting ACs, and add UI out-of-scope statement consistent with other metering PRDs. Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Moti Asayag <masayag@redhat.com> Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Moti Asayag <masayag@redhat.com>
Clarify Risk 10.1 per avishayt's review comment: the risk is not that no vendor has been selected, but that it has not been confirmed whether the vendors already integrated with OSAC (Netris, OVN-Kubernetes) expose per-tenant traffic counter APIs that OSAC can consume for metering. Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Moti Asayag <masayag@redhat.com> Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Moti Asayag <masayag@redhat.com>
AI EP Review: EP-160Score: 8/10 | Verdict: PASS
Verdict: A well-structured, focused PRD for network bandwidth metering with clear personas and business justification, held back slightly by mild design leakage in capabilities/acceptance criteria and a few internal-facing acceptance criteria that a PM couldn't verify by using the product. Feedback: Rewrite CAP-3 as a user-observable outcome (e.g., 'Bandwidth metering is available without requiring changes to existing tenant or cluster workflows') and move deployment details to assumptions. Reframe the deduplication and retention acceptance criteria as user-observable accuracy and availability requirements (e.g., 'Bandwidth usage totals are accurate — querying the same period twice returns consistent results' and 'Historical bandwidth data is available for at least 13 months'). Replace the 'deployment is independent of provisioning workflows' criterion with a scenario a PM could actually run. Critical (0)None. Important (2)
Suggestions (2)
Review costModel: claude-opus-4-6 |
…tcomes Address AI review feedback: rewrite CAP-3 from deployment constraint to user-observable outcome, replace internal acceptance criteria (deduplication, retention windows, deployment independence) with PM-verifiable criteria, and move deployment detail to Assumptions. Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Moti Asayag <masayag@redhat.com> Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Moti Asayag <masayag@redhat.com>
AI EP Review: EP-160Score: 10/10 | Verdict: PASS
Verdict: Strong PRD with clear user-facing capabilities, concrete justification, focused scope, and testable acceptance criteria across all four OSAC personas. Feedback: Minor polish opportunities: user stories use 'view' while UI is explicitly out of scope — consider clarifying the access channel (e.g., 'query via API' or 'access through the billing system's usage views') to avoid ambiguity about what this PRD delivers vs. what the billing system provides. The Cloud Infrastructure Admin story ('integrating the networking vendor's traffic data source') is mildly implementation-flavored — a cleaner phrasing might be 'configure bandwidth metering for the platform.' Consider adding metering granularity expectations (sampling interval or counter resolution) to the acceptance criteria. Critical (0)None. Important (1)
Suggestions (3)
Review costModel: claude-opus-4-6 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@enhancements/OSAC-3149-metering-bandwidth/prd.md`:
- Around line 81-82: Add an explicit acceptance criterion to the bandwidth usage
requirements stating that tenant-admin and tenant-user consumers can access only
their own tenant’s data and cannot view another tenant’s bandwidth usage, while
preserving provider-wide visibility for authorized provider consumers.
- Around line 81-85: Update the bandwidth metering acceptance criteria near the
existing accuracy and historical-data items to explicitly require vendor-counter
accuracy, no measurement gaps during upgrades, and no double-counting of
duplicate events. Replace the fixed 13-month retention wording with configurable
retention whose minimum supported duration is 13 months, while preserving the
existing repeatability and provisioning-workflow criteria.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dcd1c0cb-25ea-40bc-8040-d6ab703ed1fd
📒 Files selected for processing (1)
enhancements/OSAC-3149-metering-bandwidth/prd.md
| - [ ] Bandwidth usage is recorded per tenant as GiB transferred, broken down by direction (ingress/egress) | ||
| - [ ] Bandwidth usage can be broken down by tenant, direction, and time period; project-level breakdown is available when the vendor data source supports project attribution |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Specify tenant isolation as an acceptance outcome.
The user stories distinguish provider-wide visibility from tenant-admin and tenant-user visibility, but these criteria do not require tenant-scoped consumers to be prevented from viewing another tenant’s bandwidth data. Add an explicit authorization/isolation outcome.
🤖 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 `@enhancements/OSAC-3149-metering-bandwidth/prd.md` around lines 81 - 82, Add
an explicit acceptance criterion to the bandwidth usage requirements stating
that tenant-admin and tenant-user consumers can access only their own tenant’s
data and cannot view another tenant’s bandwidth usage, while preserving
provider-wide visibility for authorized provider consumers.
| - [ ] Bandwidth usage is recorded per tenant as GiB transferred, broken down by direction (ingress/egress) | ||
| - [ ] Bandwidth usage can be broken down by tenant, direction, and time period; project-level breakdown is available when the vendor data source supports project attribution | ||
| - [ ] Enabling bandwidth metering does not disrupt existing tenant or cluster provisioning workflows | ||
| - [ ] Bandwidth usage totals are accurate — querying the same period twice returns consistent results | ||
| - [ ] Historical bandwidth data is available for at least 13 months |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Carry the Part 1 data-integrity guarantees into acceptance criteria.
“Querying the same period twice returns consistent results” proves repeatability, not accuracy, and does not verify CAP-15/CAP-16. The criteria also omit that retention must be configurable.
Add outcome-focused criteria covering vendor-counter accuracy, no measurement gaps during upgrades, no double-counting from duplicate events, and configurable retention of at least 13 months.
Proposed acceptance-criteria update
- [ ] Bandwidth usage totals are accurate — querying the same period twice returns consistent results
- [ ] Historical bandwidth data is available for at least 13 months
+ [ ] Bandwidth usage totals accurately reflect the vendor-provided traffic counters, and repeated queries for the same period return consistent results
+ [ ] Upgrades do not lose collected bandwidth data or create gaps for ongoing workloads
+ [ ] Duplicate traffic events do not double-count bandwidth usage
+ [ ] Historical bandwidth data is retained for at least 13 months, with a configurable retention period📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [ ] Bandwidth usage is recorded per tenant as GiB transferred, broken down by direction (ingress/egress) | |
| - [ ] Bandwidth usage can be broken down by tenant, direction, and time period; project-level breakdown is available when the vendor data source supports project attribution | |
| - [ ] Enabling bandwidth metering does not disrupt existing tenant or cluster provisioning workflows | |
| - [ ] Bandwidth usage totals are accurate — querying the same period twice returns consistent results | |
| - [ ] Historical bandwidth data is available for at least 13 months | |
| - [ ] Bandwidth usage is recorded per tenant as GiB transferred, broken down by direction (ingress/egress) | |
| - [ ] Bandwidth usage can be broken down by tenant, direction, and time period; project-level breakdown is available when the vendor data source supports project attribution | |
| - [ ] Enabling bandwidth metering does not disrupt existing tenant or cluster provisioning workflows | |
| - [ ] Bandwidth usage totals accurately reflect the vendor-provided traffic counters, and repeated queries for the same period return consistent results | |
| - [ ] Upgrades do not lose collected bandwidth data or create gaps for ongoing workloads | |
| - [ ] Duplicate traffic events do not double-count bandwidth usage | |
| - [ ] Historical bandwidth data is retained for at least 13 months, with a configurable retention period |
🤖 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 `@enhancements/OSAC-3149-metering-bandwidth/prd.md` around lines 81 - 85,
Update the bandwidth metering acceptance criteria near the existing accuracy and
historical-data items to explicitly require vendor-counter accuracy, no
measurement gaps during upgrades, and no double-counting of duplicate events.
Replace the fixed 13-month retention wording with configurable retention whose
minimum supported duration is 13 months, while preserving the existing
repeatability and provisioning-workflow criteria.
Source: Learnings
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: avishayt, masayag The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Summary
Jira
Related PRDs
Assisted-by: Claude Code noreply@anthropic.com
Summary by CodeRabbit