Skip to content

Add provider timing metrics - #73

Open
hosom wants to merge 3 commits into
github:mainfrom
hosom:hosom/provider-timing-metrics
Open

Add provider timing metrics#73
hosom wants to merge 3 commits into
github:mainfrom
hosom:hosom/provider-timing-metrics

Conversation

@hosom

@hosom hosom commented Aug 31, 2026

Copy link
Copy Markdown

Add datadog metrics to keep track of how long each deployment phase takes.

Ignore this slop ## Summary

Add structured timing metrics around the shared Entitlements execution boundaries so deployment time can be attributed to a phase, provider, and configured target.

The emitted JSON records measure:

  • extras, people, and filter initialization
  • each people data source read
  • backend prefetch, validation, and calculation
  • backend pre-apply and apply operations
  • auditor setup and commit

All durations use a monotonic clock. Successful and failed operations are recorded, and a metrics logging failure cannot replace an application or provider result.

Top-level calculation and execution records are marked as parent spans for wall-clock analysis. Individual records indicate whether they can overlap, preventing parallel prefetch timings from being incorrectly summed. Every record also contains a run identifier, which can be supplied through ENTITLEMENTS_RUN_ID to match the surrounding deployment.

Example:

METRIC {"metric":"entitlements.operation.duration_seconds","value":11.204,"phase":"apply","status":"success","run_id":"fc85bb97-48f4-4f88-bb90-63e52cb01e9f","span":"leaf","concurrent":false,"provider":"aad","target":"apps/azure_aad","count":1}

Raw action identifiers are intentionally excluded because actions can identify individual users and would create an unbounded field. Provider and target fields are sufficient to separate production and testing backends while aggregating deployment latency.

The record schema and aggregation semantics are documented in docs/metrics.md.

Why

Current deployment telemetry reports only total runtime. Calculation logs expose a few broad boundaries, but they cannot reliably distinguish provider reads and diff calculation from provider writes. This makes it impossible to identify whether a long deployment is dominated by AAD, GitHub, LDAP, Stafftools, or audit persistence.

Test plan

  • bundle exec rspec spec/unit
  • bundle exec rubocop -c .rubocop.yml lib/entitlements.rb spec/unit/entitlements_spec.rb

Emit structured, monotonic timing records around initialization, provider reads, calculations, writes, and audit operations so deployment latency can be attributed by phase and provider.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d92ba3d2-77c4-461f-8d6f-25c3228476d1
Copilot AI balanced review requested due to automatic review settings August 31, 2026 14:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Ruby 3.0 syntax incompatibilities and a secondary logging failure can prevent or alter application execution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 High severity · 1 Medium severity

New issues introduced by this change (2)
Severity Finding
High severity lib/​entitlements.rb — These omitted keyword values require Ruby 3.1+, but the gem still declares Ruby &gt;= 3.0.0
Medium severity lib/​entitlements.rb — If metric logging fails and STDERR is unavailable (for example, a closed log stream), warn raises…
What changed in this PR

Adds structured duration metrics around Entitlements provider and auditing operations.

Changes:

  • Introduces monotonic timing and JSON metric logging.
  • Instruments calculation, application, people-source, and auditing phases.
  • Adds unit coverage for metric success and failure behavior.
File Description
lib/​entitlements.rb Implements and applies timing instrumentation.
spec/​unit/​entitlements_spec.rb Tests timing metrics and updates provider doubles.
Suppressed comments (3)

lib/entitlements.rb:431

  • The provider: value-omission syntax requires Ruby 3.1+, while this gem supports Ruby 3.0 (entitlements-app.gemspec:17). On Ruby 3.0 the entire file is a syntax error, so use the explicit keyword value.
      timed_operation(phase: "calculate", provider:, target: group_name) { obj.calculate }

lib/entitlements.rb:477

  • The shorthand provider: argument was introduced in Ruby 3.1, but the gem's supported range includes Ruby 3.0 (entitlements-app.gemspec:17). This makes the file unparseable for those consumers; use provider: provider.
        timed_operation(phase: "apply", provider:, target: action.ou) do

lib/entitlements.rb:470

  • This omitted keyword value is not valid syntax on supported Ruby 3.0 (entitlements-app.gemspec:17), causing the application to fail while loading this file. Pass the local variable explicitly.
        timed_operation(phase: "preapply", provider:, target: group_name) { obj.preapply }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/entitlements.rb Outdated
Comment thread lib/entitlements.rb Outdated
hosom added 2 commits August 31, 2026 10:32
Add run correlation, parent and concurrent span semantics, operation counts, and metric documentation so provider service time cannot be confused with deployment wall time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d92ba3d2-77c4-461f-8d6f-25c3228476d1
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