Skip to content

OLS-3328: Add templog spec for finalizer and OTLP log emission#252

Open
xrajesh wants to merge 1 commit into
openshift:mainfrom
xrajesh:xav/ols-3328-templog-spec
Open

OLS-3328: Add templog spec for finalizer and OTLP log emission#252
xrajesh wants to merge 1 commit into
openshift:mainfrom
xrajesh:xav/ols-3328-templog-spec

Conversation

@xrajesh

@xrajesh xrajesh commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds child spec what/templog.md for agentic-operator's role: OTLP log emission, agentic.openshift.io/templog-cleanup finalizer, Postgres cleanup on Proposal deletion
  • Updates what/audit-logging.md with OTLP log emission and finalizer rules
  • Updates what/crd-api.md with spec.templog on AgenticOLSConfig

Context

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

  • Spec-only change — no code, no tests
  • Review spec for completeness and accuracy

🤖 Generated with Claude Code

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>
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 29, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 29, 2026

Copy link
Copy Markdown

@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.

Details

In response to this:

Summary

  • Adds child spec what/templog.md for agentic-operator's role: OTLP log emission, agentic.openshift.io/templog-cleanup finalizer, Postgres cleanup on Proposal deletion
  • Updates what/audit-logging.md with OTLP log emission and finalizer rules
  • Updates what/crd-api.md with spec.templog on AgenticOLSConfig

Context

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

  • Spec-only change — no code, no tests
  • Review spec for completeness and accuracy

🤖 Generated with Claude Code

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.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for temporary audit log storage (“Templog”) with audit events emitted to both standard output and OTLP logs when enabled.
    • Introduced cleanup handling for templog-enabled proposals, including automatic removal of stored audit logs during deletion.
    • Added a new configuration option to enable or disable templog behavior.
  • Bug Fixes

    • When OTLP logging is not configured, audit logging now continues normally without extra errors or warnings.
    • Proposal deletion now waits for cleanup to complete before final removal.

Walkthrough

Adds a new templog.md spec and updates audit-logging.md and crd-api.md to define the templog feature: dual OTLP/stdout audit log emission, a spec.templog boolean field on AgenticOLSConfig, and a Proposal finalizer that performs direct Postgres cleanup on deletion.

Changes

Templog Feature Specification

Layer / File(s) Summary
spec.templog CRD field
.ai/spec/what/crd-api.md
Adds spec.templog boolean to AgenticOLSConfig docs and planned-changes list, noting it triggers OTel Collector deployment for Postgres-backed audit storage.
OTLP audit emission spec
.ai/spec/what/templog.md, .ai/spec/what/audit-logging.md
New templog.md defines OTLP log record structure (trace_id, event attribute, JSON body), conditions for dual OTLP+stdout vs stdout-only emission, and graceful no-op when the endpoint env var is absent. audit-logging.md mirrors these requirements.
Proposal finalizer and Postgres cleanup
.ai/spec/what/templog.md, .ai/spec/what/audit-logging.md
Specifies agentic.openshift.io/templog-cleanup finalizer lifecycle: added on creation when templog is enabled; on deletion, operator connects directly to Postgres (from shared credentials secret, TLS) and issues DELETE from templogs.logs by trace_id; blocks deletion and requeues with backoff on failure. Documents edge cases: toggling templog mid-lifecycle, Postgres unavailability, zero-row deletes, and pre-existing Proposals without the finalizer.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly summarizes the main change: adding templog spec for finalizer behavior and OTLP log emission.
Description check ✅ Passed The description matches the spec-only changes, context, and scope of the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot requested review from joshuawilson and onmete June 29, 2026 15:08
@openshift-ci

openshift-ci Bot commented Jun 29, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign xrajesh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai coderabbitai Bot 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.

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 win

Renumber the new JSON-format rule.

This restarts the ordered list at 20. even though rules 20-26 already exist above. Renumber it to 27. (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

📥 Commits

Reviewing files that changed from the base of the PR and between 4949b66 and 949d5b6.

📒 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)

Comment thread .ai/spec/what/crd-api.md

- [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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 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.

Suggested change
- [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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants