ci: ingest via telemetrygen (gRPC) + assert body reconstruction in the deploy smoke test - #319
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe deploy-test workflow gains a Smoke Test Overhaul
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Strengthens the existing kind-based deploy smoke test workflow to validate end-to-end log body reconstruction (not just presence of queried rows) by asserting the queried body.line matches the ingested message and body.reconstruction reports faithful.
Changes:
- Introduces
LOG_BODYas a shared input for ingestion and assertion to avoid drift. - Extends the query assertion to require
body.line == LOG_BODYandreconstruction == "faithful". - Adds a path-scoped
pull_requesttrigger intended as temporary for PR self-validation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two related strengthenings of the deploy smoke test: - Replace the hand-built OTLP/JSON curl ingest with telemetrygen (the OpenTelemetry synthetic-telemetry generator) run as an in-cluster Job, sending one OTLP log over gRPC (:4317) to the receiver. A real OTLP client on the wire, and it exercises the gRPC ingest path the curl test never did. service.name (= tenant) and the log body are set via flags; image pinned by digest. - Assert the queried row's body reconstructs to the ingested line with reconstruction "faithful" (CLAUDE.md §3.3), now that #302 is fixed — not just rows >= 1. A temporary path-scoped pull_request trigger validates this on its own PR (workflow_dispatch can't target a non-default branch); removed before merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
c210f2d to
8c90414
Compare
jensholdgaard
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/deploy-test.yml (1)
18-24: 📐 Maintainability & Code Quality | 🔵 TrivialRemove the temporary
pull_requesttrigger before merge.The trigger is self-described as temporary, and the PR objectives state it was to be dropped before merge, but it is still present here. Leaving it in widens the trigger surface beyond the intended on-demand + nightly final state.
Want me to open a tracking issue or prepare the removal diff?
🤖 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 @.github/workflows/deploy-test.yml around lines 18 - 24, Remove the temporary pull_request trigger from the deploy-test workflow so the final trigger set matches the intended on-demand + nightly-only behavior. Update the workflow configuration in the deploy-test YAML by deleting the path-scoped pull_request section and keep the remaining dispatch/scheduled triggers intact. Use the existing workflow trigger block near the temporary comment to locate the change.
🤖 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.
Nitpick comments:
In @.github/workflows/deploy-test.yml:
- Around line 18-24: Remove the temporary pull_request trigger from the
deploy-test workflow so the final trigger set matches the intended on-demand +
nightly-only behavior. Update the workflow configuration in the deploy-test YAML
by deleting the path-scoped pull_request section and keep the remaining
dispatch/scheduled triggers intact. Use the existing workflow trigger block near
the temporary comment to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 82921afe-81a1-4861-861a-18cb344918c0
📒 Files selected for processing (1)
.github/workflows/deploy-test.yml
Validated end-to-end on this PR: telemetrygen sends one OTLP log over gRPC, the querier returns it with a faithful body reconstruction (rows=1, reconstruction=faithful). Revert to on-demand + nightly only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The loop intends to retry until the row is back AND its body reconstructs faithfully, but the break checked only rows + line — so a transient retained_verbatim render whose line happened to match would break early and fail the post-loop check. Add reconstruction == faithful to the break so it retries until all three hold. Strict superset of the prior condition; the validated happy path (faithful on attempt 1) is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Strengthen the deploy smoke test (#318) to assert body reconstruction end-to-end, now that #302 (closed via #312) is done.
Previously the smoke test asserted only
rows >= 1with a comment deferring body text "to #302". The receiver now persists the miner's template audit events, so the querier reconstructs clean-row bodies bit-identically (CLAUDE.md §3.3). This makes the k8s deploy path verify that: after the row is queried back, assertbody.lineequals the ingested line andreconstruction == "faithful".Details
LOG_BODY) reused by both ingest and assertion, so they can't drift.Validation
Validated on this PR via the temporary path-scoped
pull_requesttrigger (workflow_dispatch can't target a non-default branch); dropped before merge — final state is on-demand + nightly only.🤖 Generated with Claude Code
Summary by CodeRabbit