feat: governance access query implementation [OM-360]#4409
feat: governance access query implementation [OM-360]#4409gergely-kurucz-konghq merged 16 commits into
Conversation
📝 WalkthroughWalkthroughImplements a governance query endpoint: domain models and validation, a QueryAccess service with stable cursor pagination and entitlement-based access evaluation, v3 HTTP handlers and response mapping, server/DI wiring, tests, and e2e benchmarks. ChangesGovernance Query Feature
Sequence Diagram (high-level) sequenceDiagram
participant Client
participant APIHandler
participant GovernanceService
participant CustomerService
participant EntitlementService
participant FeatureService
Client->>APIHandler: POST /governance/query (body, paging)
APIHandler->>GovernanceService: QueryAccess(input)
GovernanceService->>CustomerService: resolve customer keys
CustomerService-->>GovernanceService: customers + not-found errors
GovernanceService->>EntitlementService: fetch entitlements per customer
EntitlementService-->>GovernanceService: entitlement values
GovernanceService->>FeatureService: list/lookup features (if needed)
FeatureService-->>GovernanceService: feature metadata
GovernanceService-->>APIHandler: QueryResult (customers, errors, cursors)
APIHandler-->>Client: 200 OK + mapped JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
9313347 to
91d87c6
Compare
91d87c6 to
7639ac7
Compare
8519e29 to
26bf336
Compare
72ea64b to
a7ee66e
Compare
…eatureKeys is empty
…ervice Move the governance access-query orchestration out of the v3 HTTP handler into a new openmeter/governance service that composes the customer, entitlement, and feature services (no persistence of its own) — mirroring the customerbalance facade/service pattern. The service returns domain types (CustomerAccess, FeatureAccess, QueryError, paged QueryResult); the handler now only decodes the request, validates and decodes cursors, calls the service, and maps the domain result to api.* types. Service tests (Postgres-backed) move to the new package alongside the entitlement→access mapping unit tests. Wired idiomatically via app/common (NewGovernanceService + wire set) threaded through router.Config → cmd/server → openmeter/server → api/v3/server Config.
Follow the standard service-package layout: domain types in governance.go, the Service interface in service.go, and the implementation (Config, New, QueryAccess orchestration, entitlement→access mapping) under service/. Tests move alongside the implementation in service/. Wiring updated to call governanceservice.New; provider signature is unchanged so no wire regen.
Inject a tracer into the governance service and wrap the two I/O-bound phases of QueryAccess in child spans: - governance.QueryAccess (root): namespace, key/feature counts, page size, pagination direction - governance.resolveCustomers: requested vs resolved vs not_found - governance.resolveAccess: customer count, feature filter, all-org-features fan-out flag Errors on every early return are recorded on the active span. No per-customer spans (bounded ≤100 customers would inflate span volume). Tracer is threaded through Config/Wire; sort+paginate stays inline.
The namespace-wide feature list (all-org path, no feature filter) was fetched inside buildFeatureAccess, which runs per customer — re-fetching namespace-scoped data customer_count times. Hoist it into resolveAccess so it is fetched once per page. Add observability for future perf work: - span governance.listOrgFeatures (limit, feature_count) - resolveAccess attrs: org_feature_count, absent_feature_lookups, feature_access_total
Add BenchmarkGovernanceQuery (e2e/governance_bench_test.go) hitting a live stack across a customers x features grid, with make targets (bench-governance, bench-governance-matrix) and an e2e README runbook. Widen initClient/v3Client to testing.TB so benchmarks reuse the harness. Instrument the governance service with spans (QueryAccess, resolveCustomers, resolveAccess, listOrgFeatures) + attributes, and fetch org features once per page instead of per customer.
Governance Config now requires a Tracer; supply a noop one in the server route test, matching the service test harness.
Emit unsampled metrics for the governance query endpoint:
requests{namespace,direction,all_org_features},
customers_not_found{namespace}, and feature_access / customer_keys
histograms{namespace}. namespace is a per-tenant label (consistent with
ingest/sink/balanceworker); per-request counts are histogram values to
bound cardinality. Meter injected via Config like the tracer.
283192e to
fbe9efb
Compare
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 `@e2e/README.md`:
- Around line 54-57: The fenced code block containing the TraceQL query is
missing a language identifier; update the opening fence to include a language
(e.g., change the opening "```" to "```traceql") so the block is recognized by
the linter and syntax highlighting; locate the block with the TraceQL content
(the lines containing { name = "governance.QueryAccess" &&
span.customer_key_count = 100 && span.feature_key_count = 100 } and the
subsequent | quantile_over_time(...) line) and add the language identifier
immediately after the opening backticks.
In `@openmeter/server/router/router.go`:
- Line 121: Config.Validate() is missing a nil-check for the GovernanceService
field; update the Config.Validate method to verify that GovernanceService (type
governance.Service) is non-nil like the other services and return a descriptive
error if it is nil so startup fails fast (place this check with the other
service validations, e.g., after the existing service checks and before handler
construction that uses GovernanceService).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65ff4a57-de64-40a3-9039-a5f11a280dc5
📒 Files selected for processing (23)
api/v3/handlers/governance/handler.goapi/v3/handlers/governance/mapping.goapi/v3/handlers/governance/query.goapi/v3/server/routes.goapi/v3/server/server.goapp/common/governance.gocmd/server/main.gocmd/server/wire.gocmd/server/wire_gen.goe2e/Makefilee2e/README.mde2e/governance_bench_test.goe2e/setup_test.goe2e/v3helpers_test.goopenmeter/governance/governance.goopenmeter/governance/service.goopenmeter/governance/service/mapping.goopenmeter/governance/service/mapping_test.goopenmeter/governance/service/service.goopenmeter/governance/service/service_test.goopenmeter/server/router/router.goopenmeter/server/server.goopenmeter/server/server_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openmeter/governance/governance.go (1)
101-101: ⚡ Quick winConsider adding a docstring to the Validate method.
The validation encodes specific business rules (mutual exclusivity of cursors, positive page size, required fields) that aren't immediately obvious from the method signature alone. A brief docstring would help maintainers understand the contract. As per coding guidelines, domain helpers that compress important semantics benefit from documentation.
📝 Example docstring
+// Validate checks that the input satisfies all constraints: non-empty namespace, +// at least one customer key, positive page size, and mutual exclusivity of After/Before cursors. +// Returns a validation error aggregating all violations, or nil if valid. func (i QueryAccessInput) Validate() error {🤖 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 `@openmeter/governance/governance.go` at line 101, Add a concise docstring above the QueryAccessInput.Validate method that summarizes the validation contract: list the business rules enforced (mutual exclusivity of startCursor and endCursor, pageSize must be > 0, required fields that must be present, and any other invariants the method checks), and mention expected error behavior/return values on violation; reference the method name QueryAccessInput.Validate so maintainers can quickly understand the semantics without reading the implementation.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@openmeter/governance/governance.go`:
- Line 101: Add a concise docstring above the QueryAccessInput.Validate method
that summarizes the validation contract: list the business rules enforced
(mutual exclusivity of startCursor and endCursor, pageSize must be > 0, required
fields that must be present, and any other invariants the method checks), and
mention expected error behavior/return values on violation; reference the method
name QueryAccessInput.Validate so maintainers can quickly understand the
semantics without reading the implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3e39b1e3-0ceb-42a5-b2ba-1a1a818ee8e6
📒 Files selected for processing (8)
api/v3/handlers/governance/mapping.goapi/v3/server/server.goopenmeter/governance/governance.goopenmeter/governance/service/mapping.goopenmeter/governance/service/mapping_test.goopenmeter/governance/service/service.goopenmeter/governance/service/service_test.goopenmeter/server/router/router.go
🚧 Files skipped from review as they are similar to previous changes (7)
- openmeter/server/router/router.go
- openmeter/governance/service/mapping_test.go
- openmeter/governance/service/mapping.go
- api/v3/server/server.go
- api/v3/handlers/governance/mapping.go
- openmeter/governance/service/service_test.go
- openmeter/governance/service/service.go
What
Adds the governance access query endpoint —
POST /api/v3/openmeter/governance/query— which evaluates feature access for a batch of customers in one call, with cursor pagination, OpenTelemetry tracing + metrics, and an e2e latency benchmark.Why
Access-control / governance clients need to ask "which of these customers can use which features?" in a single request, rather than fanning out per-customer entitlement lookups themselves. Partial failures (unresolved keys) must be reported without failing the whole query.
How
openmeter/governance(root types +service/subpackage, per the/serviceskill): resolves customer keys → customers (dedup, partialerrors[]for misses), evaluates per-feature access, maps entitlements → access results. Wired idiomatically viaapp/common+ Wire.api/v3/handlers/governance: decode/validate → service → map domain →api.*.(CreatedAt, ID).feature.keys, or all org features when omitted (fetched once per page).QueryAccess,resolveCustomers,resolveAccess,listOrgFeatures) viatracex; unsampled metrics (requests,customers_not_found,feature_access,customer_keys) tagged withnamespace.e2e/governance_bench_test.go+make bench-governance[-matrix]measure latency across a customers × features grid (seee2e/README.md).Summary by CodeRabbit
New Features
Behavior
Documentation
Tests