eShopOnContainers icon indicating copy to clipboard operation
eShopOnContainers copied to clipboard

EPIC: eShop Modernization/Refresh

Open adityamandaleeka opened this issue 1 year ago • 13 comments

Creating a top-level issue for us to track the ongoing work to modernize/refresh eShop.

  • [x] https://github.com/dotnet-architecture/eShopOnContainers/pull/2064
  • [x] https://github.com/dotnet-architecture/eShopOnContainers/issues/2069
  • [x] https://github.com/dotnet-architecture/eShopOnContainers/issues/2076
  • [x] https://github.com/dotnet-architecture/eShopOnContainers/issues/2077
  • [x] https://github.com/dotnet-architecture/eShopOnContainers/issues/2093
  • [x] https://github.com/dotnet-architecture/eShopOnContainers/issues/2094
  • [x] https://github.com/dotnet-architecture/eShopOnContainers/issues/2095
  • [x] Remove superfluous UseDeveloperExceptionPage calls (it's the default)
  • [x] Fix DataProtection issues by configuring a data protection provider
  • [x] Replace Serilog with Microsoft.Extensions.Logging.Console
  • [x] Use ILogger<T> instead of injecting ILoggerFactory
  • [x] Replace Autofac with simple Microsoft.Extensions.DependencyInjection usage
  • [ ] #2108
  • [ ] Migrate from ad-hoc IConfiguration usage to strongly typed Options
  • [ ] Deduplicate use of gRPC and HTTP by using gRPC only for internal-facing microservices
  • [ ] Migrate to .NET container build (when we can support the .NET container build + docker compose scenario) and delete Dockerfiles

adityamandaleeka avatar Mar 22 '23 04:03 adityamandaleeka

cc: @erjain @ReubenBond

adityamandaleeka avatar Mar 22 '23 04:03 adityamandaleeka

EDIT: Merged into top comment * [ ] Migrate from ad-hoc `IConfiguration` usage to strongly typed Options * [ ] Replace Autofac with simple Microsoft.Extensions.DependencyInjection usage

ReubenBond avatar Mar 22 '23 16:03 ReubenBond

Why would you replace Autofac with something that has fewer features?

Also regarding IOptions be sure to read this first: https://www.dabrowski.space/posts/asp.net-options-why-you-should-not-use-it/

ardalis avatar Mar 22 '23 16:03 ardalis

EDIT: Merged into top comment * [ ] Use `ILogger` instead of injecting `ILoggerFactory`. Example: https://github.com/dotnet-architecture/eShopOnContainers/blob/0740fd42b12cbcf2e7ee0b69585bf01312adeddd/src/Services/Ordering/Ordering.API/Application/DomainEventHandlers/OrderStockConfirmed/OrderStatusChangedToStockConfirmedDomainEventHandler.cs#L14

ReubenBond avatar Mar 22 '23 22:03 ReubenBond

Why would you replace Autofac with something that has fewer features?

Autofac is undeniably more powerful and feature-rich. For the purposes of this project, however, I would rather we be explicit about what types in the application are services for DI and what interfaces they satisfy. It certainly should not be taken as endorsement of MEDI over Autofac.

Also regarding IOptions be sure to read this first: https://www.dabrowski.space/posts/asp.net-options-why-you-should-not-use-it/

The issue today is that stringly IConfiguration access is sprinkled around the codebase. The IOptions<T> type isn't supposed to be an aspiring GoF/OOP pattern, it's just a practical mechanism which works with C#'s type system and DI to attach certain functionality to a category of types. Specifically, configuration types and the functionality includes binding, programmatic configuration, validation, and post-config. In this case, the intention is to guide developers into a pattern which we consider to be a good pattern.

ReubenBond avatar Mar 22 '23 23:03 ReubenBond

EDIT: Merged into top comment * [ ] Fix DataProtection issues by configuring a data protection provider

ReubenBond avatar Apr 12 '23 16:04 ReubenBond

Many of these are addressed in #2107

ReubenBond avatar May 09 '23 14:05 ReubenBond

Is see that src/Services/Ordering/Ordering.API/Infrastructure/Filters/HttpGlobalExceptionFilter.cs (and from other services) is deleted. What is the reason for this? How are validation errors from in example FluentValidation handled. But also xxxDomainExceptions?

LockTar avatar Jun 12 '23 13:06 LockTar

We’re going to bring back some of it and use problem details instead of the custom format.

davidfowl avatar Jun 16 '23 08:06 davidfowl

Really great to see the plans to modernize eShopOnContainers - I have used it for a number of demos and developer training over the years.

One thing I noticed is that it seems like the latest version is no longer logging to Seq (presumably because of the move away from Serilog). Will this be restored or are there plans to send the logs elsewhere?

markheath avatar Jun 17 '23 14:06 markheath

I suggest to also update the Wiki, as the architecture documentation is quite out of the date and seems to relate mostly to the .NET 5 rewrite.

To pick on the previous comment as an example, Serilog alongside Seq is still documented as the current logging approach.

pjmlp avatar Jul 03 '23 09:07 pjmlp

Are we also implementing OpenTelemetry with this ongoing task to modernize the eshop app? If not, then can we add it?

cc @davidfowl @ReubenBond

ashutosh-tutwani avatar Aug 05 '23 05:08 ashutosh-tutwani

@adityamandaleeka - can this issue be closed now that we have https://github.com/dotnet/eShop?

eerhardt avatar Nov 15 '23 23:11 eerhardt