Skip to content

[DNM] metering: add changefeed traffic reporting foundations - #6205

Draft
3AceShowHand wants to merge 4 commits into
pingcap:masterfrom
3AceShowHand:cdc-traffic-metering
Draft

3AceShowHand wants to merge 4 commits into
pingcap:masterfrom
3AceShowHand:cdc-traffic-metering

Conversation

@3AceShowHand

@3AceShowHand 3AceShowHand commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: ref #6204

Add the foundation for reporting changefeed traffic through Metering SDK. This draft does not yet connect sink accounting, durable metering state, or periodic submission.

What is changed and how it works?

  • Add TrafficReporter.Report(ctx, reportedAt, []TrafficRecord) with typed changefeed results, per-call batch identity, bounded retries through the existing retry utility, and explicit upload errors. The reporter does not collect data or assume cumulative accounting semantics.
  • Initialize and close the reporter within each coordinator term. Reporting is disabled by default; configured initialization failures propagate to the server.
  • Delegate storage-provider configuration and initialization to Metering SDK. Omit the entire metering destination from configuration logs and /config; retain shared-pool path validation.
  • Add canonical single-value byte measurement in pkg/metering/traffic and focused tests. Selection, event accumulation, and delivery confirmation belong to callers.
  • Exercise Report through a local HTTP storage endpoint for retry recovery/exhaustion, batch identity and payload stability, existing objects, and in-flight cancellation. Verify coordinator shutdown after its event loop has started.
  • Remove the repository-root server.toml file.

Remaining work

Sink integration, reliable metering state, periodic submission, the consumption contract, and end-to-end recovery validation are still pending. Repeated Report calls create distinct batches; cross-call recovery and deduplication belong to the producer/consumer contract.

Check List

Tests

  • Unit test
  • go test -p 1 ./pkg/metering -run TestTrafficReporter -count=1: passed.
  • Coordinator TestMetering* tests passed in the targeted package run.
  • go test -race -p 1 ./pkg/metering ./coordinator -run 'Test(TrafficReporter|Metering)' -count=1: metering passed; coordinator could not build because the local disk ran out of space.
  • go test -p 1 ./pkg/metering/... ./pkg/config ./api ./coordinator -run 'Test(ValidateConfig|Measure|ServerMeteringConfig|RegisterRoutesConfig|Metering)' -count=1: passed after relocating the rules and simplifying configuration handling.
  • Changed Go files formatted with gci/gofumports; git diff --cached --check.

Full build and coordinator race coverage remain unverified after earlier disk-space failures. Cloud integration, fault injection, and performance tests have not run.

Questions

Will it cause performance regression or break compatibility?

Metering is disabled by default, and sink execution paths are not connected yet. Configured destinations add coordinator initialization and SDK dependencies. Storage validation follows SDK behavior; some invalid configurations may only fail on upload because the SDK does not provide a unified pre-validation API. No end-to-end performance claim is made.

Do you need to update user documentation, design documentation or monitoring documentation?

User-facing documentation and monitoring will need updates when the complete reporting flow is connected.

Release note

None

@ti-chi-bot ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Sep 9, 2026
@ti-chi-bot

ti-chi-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ti-chi-bot

ti-chi-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nongfushanquan for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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

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

@ti-chi-bot ti-chi-bot Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant