OLS-3328: Add templog spec for finalizer and OTLP log emission#252
OLS-3328: Add templog spec for finalizer and OTLP log emission#252xrajesh wants to merge 1 commit into
Conversation
Adds agentic-operator child spec for temporary audit log storage: OTLP log emission, templog-cleanup finalizer, and CRD spec.templog. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@xrajesh: This pull request references OLS-3328 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature request to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new ChangesTemplog Feature Specification
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.ai/spec/what/audit-logging.md (1)
89-91: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRenumber the new JSON-format rule.
This restarts the ordered list at
20.even though rules20-26already exist above. Renumber it to27.(or convert the subsection to bullets) so the spec does not contain duplicate rule numbers.🛠 Suggested fix
-20. All audit events MUST be single JSON lines to stdout with at minimum: `timestamp`, `level`, `event`, `trace_id`. +27. All audit events MUST be single JSON lines to stdout with at minimum: `timestamp`, `level`, `event`, `trace_id`.🤖 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 @.ai/spec/what/audit-logging.md around lines 89 - 91, The ordered list under the Structured JSON Format subsection reuses the number 20 even though the prior audit rules already occupy 20 through 26. Update the numbering for this new rule in the spec so it follows the existing sequence (for example, make it 27) or convert this subsection to bullets; keep the rest of the audit-logging guidance unchanged.
🤖 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 @.ai/spec/what/crd-api.md:
- Line 98: The `spec.templog` item is still phrased as planned work even though
`AgenticOLSConfig` already includes it, so update this bullet to remove the
future-looking mismatch. Either delete the `PLANNED`-style entry from the CRD
notes or reword it as a completed change, keeping the wording consistent with
the active `AgenticOLSConfig` surface and the `spec.templog` symbol.
---
Outside diff comments:
In @.ai/spec/what/audit-logging.md:
- Around line 89-91: The ordered list under the Structured JSON Format
subsection reuses the number 20 even though the prior audit rules already occupy
20 through 26. Update the numbering for this new rule in the spec so it follows
the existing sequence (for example, make it 27) or convert this subsection to
bullets; keep the rest of the audit-logging guidance unchanged.
🪄 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: Enterprise
Run ID: 4307abc7-2bde-4017-9364-7f6afa9d05cb
📒 Files selected for processing (3)
.ai/spec/what/audit-logging.md.ai/spec/what/crd-api.md.ai/spec/what/templog.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
|
|
||
| - [PLANNED: OLS-2940] Autonomous workflow CRD migrations may rename or reshape fields; specs MUST be updated when `v1alpha1` changes. | ||
| - [PLANNED: OLS-2894] Explicit **Agent** fields for per-step system prompts if moved from template/runtime-only assembly (today prompts are composed outside `Agent` CR — see `sandbox-execution.md`). | ||
| - [OLS-3328] Add `spec.templog` to `AgenticOLSConfig` CRD for temporary audit log storage. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the PLANNED mismatch.
This bullet now reads like future work even though the active AgenticOLSConfig surface above already includes spec.templog. Either drop the planned-change entry or reword it as a completed note.
🛠 Suggested fix
- [OLS-3328] Add `spec.templog` to `AgenticOLSConfig` CRD for temporary audit log storage.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [OLS-3328] Add `spec.templog` to `AgenticOLSConfig` CRD for temporary audit log storage. |
🤖 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 @.ai/spec/what/crd-api.md at line 98, The `spec.templog` item is still
phrased as planned work even though `AgenticOLSConfig` already includes it, so
update this bullet to remove the future-looking mismatch. Either delete the
`PLANNED`-style entry from the CRD notes or reword it as a completed change,
keeping the wording consistent with the active `AgenticOLSConfig` surface and
the `spec.templog` symbol.
Summary
what/templog.mdfor agentic-operator's role: OTLP log emission,agentic.openshift.io/templog-cleanupfinalizer, Postgres cleanup on Proposal deletionwhat/audit-logging.mdwith OTLP log emission and finalizer ruleswhat/crd-api.mdwithspec.templogonAgenticOLSConfigContext
Part of OLS-3328 (temporary audit log storage). The agentic-operator emits OTLP log records to the Collector and cleans up Postgres rows when Proposals are deleted.
Parent spec PR: openshift/ols#11
Test plan
🤖 Generated with Claude Code