Add Event-Driven Architecture BRD (Ask 3 proposal) - #1029
NannerySlalom wants to merge 6 commits into
Conversation
|
@NannerySlalom please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement ( “Agreement” ) is agreed to by the party signing below ( “You” ), 1. Definitions. “Code” means the computer software code, whether in human-readable or machine-executable form, “Project” means any of the projects owned or managed by .NET Foundation and offered under a license “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any “Submission” means the Code and any other copyrightable material Submitted by You, including any 2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any 3. Originality of Work. You represent that each of Your Submissions is entirely Your 4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else 5. Licenses. a. Copyright License. You grant .NET Foundation, and those who receive the Submission directly b. Patent License. You grant .NET Foundation, and those who receive the Submission directly or c. Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement. 6. Representations and Warranties. You represent that You are legally entitled to grant the above 7. Notice to .NET Foundation. You agree to notify .NET Foundation in writing of any facts or 8. Information about Submissions. You agree that contributions to Projects and information about 9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and 10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and .NET Foundation dedicates this Contribution License Agreement to the public domain according to the Creative Commons CC0 1. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved issues affect event contracts, tenancy, inventory semantics, delivery reliability, and acceptance criteria.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a BRD proposing a phased event-driven architecture rollout for the eShop.
Changes:
- Defines infrastructure, event contracts, subscriptions, and persistence epics.
- Adds roadmap, success criteria, staffing, and risk mitigations.
File summaries
| File | Description |
|---|---|
EVENT_DRIVEN_ARCHITECTURE_BRD.md |
Documents the proposed event-driven architecture and delivery plan. |
Review details
Suppressed comments (5)
EVENT_DRIVEN_ARCHITECTURE_BRD.md:107
- The stated persistence/replay criteria are insufficient for subscriber recovery. The existing
IntegrationEventLogServiceis a publisher-side outbox tied to the originating transaction, whileRabbitMQEventBushas no per-subscriber delivery state and acknowledges the message even when a handler throws. An event log alone therefore cannot replay a failed subscriber or support onboarding. Specify durable retention, subscriber delivery/attempt state, DLQ/retry/ack semantics, and a replay operation.
**Acceptance Criteria:**
- Generic event handler pattern available to all services
- Webhooks.API enhanced to subscribe to and forward events to external partners
- Event payload transformation/mapping examples
EVENT_DRIVEN_ARCHITECTURE_BRD.md:172
- Production event emission is scheduled in Phases 1–2, but the outbox/replay capability relied on by the risk mitigation is not operational until Phase 3. The current code already has direct publishers such as PaymentProcessor, and the RabbitMQ consumer acknowledges failed handlers, so broker or consumer failures before Phase 3 can permanently lose events. Make durability/retry a prerequisite for the first production events, or explicitly defer the reliability claim until Phase 3.
---
EVENT_DRIVEN_ARCHITECTURE_BRD.md:58
- The
without blocking order servicerequirement is not met by the current path:TransactionBehaviorawaitsPublishEventsThroughEventBusAsyncafter committing, and that method awaits the RabbitMQ publish/retry path inline. An outbox entry alone is not non-blocking until a background dispatcher exists; make that dispatcher part of Epic 3, or narrow the criterion to subscriber handling and add an outage-latency test.
- Example events from Order, Catalog, Basket, and Payment domains
EVENT_DRIVEN_ARCHITECTURE_BRD.md:184
- The reliability target is not actionable or supported by the current delivery semantics. The RabbitMQ consumer acknowledges messages even when handler processing throws (
src/EventBusRabbitMQ/RabbitMQEventBus.cs:159-180), while the outbox marks publish failures asPublishedFailed; an outbox/replay claim alone does not guarantee subscriber delivery. Define the SLO scope and at-least-once/exactly-once semantics, and require failed deliveries to remain recoverable through requeue or a DLQ.
**Phase 2 (Weeks 5–7): Catalog, Payment, Fulfillment Events**
EVENT_DRIVEN_ARCHITECTURE_BRD.md:195
outbox pattern ensures no event lossoverstates the mitigation. An outbox protects the producer transaction-to-message handoff, but broker replication, consumer retry/DLX, and replay are still needed; the current consumer acknowledges messages after handler exceptions rather than dead-lettering them. Separate producer durability from broker/subscriber recovery in this risk mitigation.
- Success metric: New developers can emit an event end-to-end
- Files reviewed: 1/1 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - Event contract library established (shared types across services) | ||
| - Versioning strategy defined (backward compatibility rules) | ||
| - Documentation template for event payloads (schema registry or OpenAPI extension) | ||
| - Example events from Order, Catalog, Basket, and Payment domains | ||
| - Contract testing framework in place |
| - PaymentProcessed, PaymentFailed events from Payment Processor | ||
| - InventoryReserved, InventoryReleased events from Catalog/Basket | ||
| - Fulfillment status events from OrderProcessor | ||
| - All events follow established contract patterns (Epic 2) | ||
| - End-to-end integration tests |
| - Generic event handler pattern available to all services | ||
| - Webhooks.API enhanced to subscribe to and forward events to external partners | ||
| - Event payload transformation/mapping examples | ||
| - Retry and idempotency logic documented |
|
|
||
| **Phase 1 (Weeks 3–4): Order Event Emission** | ||
| - Epic 3: OrderCreated, OrderStatusChanged events flowing | ||
| - Success metric: External systems (webhooks) receive order events in real-time |
| - Message broker (RabbitMQ, Azure Service Bus, or equivalent) provisioned and running | ||
| - Event bus abstraction layer added to `eShop.ServiceDefaults` | ||
| - Publisher/subscriber patterns documented and tested | ||
| - Local dev environment supports event broker (Docker Compose or Aspire integration) | ||
| - Monitoring and dead-letter queue strategy in place |
| - OrderCreated, OrderStatusChanged, OrderFailed, OrderShipped events published | ||
| - Events include order ID, customer ID, affected items, timestamps | ||
| - Existing Order API behavior unchanged; events fire alongside current logic | ||
| - Integration tests verify event publication | ||
| - Events flow to downstream subscribers without blocking order service |
| 1. **Real-time capability:** Downstream systems receive events within <500ms of domain event occurrence | ||
| 2. **Reliability:** 99.9% event delivery guarantee (with replay/outbox guarantees) |
Provide .env.example with required test credentials and optional custom base URL configuration. Default test users are alice/bob with password Pass123$. Supports PLAYWRIGHT_BASE_URL override for testing against non-default deployments.
- Add PLAYWRIGHT_BASE_URL environment variable support for running tests against custom deployments - Auto-enable ignoreHTTPSErrors when using HTTPS URLs (self-signed certificates) - Skip AppHost startup when custom base URL is provided (assumes server already running) - Maintain default localhost:5045 behavior when no custom URL specified - Remove commented-out browser configurations for clarity - Fixes issue where tests could only run against localhost:5045
- Replace actual passwords with placeholders (your_test_username, your_test_password) - Add clear documentation on where to find default test credentials - Reference UsersSeed.cs for Identity.API seed configuration - Update README with comprehensive E2E test setup instructions - Document how to run tests with custom base URL for remote deployments - Best practice: Never commit actual credentials to version control, even for tests
…nd disable strict table/emphasis rules
w00t sauce