[Event Hubs] Correct the premature receiver-attach log and document the deferred attach - #5106
Draft
Johnathan W (j7nw4r) wants to merge 2 commits into
Draft
[Event Hubs] Correct the premature receiver-attach log and document the deferred attach#5106Johnathan W (j7nw4r) wants to merge 2 commits into
Johnathan W (j7nw4r) wants to merge 2 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 3 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
`open_receiver_on_partition` builds an `EventReceiver` and returns. The AMQP link attaches on the first `stream_events()` poll, but the method logs "Receiver attached on partition." before it returns. The log claims an attach that did not happen, and it claims it again when the attach later fails. Add three tests in the `consumer::tests` module: - `open_receiver_on_partition_logs_a_deferred_attach` asserts the log says the link attaches on the first poll, and that the old attach claim is absent. This test is red until the source changes. - `open_receiver_on_partition_defers_the_attach_to_the_first_poll` arms an attach error and shows that the open succeeds and the first poll fails. It pins the lazy contract. - `open_receiver_on_partition_never_logs_an_attach_that_failed` shows that a failed attach records no attach message. Add a `LogBuffer` writer and a `capture_logs` helper that install a thread-local `tracing` subscriber, plus two helpers that build an unconnected client. The tests use `MockCredential` and reach no network.
`ConsumerClient::open_receiver_on_partition` does no network I/O, but it
wrote `info!("Receiver attached on partition.")` before it returned the
`EventReceiver`. The AMQP link attaches on the first poll of
`stream_events()`, and the attach closure in the recoverable connection
already writes the truthful record. A reader of the log saw an attach
that had not happened, and the log kept that claim even when the attach
failed later.
The message now says the call created the receiver, and that the link
attaches on the first poll. The level, the position, and the four
fields stay the same. No eager attach is added.
The documentation on `open_receiver_on_partition`, `EventProcessor::run`
and `PartitionClient::stream_events` now states where the attach
happens, and that the service reports an unknown consumer group or an
unknown partition id from that first poll. The old text also named
`MessageReceiver`, a type that does not exist.
Refs #5094
Johnathan W (j7nw4r)
force-pushed
the
j7nw4r/fix-eventhubs-receiver-attach
branch
from
August 25, 2026 18:13
90c20e8 to
6339dc1
Compare
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.
Summary
ConsumerClient::open_receiver_on_partitionloggedReceiver attached on partition.before it opened any AMQP link, which misled anyone reading a trace of a failed connection. This change corrects that log line and documents the deferred attach on the three public surfaces that depend on it. The eager-attach part of the issue is deliberately not implemented and needs a maintainer decision. See "Open decision" below.Motivation
open_receiver_on_partitionperforms no network I/O. It builds anEventReceiverand returns it, and its only fallible step is a URL parse. The AMQP link attaches on the first poll ofstream_events(), which is where the broker reports an unknown consumer group or an unknown partition id. The old log line claimed the attach had already happened, and the doc comment claimed the call "establishes a connection", so a caller who checked the result believed the receiver was connected when it was not. A truthful attach record already exists at the real attach site inRecoverableConnection::ensure_receiver, so the premature line was a duplicate rather than the only record.Changes
info!inopen_receiver_on_partitionto state that the receiver was created and that the AMQP link attaches on the firststream_events()poll.info!("Attached receiver on partition.")inRecoverableConnection::ensure_receiveruntouched, so a real attach is still recorded once per attach and not once per delivery.open_receiver_on_partitiondoc comment, which said the call "establishes a connection" and twice namedMessageReceiver, a type that does not exist.EventProcessor::runand onPartitionClient::stream_events.add_partition_clientthat stopped in mid sentence.No public API changes. The diff touches doc text, one log message, one comment, the CHANGELOG, and tests.
Open decision
Issue items "attach the receiver inside
open_receiver_on_partition" and "report an invalid consumer group fromEventProcessor::run()" are not implemented here. The second cannot be met without an eager attach somewhere, and each placement changes public behavior, so the choice belongs to a maintainer.open_receiver_on_partition. This reports the error where the caller asks for the receiver, but it adds a network round trip to every call and can break a caller that opens many receivers up front.ConsumerClient.NewPartitionClientstays lazy, andProcessor.openPartitionClientImplforces the link with the comment "if we're stealing we want to stake a claim now, rather than later when the user actually calls ReceiveEvents()". It also stakes the epoch claim at dispatch time, which changes load-balancing timing between processor instances.Two facts to weigh, both unverified here because live tests were out of scope for this change. The broker condition for an unknown consumer group is inferred to be
amqp:not-foundwith the description "The messaging entity ... could not be found", and it is not established whether the CBSput-tokencall inauthorize_pathrejects an unknown consumer group before the link attach does. If it does, the second item is reachable with no link attach at all, which would be the better fix.AmqpErrorCondition::NotFoundalready exists inazure_core_amqp, so neither option needs new plumbing there.For reference, the .NET processor validates by opening a consumer on a random partition and doing a 5 ms probe read inside
StartProcessingAsync, not by a cheap metadata call, and the .NETEventHubConsumerClientis lazy in the same way this client is.Test plan
Three offline tests in
src/consumer/mod.rs. None needs a namespace, a credential, or a recording.open_receiver_on_partition_logs_a_deferred_attachcapturestracingoutput and asserts that the new message is present and thatReceiver attached on partition.is absent. Proven red before the fix, with the old string visible in the captured text.open_receiver_on_partition_defers_the_attach_to_the_first_pollarms an attach error and shows that the open returnsOkwhile the first poll surfaces the error. It is green by design and pins the lazy contract. Mutation proof: inserting an eager attach before the return makes it fail.open_receiver_on_partition_never_logs_an_attach_that_failedasserts that nothing records an attach when no attach happened, with a positive anchor assertion so it cannot pass vacuously. Mutation proof: moving the attach log onto the per-poll path makes it fail.Gates run locally, each with
CARGO_BUILD_JOBS=1:cargo test --no-runwithRUSTFLAGS=-Dwarnings(exit 0), the lib suite with--test-threads=1(147 passed, 0 failed, 14 ignored),cargo fmt --check,cargo clippy --all-targetswith-Dwarnings,cargo doc --no-deps --all-featureswithRUSTDOCFLAGS=-Dwarnings, 45 doctests, and cspell on the four changed files. No live test ran.Refs #5094. This pull request does not close that issue. Two of its four proposed items stay open: reporting an invalid consumer group from
EventProcessor::run(), and attaching the receiver insideopen_receiver_on_partition. Both need a maintainer decision, and the options are set out below.