Skip to content

feat(observability): instrument durable checkpoint operations - #78

Closed
seonghobae wants to merge 1 commit into
agent/persistent-result-checkpoint-storefrom
agent/checkpoint-opentelemetry-observability-v2
Closed

feat(observability): instrument durable checkpoint operations#78
seonghobae wants to merge 1 commit into
agent/persistent-result-checkpoint-storefrom
agent/checkpoint-opentelemetry-observability-v2

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Linearized replacement for #61

This Draft replays the exact reviewed checkpoint-observability feature delta onto the exact current predecessor agent/persistent-result-checkpoint-store at 6b2e19e3503ef3671aa2d2f974df7561ba1e1ca0 without force-pushing or choosing conflict sides.

RCA and replacement boundary

Predecessor #60 was safely replayed onto the current stack after old #61's earlier reconciliation. The old #61 branch therefore diverged from the new predecessor and GitHub reported direct reconciliation probe #77 non-mergeable. Before replay, the five files modified by the #61 feature and present in both former base efe2c81f8a9cd8301461fe8ae3ce9d5901652185 and current predecessor were verified byte-identical: AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md, and pg_llm_batch/__init__.py.

Replacement head 64eea7edb28e391634a5d13c83495d20c6388c81 is exactly one commit ahead and zero behind exact current predecessor 6b2e19e3503ef3671aa2d2f974df7561ba1e1ca0 and changes exactly the same 17 checkpoint-observability feature files as the former accepted #61 delta: storage-agnostic low-cardinality OpenTelemetry checkpoint spans/metrics, confidentiality and observer-failure isolation, explicit Error status without descriptions, public API export, deterministic tests, ADR/operator/doctoring documentation, and governing contracts.

Old #61 is closed unmerged as superseded. No predecessor-head check, review, or approval transfers.

Current staged evidence

  • Head: 64eea7edb28e391634a5d13c83495d20c6388c81.
  • Exact current base tip: 6b2e19e3503ef3671aa2d2f974df7561ba1e1ca0 (PR feat(checkpoint): persist resumable result progress #60 branch).
  • Ancestry: one commit ahead, zero behind; merge base equals the exact base tip.
  • CI 31285605089: completed / success on the exact replacement source head.
  • Release Acceptance 31285605090: completed / success on the exact replacement source head.
  • Formal reviews: none at the latest inspection.
  • Unresolved inline review threads: zero.
  • A fresh exact-head CodeRabbit review-only request has been issued after staged validation; automated review is evidence only and cannot substitute for qualifying independent approval.

Because this is still a stacked Draft, default-branch-only organization workflows and final protected-main gates are not inferred as passing.

Merge boundary

Required order remains .github#790 -> pg-llm-batch#53 -> #55 -> #56 -> #57 -> #58 -> #59 -> #60 -> this PR -> #79.

Keep Draft while the earlier stack remains unintegrated. After every prerequisite safely reaches protected main, reconcile or retarget this slice onto the actual integrated base and regenerate every required exact-head/exact-base CI, security, dependency, SBOM/provenance, packaging, live-integration, automated-review, branch-protection, repository-policy, and Release Acceptance gate. Merge only with zero unresolved valid findings and a qualifying independent non-author GitHub APPROVED review on the unchanged final head. Queued, pending, failed, cancelled, skipped-required, neutral-required, absent, stale-head/base, predecessor, status-only, author-only, or synthetic-only evidence is never acceptance.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 19 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 71bf2f12-f683-418b-9ba2-1261c6493413

📥 Commits

Reviewing files that changed from the base of the PR and between 6b2e19e and 64eea7e.

📒 Files selected for processing (17)
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • docs/adr/0008-checkpoint-opentelemetry-observability.md
  • docs/checkpoint-observability.md
  • docs/doctoring/checkpoint-opentelemetry-observability.md
  • pg_llm_batch/__init__.py
  • pg_llm_batch/checkpoint_telemetry.py
  • tests/test_checkpoint_telemetry.py
  • tests/test_checkpoint_telemetry_clock.py
  • tests/test_checkpoint_telemetry_concurrency.py
  • tests/test_checkpoint_telemetry_documentation.py
  • tests/test_checkpoint_telemetry_exception_boundary.py
  • tests/test_checkpoint_telemetry_public_api.py
  • tests/test_checkpoint_telemetry_span_status.py
  • tests/test_checkpoint_telemetry_storage_semantics.py

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

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Review exact replacement source head 64eea7edb28e391634a5d13c83495d20c6388c81 against exact current predecessor agent/persistent-result-checkpoint-store at 6b2e19e3503ef3671aa2d2f974df7561ba1e1ca0. Review only; do not mutate the branch. Validate the replayed checkpoint OpenTelemetry boundary: storage-agnostic low-cardinality attributes, no tenant/consumer/batch/endpoint/file/digest/cursor/DSN/provider payload/exception-message/dynamic-class leakage, explicit Error status without descriptions, observer/clock/optional-API failure isolation, exact application result/exception and transaction semantics, concurrency isolation, and public export compatibility. CI 31285605089 and Release Acceptance 31285605090 are successful staged exact-head evidence. Old #61 is closed superseded; do not reuse predecessor-head reviews or checks.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

source_commit='64eea7edb28e391634a5d13c83495d20c6388c81'
base_commit='6b2e19e3503ef3671aa2d2f974df7561ba1e1ca0'

echo '== Exact commits and ancestry =='
git show -s --format='source=%H%nparents=%P%nsubject=%s' "$source_commit"
git show -s --format='base=%H%nparents=%P%nsubject=%s' "$base_commit"
git merge-base --is-ancestor "$base_commit" "$source_commit" && echo 'base_is_ancestor=true'
git rev-list --left-right --count "${base_commit}...${source_commit}"

echo '== Changed files =='
git diff --name-status "$base_commit" "$source_commit"
echo '== Diff stat =='
git diff --stat "$base_commit" "$source_commit"

echo '== Source file outline =='
ast-grep outline pg_llm_batch --items all --type class,function 2>/dev/null || true

echo '== Changed Python symbols and observability references =='
git diff --unified=80 "$base_commit" "$source_commit" -- 'pg_llm_batch/**/*.py' ':!**/tests/**'

echo '== Test and documentation diff =='
git diff --unified=30 "$base_commit" "$source_commit" -- 'tests/**' 'AGENTS.md' 'ARCHITECTURE.md' 'CHANGELOG.md' 'CLAUDE.md' 'docs/**' '.github/**'

echo '== Focused repository references =='
rg -n -i -C 3 \
  'checkpoint|opentelemetry|trace\.get_tracer|start_as_current_span|start_span|set_attribute|set_status|record_exception|histogram|counter|monotonic|perf_counter' \
  pg_llm_batch tests 2>/dev/null || true

Length of output: 186010


🏁 Script executed:

#!/bin/bash
set -euo pipefail

source_commit='64eea7edb28e391634a5d13c83495d20c6388c81'
base_commit='6b2e19e3503ef3671aa2d2f974df7561ba1e1ca0'

echo '== Commit relationship =='
git show -s --format='source %H%nparents %P%nsubject %s' "$source_commit"
git merge-base --is-ancestor "$base_commit" "$source_commit" && echo 'base is an ancestor'
git rev-list --left-right --count "${base_commit}...${source_commit}"

echo '== Changed files =='
git diff --name-status "$base_commit" "$source_commit"

echo '== New checkpoint telemetry implementation =='
git show "${source_commit}:pg_llm_batch/checkpoint_telemetry.py" | cat -n

echo '== Package-root export =='
git show "${source_commit}:pg_llm_batch/__init__.py" | cat -n

echo '== Checkpoint store public method signatures =='
ast-grep outline pg_llm_batch/checkpoint_store.py --items all --type class,function --view expanded
rg -n -A18 -B3 '^(class |    def (load|save|load_in_transaction|save_in_transaction)\b)' \
  pg_llm_batch/checkpoint_store.py

echo '== Focused wrapper tests =='
for test_file in \
  tests/test_checkpoint_telemetry.py \
  tests/test_checkpoint_telemetry_clock.py \
  tests/test_checkpoint_telemetry_span_status.py \
  tests/test_checkpoint_telemetry_exception_boundary.py \
  tests/test_checkpoint_telemetry_concurrency.py \
  tests/test_checkpoint_telemetry_storage_semantics.py \
  tests/test_checkpoint_telemetry_public_api.py
do
  echo "---- ${test_file} ----"
  cat -n "$test_file"
done

echo '== Packaging dependency and type/lint configuration references =='
rg -n -C 3 'opentelemetry|python_requires|requires-python|mypy|ruff' \
  pyproject.toml setup.cfg setup.py requirements*.txt 2>/dev/null || true

Length of output: 80130


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Documented optional-API and failure-isolation contract =='
rg -n -i -C 4 \
  'optional|StatusCode|status Error|failure isolation|observer|best-effort|constructor|degrade' \
  docs/adr/0008-checkpoint-opentelemetry-observability.md \
  docs/checkpoint-observability.md \
  docs/doctoring/checkpoint-opentelemetry-observability.md \
  AGENTS.md ARCHITECTURE.md CHANGELOG.md CLAUDE.md

echo '== Status resolution call sites and existing regression coverage =='
rg -n -C 5 \
  '_resolve_error_status|StatusCode\.ERROR|missing_opentelemetry|optional API' \
  pg_llm_batch/checkpoint_telemetry.py \
  tests/test_checkpoint_telemetry_span_status.py \
  tests/test_checkpoint_telemetry_exception_boundary.py

echo '== Requested staged workflow results =='
gh run view 31285605089 --json databaseId,headSha,status,conclusion,event,name,url 2>&1 || true
gh run view 31285605090 --json databaseId,headSha,status,conclusion,event,name,url 2>&1 || true

Length of output: 27517


@seonghobae I reviewed source commit 64eea7edb28e391634a5d13c83495d20c6388c81 against direct predecessor 6b2e19e3503ef3671aa2d2f974df7561ba1e1ca0.

CI 31285605089 and Release Acceptance 31285605090 both succeeded for the exact source commit.

I found one issue:

  • Line 146, pg_llm_batch/checkpoint_telemetry.py: StatusCode.ERROR is read outside the guarded optional-API resolution block. If opentelemetry.trace imports but provides an incompatible or partial StatusCode without ERROR, construction raises AttributeError. This makes optional status support an application dependency. Keep the StatusCode.ERROR lookup inside the same _TELEMETRY_FAILURES boundary and return None when it fails. Add a regression test for an importable opentelemetry.trace module whose StatusCode lacks ERROR.

The remaining inspected behavior meets the requested boundary: fixed low-cardinality attributes, no protected checkpoint identifiers or exception payloads in package-owned telemetry, explicit Error status without a description, unchanged delegation and transaction ownership, operation-local attribute mappings, and package-root export compatibility.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

Superseded by #92. PR #60 advanced from 6b2e19e3503ef3671aa2d2f974df7561ba1e1ca0 to be9a2ff90aa4b4219462392c2a49ff5b3218a1a8, making this one-commit stack node stale. #92 is rebuilt directly on the exact current #60 head, preserves #60's newer DSN-boundary changelog entry, reuses the reviewed observability result blobs, and reproduces the same 17-file feature delta and 2059 additions / 3 deletions. No checks, reviews, or approvals from this stale head transfer.

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.

1 participant