Skip to content

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

Closed
seonghobae wants to merge 2 commits into
agent/persistent-result-checkpoint-storefrom
agent/checkpoint-opentelemetry-observability-v3
Closed

feat(observability): instrument durable checkpoint operations#92
seonghobae wants to merge 2 commits into
agent/persistent-result-checkpoint-storefrom
agent/checkpoint-opentelemetry-observability-v3

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Linearized replacement for #78

This Draft replays the checkpoint-observability feature onto the exact current predecessor #60 head be9a2ff90aa4b4219462392c2a49ff5b3218a1a8 after #60 advanced its durable-store DSN boundary.

RCA and feasibility

#78 remained one commit ahead of an obsolete predecessor, while live comparison showed the actual #60 head had advanced. The predecessor advancement touches only CHANGELOG.md, durable checkpoint-store implementation/tests/doctoring, and a DSN-boundary regression. The observability feature touches 17 files; the only overlap with that predecessor advancement is CHANGELOG.md.

The replay therefore uses the exact #78 result blobs for all observability files and preserves #60's later nonblank-DSN changelog entry while adding the same observability entry. No conflict-side selection, force-push, product-behavior rewrite, dependency change, migration change, version bump, or release action is used.

Exact replacement evidence

Historical #78 CI/review evidence is provenance only and does not transfer. Successful staged CI/release-acceptance is not final protected-main security/review/policy evidence.

Dependency and merge boundary

Required order is .github#790 -> pg-llm-batch#53 -> #57 -> #58 -> #59 -> #60 -> this replacement -> #94 -> #95 -> #96 -> #97.

Keep Draft while earlier prerequisites remain unintegrated. After prerequisite integration, reconcile/retarget only as integration correctness requires and regenerate every applicable 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 valid unresolved findings and a qualifying independent non-author GitHub APPROVED review on the unchanged final head where required. Queued, pending, cancelled, skipped-required, absent, neutral-required, stale-head/base, predecessor, status-only, synthetic-only, author-only, rate-limited, or failed evidence is not acceptance.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ab6438ec-3878-40e5-9c30-14756eca9247

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

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

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
⚠️ 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

@coderabbitai review

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact current replacement head 2235ec3eec2f73cb7ea3117c7b881d3bd1f0bdb7 against exact predecessor #60 head be9a2ff90aa4b4219462392c2a49ff5b3218a1a8. Validate the replayed checkpoint OpenTelemetry contracts, confidentiality/low-cardinality bounds, observer-failure isolation, and that no evidence or assumptions transfer from superseded #78.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='be9a2ff90aa4b4219462392c2a49ff5b3218a1a8'
head='2235ec3eec2f73cb7ea3117c7b881d3bd1f0bdb7'

echo '== checked-out commit =='
git rev-parse HEAD

echo '== object availability and ancestry =='
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '
git merge-base "$base" "$head"
printf 'base is ancestor: '
git merge-base --is-ancestor "$base" "$head" && echo yes

echo '== exact changed paths =='
git diff --name-status "$base" "$head"

echo '== candidate telemetry/checkpoint files =='
git diff --name-only "$base" "$head" \
  | while IFS= read -r path; do
      case "$path" in
        *.py|*.ts|*.tsx|*.js|*.jsx|*.go|*.rs)
          ast-grep outline "$path" --items all --view compact || true
          ;;
      esac
    done

echo '== PR metadata and current reviews/checks summary =='
gh pr view 92 --json number,state,isDraft,baseRefOid,headRefOid,reviews,statusCheckRollup \
  --jq '{number,state,isDraft,baseRefOid,headRefOid,reviews:[.reviews[]|{author:.author.login,state:.state,commit:.commit.oid}],checks:[.statusCheckRollup[]|{name:.name,status:.status,conclusion:.conclusion,headSha:.headSha}]}'

Length of output: 4616


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='be9a2ff90aa4b4219462392c2a49ff5b3218a1a8'
head='2235ec3eec2f73cb7ea3117c7b881d3bd1f0bdb7'

echo '== Python module maps =='
for path in pg_llm_batch/checkpoint_telemetry.py pg_llm_batch/__init__.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; do
  echo "--- $path"
  ast-grep outline "$path" --items all --view signatures || true
done

echo '== implementation and public API =='
cat -n pg_llm_batch/checkpoint_telemetry.py
cat -n pg_llm_batch/__init__.py

echo '== focused telemetry contract tests =='
for path in tests/test_checkpoint_telemetry.py \
  tests/test_checkpoint_telemetry_clock.py \
  tests/test_checkpoint_telemetry_concurrency.py \
  tests/test_checkpoint_telemetry_exception_boundary.py \
  tests/test_checkpoint_telemetry_span_status.py \
  tests/test_checkpoint_telemetry_storage_semantics.py \
  tests/test_checkpoint_telemetry_public_api.py \
  tests/test_checkpoint_telemetry_documentation.py; do
  echo "--- $path"
  cat -n "$path"
done

echo '== docs and instruction changes =='
git diff --unified=80 "$base" "$head" -- \
  docs/adr/0008-checkpoint-opentelemetry-observability.md \
  docs/checkpoint-observability.md \
  docs/doctoring/checkpoint-opentelemetry-observability.md \
  AGENTS.md ARCHITECTURE.md CLAUDE.md CHANGELOG.md

Length of output: 132122


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='ContextualWisdomLab/pg-llm-batch'
base='be9a2ff90aa4b4219462392c2a49ff5b3218a1a8'
head='2235ec3eec2f73cb7ea3117c7b881d3bd1f0bdb7'

echo '== durable-store interface map =='
store_path="$(fd -t f '^checkpoint_store\.py$' pg_llm_batch | head -n 1)"
test -n "$store_path"
ast-grep outline "$store_path" --items all --view signatures
rg -n -A24 -B4 \
  '^\s*def\s+(load|load_in_transaction|save|save_in_transaction)\s*\(' \
  "$store_path"

echo '== wrapper call and composition sites =='
rg -n -C3 \
  'OpenTelemetryCheckpointStore|\.load_in_transaction\s*\(|\.save_in_transaction\s*\(' \
  pg_llm_batch tests

echo '== exact-head diff hygiene =='
git diff --check "$base" "$head"
printf 'changed path count: '
git diff --name-only "$base" "$head" | wc -l
printf 'changed Python path count: '
git diff --name-only "$base" "$head" -- '*.py' | wc -l

echo '== `#92` and `#78` metadata; source comparison only =='
gh pr view 92 --repo "$repo" --json headRefOid,baseRefOid,body,commits \
  --jq '{headRefOid,baseRefOid,body,commits:[.commits[]|{oid:.oid,messageHeadline:.messageHeadline}]}'
gh pr view 78 --repo "$repo" --json state,headRefOid,baseRefOid,files \
  --jq '{state,headRefOid,baseRefOid,files:[.files[]|{path:.path,additions:.additions,deletions:.deletions}]}'

echo '== direct check runs for requested exact head =='
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/$repo/commits/$head/check-runs?per_page=100" \
  --jq '[.check_runs[]|{name,status,conclusion,head_sha:.head_sha,app:.app.slug,details_url}]'

echo '== current `#92` review records; do not use `#78` reviews =='
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/$repo/pulls/92/reviews?per_page=100" \
  --jq '[.[]|{user:.user.login,state,commit_id,submitted_at}]'

echo '== references to superseded pull request in current PR materials =='
gh pr view 92 --repo "$repo" --comments \
  --json body,comments \
  --jq '{body,comments:[.comments[]|{author:.author.login,body:.body}]}'
git log --format='%H%n%B%n---' "$base..$head"

Length of output: 45024


@seonghobae Reviewed 2235ec3eec2f73cb7ea3117c7b881d3bd1f0bdb7 against be9a2ff90aa4b4219462392c2a49ff5b3218a1a8.

I found no blocking issues in the requested scope.

  • The merge base equals the specified #60 head. The base is an ancestor of the reviewed head.
  • The delta contains 17 paths. The path set matches the 17 paths listed for #78. My earlier count of 18 was incorrect.
  • OpenTelemetryCheckpointStore preserves the load, save, load_in_transaction, and save_in_transaction method contracts. It delegates arguments unchanged.
  • Package-owned span and metric attributes are fixed and low-cardinality. The implementation excludes tenant, consumer, batch, endpoint, file, digest, cursor, DSN, payload, exception message, and dynamic exception-class values.
  • Failed operations use the bounded checkpoint_conflict, validation_error, or internal_error classifications. Successful operations omit error.type.
  • The wrapper disables automatic exception recording and automatic status-on-exception. It explicitly sets StatusCode.ERROR without a description when available. Successful spans retain Unset status.
  • Tracer, meter, span, status, clock, and telemetry-originated cancellation failures are isolated. The original result or exact application exception remains authoritative.
  • The direct GitHub check runs for this exact head completed successfully. These checks have head SHA 2235ec3eec2f73cb7ea3117c7b881d3bd1f0bdb7.

I treated #78 only as source provenance. I did not use its CI, review, or acceptance evidence. The current PR body also states that #78 evidence does not transfer.

This review does not replace the required regeneration of final-base, branch-protection, security, dependency, SBOM/provenance, packaging, and independent-review evidence after prerequisite integration.

You are interacting with an AI system.

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