Move public changelog registry ownership to the scrubber Lambda - #3738
Move public changelog registry ownership to the scrubber Lambda#3738cotti wants to merge 2 commits into
Conversation
Phase 1 of elastic/docs-eng-team#688. The public registry.json was a log of upload operations (client-written, pass-through copied); every known consistency gap followed from that. The scrubber Lambda now derives it from the public bucket's actual state: registry = f(state), never f(event). - Extract the Lambda's top-level handler logic into testable classes in Elastic.Changelog: ScrubberProcessor (batch coalescing by key and group, object-level reconcile with post-write source validation) and RegistryReconciler (delimited/paginated group listing, ETag reuse with amends always recomputed, semantic idempotence, conditional PUT/DELETE with bounded jittered retries on 412/409, newer-schema refusal). Program.cs is now a thin adapter. - Retire the registry pass-through in the same deploy: registry-key events only schedule a group reconcile, so client-authored JSON no longer reaches the public bucket uninspected. - Add a producer (algorithm version) field to the manifest; a mismatch — including legacy pass-through manifests — forces a full metadata recompute and a write even when entries are identical. - Emit per-invocation reconcile metrics as CloudWatch EMF (the Phase 0 observability item that could only land with the reconciler). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b488f2f to
b0042e1
Compare
| namespace Elastic.Documentation.Lambda.ChangelogScrubber; | ||
|
|
||
| /// <summary> | ||
| /// Emits the per-invocation reconcile counters as a CloudWatch Embedded Metric Format line |
There was a problem hiding this comment.
This is overly defensive IMO, we use AOT trimming and source generated json contracts heavily. We should not fallback to handcrafted json emission.
These are emitted as cloudwatch metrics, should we push (some of) them as OTEL metrics too?
Mpdreamz
left a comment
There was a problem hiding this comment.
Request changes: drop per-product changelogs/<product>/registry.json
Pushing back on the core assumption in this stack — that we need (and should reconcile) a per-product / per-group changelogs/<product>/registry.json as a description of every changelog object under that prefix.
Why we don't need it
Release-note discovery is not "walk the registry and see what's there." The automated release discover moments (see elastic/docs-eng-team#698) always start from a list of PRs:
- Prestage → query for list of PRs
- GitHub release → already have a list of PRs
- Serverless release → query for list of PRs
Given that list, we can hit CloudFront and decide whether a checked-in changelog exists by whether the object returns 404. If it 404s, changelog bundle inlines a changelog for that PR into the bundle anyway.
The changelogs/ tree on S3 is only a mirror of explicitly checked-in changelogs in repositories. Those objects are not mandatory to exist at bundle time. A growing changelogs/<product>/registry.json (or changelogs/products/registry.json) would just accumulate forever for little consumer value — please remove that model and the reconcile/heal machinery built around it.
A lot of the code in this PR (group listing → entry-level reconcile → per-group manifest write/delete, producer/schema cutover heal, etc.) exists to keep that per-product registry correct. With the assumption dropped, that surface can shrink substantially.
What we do want instead
Keep (or introduce) two shallow folder-level registries on S3 for cache opt-out only:
changelogs/registry.jsonbundles/registry.json(orbundle/registry.json— match existing key layout)
Shape:
{
"<folder>": "<etag>"
}Where <etag> is the ETag of the last-touched file under that folder. Caching layers can then bail out eagerly before diving into a per-folder / per-product bundle registry.
Please rework this PR around that thinner contract: scrubber remains the public-bucket writer for changelog objects, but it should maintain the shallow folder→etag map — not a full per-product object inventory.
|
Agreed on the thinner contract. I'll rework this PR so the scrubber maintains the shallow folder→ETag maps ( |
…only
Rework after review: per-pool changelog/{org}/{repo}/{branch}/registry.json
manifests are no longer reconciled — they stay client-authored and are
mirrored verbatim (state-decided, so deletes propagate) until Phase 3
retires them. Group reconciliation is scoped to bundle/{product}/, the only
tree consumers enumerate.
The scrubber now also maintains shallow per-tree maps (bundle/registry.json
and changelog/registry.json) mapping each folder to an opaque change token,
so caching layers can skip unchanged folders with one small read. The token
digests the folder's sorted file/ETag listing rather than reusing the
last-touched file's ETag: deleting an older file must change the token.
Restacked on the scrubber-ownership chain: the scrubber Lambda owns the
public bundle/{product}/registry.json manifests and the shallow per-tree
maps (#3738), and the client-side refresh machinery is retired (#3760).
The migration command now writes YAML bundle objects only; the S3 events
those creates emit trigger the reconciliation that materializes the
manifests. Tests assert no registry.json is ever PUT.
Why
Phase 1 of elastic/docs-eng-team#688. The public
registry.jsonis produced client-side against the private bucket and pass-through copied by the scrubber Lambda, which makes it a log of upload operations rather than a description of the public bucket. Every known consistency gap follows from that: ad-hoc uploads leave it stale (the 2026-07-22 incident),--skip-etag-checkre-uploads can never reach a stale public manifest, deletions orphan entries, failed scrubs leave phantom entries, and the recorded ETag is documented as useless.Changes since the review
The review rejected per-product manifests as a growing inventory with little consumer value, and asked for shallow folder→ETag maps instead. Reworked accordingly — the two trees now part ways:
bundle/{product}/registry.jsonstays, reconciler-owned. This is the one place consumers genuinely enumerate: the{changelog}directive and external CDN consumers list a product's bundles, and date-based targets (serverless) are not derivable client-side. Registry-key events never pass through; they only schedule a group reconcile that derives the manifest from public state.changelog/{org}/{repo}/{branch}/registry.json) are no longer reconciled. Release-note discovery starts from PR lists (RFC elastic/docs-eng-team#698), so the scrubber stops owning these. They remain client-authored and are mirrored verbatim (state-decided, so Phase 3's private-manifest cleanup deletes will propagate) until Phase 3 retires them entirely —changelog bundlestill enumerates a pool through its manifest today, and 404-probing only works once entries are guaranteed one-per-PR.bundle/registry.jsonandchangelog/registry.json(matching the existing singular key layout), shaped{"<folder>": "<token>"}. One map write per touched tree per batch; optimistic concurrency (If-Match/If-None-Match: *, bounded jittered retries, batch-item failure on exhaustion); an absent or unparseable map is rebuilt from one full tree listing (which is also the first-deploy seed path); the map is conditionally deleted when its tree empties.Reconcile_TokenChangesWhenAnyFileChangesOrIsDeleted).What
The scrubber Lambda becomes the sole producer of the public bundle manifests and the shallow maps, computed by reconciling against the current state of the buckets —
registry = f(state), neverf(event):bundle/{product}/group then gets one registry reconcile from the public listing, and each touched tree gets one shallow-map reconcile./-delimited so verbatim branches don't sweep nested pools, paginated), reuses entries whose recorded ETag still matches, recomputes the rest from the scrubbed public YAMLs (amends always recomputed — their target depends on the parent), and writes back with optimistic concurrency. An empty group's manifest is conditionally deleted — absent ≠ empty for consumers. A manifest with a newerschema_versionis reported and left untouched.producerfield on the bundle manifest identifies the reconcile algorithm version; any mismatch — including every legacy pass-through manifest at cutover — forces a full metadata recompute and a write even when entries come out identical. ETags recorded are now the public object's, finally usable by CDN consumers.Elastic.Changelog(ScrubberProcessor,RegistryReconciler,ShallowRegistryReconciler,ChangelogContentScrubber);Program.csis a thin adapter. Per-invocation reconcile metrics are emitted as CloudWatch EMF via a source-generated contract, now including shallow-map write/unchanged counters (the docs-eng-team#688 Phase 0 observability item). TheFakeS3test fake (stateful, MD5 ETags, real conditional PUT/DELETE semantics, delimiter+pagination, interleaving hooks) is harvested from Add changelog registry state discovery and reconciliation commands #3717.The guarantee is convergence, not atomicity: any successfully processed event repairs all accumulated drift in the group and tree it touches. Consumers' listed-but-missing tolerance stays. The CLI upload path is untouched in this phase (#3760 retires the client-side refresh); rollback = revert the Lambda deploy.
Requires the IAM grants from elastic/docs-infra#360 (
s3:GetObject/s3:ListBucketon the public bucket) at deploy time.Testing
Elastic.Changelog.Tests: 868/868 passing — including 12ShallowRegistryReconcilertests (seeding, patching, deletion visibility, nested pools, unparseable-map rebuild, conditional-write races and exhaustion) and newScrubberProcessortests for pool pass-through and shallow-map coalescing../build.sh build --skip-dirty-checkclean; scrubber Lambda publishes Native AOT with zero trim warnings.docs/cli-schema.jsonuntouched); docs rewrite follows in Rewrite changelog registry docs for scrubber Lambda ownership #3761 once this shape settles.Part of elastic/docs-eng-team#688 (Phase 1).