Skip to content

feat: add lookup methods metrics#391

Open
yosiharan wants to merge 4 commits intomainfrom
fga-cache-metrics
Open

feat: add lookup methods metrics#391
yosiharan wants to merge 4 commits intomainfrom
fga-cache-metrics

Conversation

@yosiharan
Copy link
Contributor

@yosiharan yosiharan commented Feb 26, 2026

Related Issues

Resolves: https://github.com/descope/etc/issues/14210

Related PRs

Depends on: https://github.com/descope/managementservice/pull/3638

Caution

Do not auto-merge before backend is deployed

Description

Adding thread safe metrics reporting to authzcache, starting with lookup methods.
Default reporting is once per minute, reporting interval and complete reporting opting out can be configured via env variables.
Only aggregated cache hit/miss data is reported.

Env Variables

  • AUTHZCACHE_METRICS_REPORT_ENABLED - Whether to periodically report cache performance metrics (hit/miss rates, latency) to Descope (TRUE/FALSE, default: TRUE)
  • AUTHZCACHE_METRICS_REPORT_INTERVAL_IN_SECONDS - How often to report metrics, in seconds (default: 60, minimum: 10)

Must

  • Tests
  • Documentation (if applicable)

Copilot AI review requested due to automatic review settings February 26, 2026 07:27
@shuni-bot-dev
Copy link

shuni-bot-dev bot commented Feb 26, 2026

✅ Code review completed successfully

#391

Copy link

@shuni-bot-dev shuni-bot-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐕 Shuni's Review

Adds thread-safe per-project, per-API metrics collection for WhoCanAccess and WhatCanTargetAccess lookup methods, with a background reporter that POSTs aggregated data to the management service.

Sniffed out 1 issue:

  • 1 🟡 MEDIUM: HTTP request missing context propagation

Good bones overall — clean separation between collector and reporter, proper mutex usage, and solid test coverage. Woof!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds thread-safe metrics reporting for authzcache lookup methods (WhoCanAccess and WhatCanTargetAccess). The implementation tracks cache hit/miss ratios, candidate filtering statistics, and operation durations per project and API. A background reporter periodically aggregates and sends these metrics to a management service endpoint.

Changes:

  • Introduced metrics collector and reporter infrastructure for aggregating and posting cache performance data
  • Integrated metrics recording into WhoCanAccess and WhatCanTargetAccess methods
  • Added configuration options to control metrics reporting behavior (enabled/disabled, interval)

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
internal/services/remote/remote.go Exported BaseURL() and ManagementKey() functions to provide credentials for metrics reporter
internal/services/metrics/collector.go Thread-safe metrics collector using sync.Map and per-project mutexes to aggregate call data
internal/services/metrics/collector_test.go Unit tests for collector including concurrent access scenarios
internal/services/metrics/reporter.go Background reporter that periodically snapshots metrics and POSTs to management service
internal/services/metrics/reporter_test.go Tests for reporter covering happy path, disabled state, HTTP errors, and payload computation
internal/services/authz.go Integrated metrics recording into lookup methods with timing and cache hit/miss tracking
internal/services/authz_test.go Tests verifying metrics are correctly recorded for cache hits and misses
internal/config/config.go Added config options for metrics reporting interval and enable/disable flag
cmd/authzcache/main.go Wired up collector and reporter at application startup

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yosiharan yosiharan requested a review from orius123 February 26, 2026 08:50
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.

2 participants