Telemetry slice 3 — DriverLocationUpdated to Kafka + ADR-019 transport-agnostic topic naming - #46
Merged
Merged
Conversation
…fka) Session prompt for PR C. The IDriverLocationPublisher seam shipped in PR #45 already carries the generated DriverLocationUpdated, so this session swaps only the implementation and must not touch the contract. A jasperfx-source-verifier pass ran during authoring and closed all six verify-before-wiring gates against local wolverine @ V6.21.0-12. It surfaced the session's one load-bearing fork: W006 §6.3 argues publish-first from failure-mode asymmetry, but a Kafka publishing endpoint defaults to BufferedInMemory, where PublishAsync returns before the broker acks — making that ordering nominal. Resolved to SendInline + UseSyncRetryBlock + UseIdempotentProducer. W006 §11 candidate #1 fires here (first Kafka topic lands) and lands as ADR-019 inside this PR, which also supplies the ADR-004 design-return interleave.
Swaps LoggingDriverLocationPublisher for a real WolverineFx.Kafka producer behind the unchanged IDriverLocationPublisher seam (W006 §6.3). CritterCab's first Kafka topic and second live transport; ReportLocationsHandler is untouched, which is what the seam existed for. Wiring: telemetry.driver-location-updated, partition key driverId, endpoint-scoped binary protobuf, AutoProvision, and the broker read by name so Aspire, the Testcontainer, and Event Hubs all work without branching. SendInline + UseSyncRetryBlock rather than the BufferedInMemory default. Buffered returns before the broker acks and Wolverine's default async retry block swallows the failure, which would have made §6.3's publish-before-store ordering true only in statement order — the upsert would land on a lost publish, the branch §6.3 argued against. Inline plus the sync block makes a rejection throw, so the baseline stays stale and the next ping republishes. Tests: a broker-backed round trip asserting the partition key and that the value is binary protobuf the generated parser accepts, on its own fixture so the other suites do not wait on Kafka; plus an ordering test pinning that a failed publish leaves no baseline. 33/33 green.
… retro Fires W006 §11 candidate #1, whose trigger was literally "first Kafka topic lands". ADR-019 generalizes ADR-014's <source-bc>.<event-name-kebab> across transports while explicitly withholding its two ASB-specific operational clauses — session keying and outbox coordination — behind a per-transport table. ADR-014 stays Accepted and authoritative for ASB, with a scope note pointing at 019. It also resolves a contradiction that predated this session: the wolverine-kafka skill had independently proposed a stream-descriptive naming rule for Kafka, so the repo held two conventions before either had a real topic to name. That skill's topic-naming and serialization sections are corrected here under the session-runner-blocking exception — a session cannot follow a skill that contradicts the ADR it is authoring. The listener-side examples still name a LocationPing/telemetry.location-pings pairing that never existed; deferred to PR D as DEBT rather than swapped for differently speculative names. W006's Document History records the publish-first qualifier as an amendment rather than a correction: §6.3's failure-mode argument is sound but never states its own precondition, that the publish outcome is known when the store runs. Three DEBT rows: wolverine-kafka listener examples, transport-selection's missing built-vs-modeled status axis, and the aspire skill's AddKafka example, which does not compile on 13.4.6.
…udit The topic-naming correction left section 'Convention-based routing' two sections below still justifying itself with the rejected descriptive-name rule, inside a file whose new banner claimed that section was reconciled. Rewritten to the real reason Cab declines convention-based routing: it would bind a wire-visible topic to a C# type name, so a refactor rename would repoint the producer silently. Fourth DEBT row from the same audit — service-bootstrap does not sanction the optional connection-string guard Telemetry has now used twice.
Standards axis, both live rather than theoretical: - Testcontainers were unnamed. Latent while the project had one container; slice 3 added a second Postgres and xUnit runs the two collections in parallel, so a fixed name now collides. Both fixtures get unique names. Following the skill's own fix required correcting it twice over -- WithPullPolicy is really WithImagePullPolicy, and PullPolicy lives in DotNet.Testcontainers.Images. - A guard clause inside the UseWolverine lambda would have silently swallowed any configuration appended below it on the broker-less path, and the connection string was branched on twice in opposite polarity. Read once into kafkaEnabled, extracted ConfigureKafkaPublishing. - StreamAsync/PingAt had been copied into three test classes. Extracted ReportLocationsClient; updated the pre-existing slice-2 copy too, since leaving one caller off a helper this session introduced is worse than the duplication it replaces. Spec axis: closed deliverable 7 (the proto comment now records shipped state), and dropped two speculative topic names the skill correction had minted in the same file whose new banner warns against speculative names. Two DEBT rows added: testing-integration's wrong API names and its collection convention that no shipped fixture follows, and the CI-cannot-build-apphost gap. 33/33 green.
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
1 participant
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.
Realizes W006 slice 3 — CritterCab's first Kafka topic and second live transport. After this PR, two of ADR-005's three transports are running and only Azure Service Bus remains modeled-but-unbuilt.
Session prompt:
docs/prompts/implementations/008-telemetry-slice-3-kafka-publish.md· Retro:docs/retrospectives/implementations/008-telemetry-slice-3-kafka-publish.mdThe seam did its job
PR #45 introduced
IDriverLocationPublishertyped against the generatedDriverLocationUpdated, with the proto already in codegen. So this PR swaps one implementation class and the host wiring —ReportLocationsHandler.csdoes not appear in the diff at all.KafkaDriverLocationPublisheris one method.The load-bearing decision
W006 §6.3 argues publish-before-store from failure-mode asymmetry: a failed store after a good publish costs a duplicate the consumer's dedup absorbs, while a failed publish after a good store costs a miss Dispatch cannot detect. Sound — but it silently assumes the publish's outcome is known when the store runs. It is not, twice over:
BufferedInMemory;PublishAsyncreturns once the record is queued in-process, before the broker acks.SendInline()is insufficient alone, because Wolverine's default async retry block catches the send failure, logs it, re-posts to a background block and returns success.The naive implementation would have produced a system where publish-first is true in statement order only, every broker failure lands in the branch §6.3 rejected, and nothing looks wrong. Shipped instead:
SendInline()+Durability.UseSyncRetryBlock = true+UseIdempotentProducer(), so a rejection throws, the upsert is skipped, the baseline stays stale, and the next ping republishes.Recorded in W006's Document History as an amendment, not a correction — §6.3's reasoning stands; it did not name its own precondition.
UseSyncRetryBlockis process-global; affordable because Telemetry publishes to one transport.ADR-019
W006 §11 candidate #1's trigger was literally "first Kafka topic lands," so it fires here. ADR-019 generalizes ADR-014's
<source-bc>.<event-name-kebab>across transports while explicitly withholding its two ASB-specific operational clauses (session keying, outbox coordination) behind a per-transport table. ADR-014 stays Accepted and authoritative for ASB.It also resolves a contradiction that predated this session:
wolverine-kafka/SKILL.mdhad independently proposed a stream-descriptive naming rule for Kafka, so the repo held two conventions before either had a real topic to name. That argument was answered on its merits rather than dismissed.This ADR is also the ADR-004 design-return interleave for the third consecutive Telemetry implementation PR.
Wiring
telemetry.driver-location-updated· partition keydriverId· dedup keyserverReceivedAt· no outbox ·AutoProvision· binary protobuf, endpoint-scoped. Broker read by name, so Aspire (host port 5392, a slot theaspireskill had reserved), the Testcontainer, and Event Hubs all work without branching. Guarded on the connection string — no broker configured means the logging fallback and a service that still boots.Tests — 33/33 green
DriverLocationUpdated.Parseraccepts. The key assertion is the load-bearing one — without a key the records still arrive and only the ordering silently breaks.Honest gaps
aspire/SKILL.md'sAddKafkaexample does not compile on 13.4.6 — hit on first build. Unnoticed because CI cannot seeapphost.cs. DEBT row; the CI fix is its own session.testing-integration's[CollectionDefinition(Name, DisableParallelization = true)]convention is followed by no shipped collection. Repo-wide, so deliberately not fixed slice-locally.Review
jasperfx-source-verifier(6 gates, closed at authoring time) ·critter-skill-auditorPhase 1 + Phase 2 · two-axis/code-review. All four found disjoint problems; fixes for the actionable ones are in the last two commits. 6 DEBT rows registered.Next: PR D — slice 5, the Dispatch consumer, which also owns §6.3's Dedup GWT.