Repository navigation
feat(s3): attribute S3 GET and PUT metrics to WAL sources - #3746
Open
ashsamantaray-stripe wants to merge 1 commit into
Open
ashsamantaray-stripe wants to merge 1 commit into
ashsamantaray-stripe wants to merge 1 commit into
Conversation
Committed-By-Agent: codex Co-authored-by: codex <noreply@openai.com>
ashsamantaray-stripe
marked this pull request as ready for review
November 8, 2026 15:24
ashsamantaray-stripe
requested review from
1sonofqiu,
Gezi-lzq and
superhx
as code owners
November 8, 2026 15:24
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
S3 GET/PUT metrics currently combine requests from all callers, which makes it difficult to identify how much request traffic comes from WAL persistence and routing. This is a WAL-focused first increment toward #2991.
What
Add a bounded
sourcelabel to the existing GET/PUT observations inkafka_stream_operation_latency, starting with the WAL paths involved in producer persistence and acknowledgement:operation_namesourceput_objectROUTER_WALget_objectROUTER_WALput_objectDELTA_WALDIRECTwrites.get_objectDELTA_WALLocal
DIRECTwrites skip RouterWAL. Local linked writes retain the RouterWAL PUT and avoid the remote payload GET. Existing callers without attribution receiveUNCLASSIFIED.These labels identify WAL ownership. Shared WAL readers and writers also retain their source during replay, close flushes, and synthetic trim writes, so the label alone cannot isolate requests for a single producer ACK. Broader classification of compaction, catchup reads, data-object uploads, startup reservation acquisition, multipart, LIST, and DELETE remains follow-up work.
How
WAL factories set the source explicitly, independently of the object-key layout. Readers and writers carry it in the existing request options, including both preferred-layout and fallback GETs. Options are copied at asynchronous submission so later caller mutation cannot relabel a pending request; the copy retains existing timeout and retry settings.
The source is attached at the existing success/failure observation points, including normal retries and fast retries. Existing counting boundaries, SDK retry behavior, read-merging decisions, routing, and append completion ordering are preserved. The change adds no S3 calls, object metadata, dependencies, configuration switches, or export timers.
GET/PUT histogram registrations are cached lazily by source and the existing status/size dimensions. The four enum values bound cardinality; unused values allocate no series. The label appears on count, sum, p50, p99, and max exports. Count and sum remain cumulative, and aggregation across sources recovers their previous totals. Percentiles describe each source's own latency distribution and cannot be added together.
Why
MIXEDexistsThe storage layer already coalesces eligible range reads of the same object through the same client. If every child read has the same source, the merged GET keeps that source. If sources differ, it uses
MIXED.For example, overlapping
ROUTER_WALandUNCLASSIFIEDreads can become one GET. Assigning it to the first caller would misattribute shared work; recording it once for each caller would inflate the count; splitting the reads would change S3 traffic.MIXEDpreserves one observation for the merged GET, with the same retry accounting as before.Interpret
MIXEDas a shared GET with conflicting caller attribution. Include it in totals; its label cannot apportion the request among callers. It is not expected in the normal producer flow: RouterWAL and DeltaWAL use distinct object paths, and this merge rule applies only to reads. Its series is created only when observed.Reviewer notes
DefaultRouterChannelProvider,DefaultWalFactory, andObjectWALConfigestablish WAL ownership;DefaultReader,DefaultWriter, andObjectReservationServicecarry it into GET/PUT options.AbstractObjectStoragepreserves the existing I/O paths and resolves attribution after eligible reads have been merged.ObjectStorageMetricsandOperationLatencyMetricspartition the existing histogram observations. Focused coverage protects source propagation, fallback reads, reservation-gated completion, retry/hedge accounting, buffer ownership, merged-read attribution, and cumulative exports.