Skip to content

Fix audit events logged as INFO+2 instead of AUDIT#5256

Merged
ChrisJBurns merged 6 commits into
stacklok:mainfrom
kimjune01:fix/audit-level-name
May 15, 2026
Merged

Fix audit events logged as INFO+2 instead of AUDIT#5256
ChrisJBurns merged 6 commits into
stacklok:mainfrom
kimjune01:fix/audit-level-name

Conversation

@kimjune01
Copy link
Copy Markdown
Contributor

@kimjune01 kimjune01 commented May 12, 2026 •
edited
Loading

Summary

  • Add ReplaceAttr to the audit logger's JSON handler so the custom audit level renders as "AUDIT" instead of "INFO+2"
  • Fixes compatibility with log aggregation systems (Loki, Elasticsearch) that expect standard level names for filtering

Fixes #4296

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

Added two tests: one verifying audit events render as "AUDIT", another verifying standard levels (INFO, WARN) are preserved. go test ./pkg/audit/... -count=1: 28 tests passed.

API Compatibility

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Does this introduce a user-facing change?

No. Audit log output changes from "INFO+2" to "AUDIT" in JSON logs, but this is a bug fix restoring intended behavior.

kimjune01 added 2 commits May 11, 2026 21:39
@kimjune01
Fix audit events logging at non-standard level INFO+2
cef5de6 
Audit events were logging as "INFO+2" which breaks level detection
in log aggregation systems (Loki, Elasticsearch, Splunk). This
caused audit events to appear as unknown level and prevented
level-based filtering in log pipelines.

Changed NewAuditLogger to add a ReplaceAttr function that renders
the custom audit level (slog.Level(2)) as "AUDIT" string in JSON
output, while preserving the numeric level for internal filtering.

Preserves the original design intent of placing audit between INFO
and WARN, while making events filterable via level="AUDIT" in LogQL
and other query languages.

Fixes stacklok#4296

Signed-off-by: June Kim <kim.june.01@gmail.com>
Signed-off-by: June Kim <kimjune01@gmail.com>
@kimjune01
Remove TRIAGE_GRAPH.md from tracked files
f658757 
Internal pipeline metadata should not be committed to the upstream repo.
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label May 12, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.41%. Comparing base (439977d) to head (22c48b7).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #5256      +/-   ##
==========================================
- Coverage   68.42%   68.41%   -0.01%     
==========================================
  Files         620      620              
  Lines       63354    63358       +4     
==========================================
  Hits        43349    43349              
- Misses      16768    16774       +6     
+ Partials     3237     3235       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

amirejaz
amirejaz reviewed May 14, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@amirejaz amirejaz left a comment

Choose a reason for hiding this comment

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

Nice fix, and a clear PR description — the problem ("INFO+2" leaking into log aggregators) is well-motivated. The implementation is small and correct. I have two suggestions below: one about making the level comparison more explicit, and one about the second test case re-implementing rather than exercising NewAuditLogger.

Comment thread pkg/audit/auditor.go Outdated
Comment thread pkg/audit/auditor_test.go Outdated
@kimjune01
Address review: explicit type assertion + use NewAuditLogger in test
9bd90a0 
Per @amirejaz review:

1. auditor.go: Replace implicit any-equality (a.Value.Any() == LevelAudit)
   with an explicit slog.Level type assertion. Defensive against future
   slog changes that might wrap level attrs differently.

2. auditor_test.go: 'preserves standard log levels' subtest no longer
   re-implements ReplaceAttr — it now uses NewAuditLogger directly so a
   regression in the production handler would be caught. Tests WARN
   (above LevelAudit threshold) since INFO is filtered out before
   ReplaceAttr runs.
@kimjune01
Copy link
Copy Markdown
Contributor Author

kimjune01 commented May 14, 2026

Thanks for the review @amirejaz — both points addressed in 9bd90a0.

  1. Explicit level comparison (auditor.go:64-68): replaced a.Value.Any() == LevelAudit with a slog.Level type assertion. Makes the contract explicit and defensive.

  2. Test exercises NewAuditLogger (auditor_test.go:1027-1041): the preserves standard log levels subtest now calls NewAuditLogger directly and asserts on WARN (above the LevelAudit threshold, so it survives the level filter). A regression in the production ReplaceAttr would now be caught. Comment in the test explains why INFO can't be tested through the production path.

go test ./pkg/audit/... passes locally.

@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels May 14, 2026
@amirejaz
Copy link
Copy Markdown
Contributor

amirejaz commented May 14, 2026

Linter is flagging two things in the fixup commit:

  1. auditor.go:59 — groups in the ReplaceAttr signature is unused. Rename it to _.
  2. auditor_test.go:1015 — logger.Log(nil, ...) passes a nil context. Replace nil with context.TODO().

Otherwise looks good — happy to approve once those are fixed.

ChrisJBurns and others added 2 commits May 14, 2026 15:59
@ChrisJBurns
Merge branch 'main' into fix/audit-level-name
134428b
@kimjune01
Address lint findings
081101c 
- Rename unused groups param to _ in ReplaceAttr
- Replace nil context with context.TODO() in test
@kimjune01
Copy link
Copy Markdown
Contributor Author

kimjune01 commented May 14, 2026

Lint findings addressed in 081101c.

  • groups → _ in auditor.go:59
  • nil → context.TODO() in auditor_test.go:1015

Tests pass.

@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels May 15, 2026
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels May 15, 2026
@ChrisJBurns ChrisJBurns merged commit 4893b65 into stacklok:main May 15, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amirejaz amirejaz amirejaz left review comments

@ChrisJBurns ChrisJBurns ChrisJBurns approved these changes

@jhrozek jhrozek Awaiting requested review from jhrozek jhrozek is a code owner

@JAORMX JAORMX Awaiting requested review from JAORMX JAORMX is a code owner

@yrobla yrobla Awaiting requested review from yrobla yrobla is a code owner

Assignees

No one assigned

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Audit events logged at non-standard level INFO+2, breaking level detection in log pipelines

3 participants

@kimjune01 @amirejaz @ChrisJBurns