Skip to content

fix: disable metric formatters#542

Open
andrestejerina97 wants to merge 1 commit into
mainfrom
fix/disable-metric-formatters
Open

fix: disable metric formatters#542
andrestejerina97 wants to merge 1 commit into
mainfrom
fix/disable-metric-formatters

Conversation

@andrestejerina97
Copy link
Copy Markdown
Contributor

@andrestejerina97 andrestejerina97 commented May 12, 2026

ref: https://app.clickup.com/t/86b9pvafe

Summary by CodeRabbit

Release Notes

  • New Features

    • Audit logging can now be selectively disabled for specific entity types through configuration.
    • Audit logging is now disabled for SummitEventAttendanceMetric and SummitMetric entities.
  • Tests

    • Added comprehensive test coverage for per-entity audit logging configuration and behavior.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6adade88-e2ee-41e1-a786-211012333e74

📥 Commits

Reviewing files that changed from the base of the PR and between 34e47f9 and 645f3f3.

📒 Files selected for processing (3)
  • app/Audit/AuditLogFormatterFactory.php
  • config/audit_log.php
  • tests/OpenTelemetry/AuditLogFormatterFactoryTest.php

📝 Walkthrough

Walkthrough

This PR adds entity-level audit logging control to the summit API. The AuditLogFormatterFactory now checks a per-entity configuration flag and returns null when auditing is disabled, preventing audit logs from being generated for specific entity types. Two summit metric entities are configured to disable auditing, with comprehensive tests covering the new behavior.

Changes

Entity-Level Audit Disablement

Layer / File(s) Summary
Audit disablement mechanism in factory
app/Audit/AuditLogFormatterFactory.php
AuditLogFormatterFactory::make() now short-circuits early by checking if auditing is disabled for the subject via a new isAuditDisabledForSubject() helper that inspects the cached audit_log config and returns true only when the entity config explicitly sets enabled to false.
Entity-level audit configuration
config/audit_log.php
Two summit metric entity types are disabled in the audit_log config by setting their enabled flag to false: SummitEventAttendanceMetric and SummitMetric.
Test coverage for disablement behavior
tests/OpenTelemetry/AuditLogFormatterFactoryTest.php
New tests validate that isAuditDisabledForSubject correctly returns true/false based on entity config, that make() returns null for disabled entities, and support the tests with imports, a config injection helper, and test double classes (FakeAuditEntity and FakeAuditFormatter).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A audit log that bends the knee,
To config flags that set things free,
Some metrics rest, by design's decree,
While others log for all to see!
—The Summit Rabbit 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: disable metric formatters' directly aligns with the main change: disabling audit logging for metric entities (SummitEventAttendanceMetric and SummitMetric) by adding configuration and logic to skip formatter creation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/disable-metric-formatters

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 and usage tips.

@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-542/

This page is automatically updated on each push to this PR.

@andrestejerina97 andrestejerina97 marked this pull request as ready for review May 12, 2026 11:38
Copy link
Copy Markdown
Contributor

@caseylocker caseylocker left a comment

Choose a reason for hiding this comment

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

LGTM. Nothing blocking. Follows the ticket direction.

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