Skip to content

fix(managedobserve): refresh rotated install tokens#296

Closed
michiosw wants to merge 1 commit into
mainfrom
feat/refresh-managed-observe-options
Closed

fix(managedobserve): refresh rotated install tokens#296
michiosw wants to merge 1 commit into
mainfrom
feat/refresh-managed-observe-options

Conversation

@michiosw

Copy link
Copy Markdown
Contributor

Where We Are

PR #295 was closed because it fixed ledger token rotation but left GitHub policy refresh on the startup install token. It also kept shrinking batches for generic hosted 400/422 validation errors, which can retry the same invalid payload shape instead of stopping.

Where We Want To Go

A running managed daemon should use a rotated install token for both policy refresh and ledger streaming. Hosted validation errors should fail once without advancing the cursor. Real 413 payload-size responses should still shrink batches, and a single rejected record should not be resent forever.

How do we get there

Reload managed config and the install token before each policy refresh and stream flush. Limit hosted shrink retries to 413, and advance past a one-record 413 after recording the diagnostic error.

Verified with:

  • go test ./internal/managedobserve ./internal/managedstream
  • go test ./...
  • go vet ./...
  • go test -race ./internal/managedobserve ./internal/managedstream

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR refreshes managed install tokens during long-running daemon work. The main changes are:

  • Reload managed config before GitHub policy refreshes.
  • Reload managed config before ledger stream flushes.
  • Stop shrinking hosted batches for generic 400 and 422 validation errors.
  • Advance past a single-record hosted 413 after recording a diagnostic error.

Confidence Score: 3/5

This is close, but the single-record 413 handling should be fixed before merging.

  • Token refresh paths are wired through the daemon refresh and flush loops.
  • Generic hosted validation errors now stop without advancing the cursor.
  • A generic 413 can now persistently skip a ledger record even when the error was not caused by that record being permanently too large.

internal/managedstream/stream.go

Important Files Changed

Filename Overview
internal/managedobserve/daemon.go Adds per-refresh and per-flush managed config reloads for policy and ledger token rotation.
internal/managedstream/stream.go Narrows batch shrinking to 413 responses and adds cursor advancement for one-record 413 failures.

Reviews (1): Last reviewed commit: "fix(managedobserve): refresh rotated ins..." | Re-trigger Greptile

Comment on lines 195 to 197
if limit == 1 {
return err
return advancePastMinimumBatch(statePath, batch, fmt.Sprintf("hosted status %d", hostedErr.StatusCode), err)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Avoid skipping generic 413s

This branch saves the cursor past a one-record batch for any HTTP 413. A 413 can come from a proxy, load balancer, or misconfigured route rather than from the ledger service proving this specific record can never be accepted. In that case the daemon records the cursor before returning the error, so the next flush starts after this action and the ledger entry is never retried or streamed. Please only advance here when the client has a local payload-limit violation or a structured hosted response that identifies the single record as permanently too large.

@michiosw

Copy link
Copy Markdown
Contributor Author

Closing this version. The replacement will keep the token-refresh fix but avoid the single-record 413 cursor skip that Greptile flagged.

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