Rotorsoft / eventually-monorepo

Reactive micro-services framework
MIT License
26 stars 6 forks source link

System artifact type removed #83

Open TomKaltz opened 1 week ago

TomKaltz commented 1 week ago

What is the intention of this removal and how should we seek to cleanly integrate Eventually with external systems going forward?

Rotorsoft commented 1 week ago

Good question. From my (limited) experience, I think external systems are "unnecessary complexity", especially when they’re used mainly to represent long-running external integration processes that are eventually handled as internal aggregates. This additional layer tends to be redundant, and treating external systems as internal representations can be seen as an anti-pattern. Basically, I'm questioning the need for "eventually" to support internal copies of the external streams... something that can be achieved with simpler mechanisms.

A cleaner approach is refactoring the design using a command adapter. This adapter can map external events directly to internal commands targeting an aggregate, eliminating the need for an intermediary "system" layer. If you've found scenarios where this pattern doesn’t apply or if the system abstraction proves useful, feel free to share your use cases and we can discuss alternative solutions.

TomKaltz commented 1 week ago

I see your points. I will admit that it's tempting to use Eventually for everything due to it's simple building blocks, strict message type safety and abstracted storage mechanisms.

For example, in my application I record incoming webhooks using an eventually system. This obviously gives me a way to record the webhook and also the abilty to use policies to send commands to relevent aggregates depending on the data that was received. The actual external system that I receive webhooks from has no api to query historical data, so it's very convenient and useful to store this data as it comes in using Eventually's event store. This storage abstraction perfectly meets this use case IMHO and doesn't require me to write boilerplate to store this data otherwise and then feed it into the domain model. Another advantage is that I can easily replay these events to policies or read models if needed by changing a subscription offset.

All that being said, I could refactor my webook system into an aggregate instead but that still does bring up the anti-pattern you mentioned of storing redundant or unnecessary external data in a domain service. To me, the tools available within eventually are just too powerful not to use. The typing magic is what tempts me the most to contort eventually into solving problems that the framework probably wasn't intended to solve.

Another example of how I use external systems is with a CommandScheduler system that schedules future timeout commands using BullMQ. I know very well that I don't need to implement this as an Eventually system but I find it very useful to use that system's internal event stream as a poor mans(or not so poor) logging/tracing facility. Again, perhaps redundant data and using the framework for unintended purposes. Something I may need to reconsider.

All-in-all I welcome the removal of the system artifact, I just wanted to mention my current use-cases. Maybe I am trying to contort Eventually into handling more things than it should. Either way, thanks @Rotorsoft for this great framework. It has given me the opportunity to learn a great deal about event driven systems in general.

Rotorsoft commented 1 week ago

Hey @TomKaltz,

Thank you for becoming a user and for helping me explore more potential use cases. I still view “eventually” as a small research project, but with the feedback and input from users like you, it might (eventually 😉) evolve into something real.

When I set out to create this framework, my goal was to keep it small, easy to grasp, and grounded in best design principles and patterns. At its core, there are two layers of abstraction to understand:

My current thinking is that by removing the “System” artifact from the logical layer, the framework will prevent users from introducing accidental complexity into the internal models. Note that the "storage layer" still provides functionality for managing external streams... which brings me to another key abstraction of event-sourced systems: the ability to record multiple independent event sub-streams within a time-correlated “all” stream.

The "stream" abstraction sits between the infrastructure layer and the business model, and it can be leveraged in your various integration scenarios. I’m actually planning future work in this area, focusing on two main aspects:

This is my current thinking, but since this is a research project, I’m open to new insights that could shift my perspective.

Thanks again for adopting the framework. If it ever evolves into something bigger, it’ll be because of users like you helping to shape it... something I won't be able to do alone.

TomKaltz commented 6 days ago

Ok so lets say in the case of webhook recording and without the System artifact I would use the store() directly to store the incoming webhook stream in my web application layer.

const IncomingWebhookEvents = {
    WebhookArrived: z.object({
      typescript: z.string(),
      provide: z.string(),
    }),
  };

  const committed = await store.commit<typeof IncomingWebhookEvents>(
    "incoming-webhooks",
    [{
        stream: "incoming-webhooks",
        name: "WebhookArrived",
        data: {
          typescript: "would",
          provide: "intellisense?",
        },
      }],
    { correlation: "sdfasdfasdf", causation: {} }
  );

I'm not very adept at Typescript generics so the above I don't think quite works. It was very convenient to use a System artifact where I could use Zod objects for my messages and Eventually would infer typing, validate my messages, etc. Going the above route, the messages for my sub-streams are not tracked in the builder so I'm on my own with validation, etc. I feel like this adds more complexity since I will have to build more boilerplate to do what an Eventually System would do for me but perhaps I'm just being stubborn and trying to avoid writing extra application code?😅

Rotorsoft commented 6 days ago

You should probably build a generic utility function to do this... shouldn't be too hard to refactor it out of the existing handler. At some point I will have to do it anyway to remove some of the code related to non-reducibles streams.

Guess you are also aware that the webhook recorder you are describing is kind of a "best effort" approach since you can loose events for a few reasons and a more robust solution with "at-least-once" delivery guarantees will require retries and/or backfilling logic on the producer or consumer side

TomKaltz commented 6 days ago

You should probably build a generic utility function to do this...

Yes, I need to actually learn how to work with and write complex generics in Typescript. I will admit I still don't understand exactly how all the typing magic works in Eventually.

Guess you are also aware that the webhook recorder you are describing is kind of a "best effort" approach since you can loose events for a few reasons....

Indeed I have run into concurrency issues with the store when webhooks arrive too fast. I added a backed-off retry wrapper to my CatchWebhook command call which alleviated the issue. (IMHO This sort of behavior I believe should be added to eventually with sane defaults.)

That reminds me....and it's a moot point now but when I started using a System for this type of stream I realized that the PG store implementation would allow for duplicate version numbers for system streams. The database would block it though since there was an indexed constraint on the events table for the version field. While I was tinkering I actually removed the constraint in the database index to allow for duplicate versions for the very reason of more reliability for writing system streams. My reasoning for the hack was that the store implementation already did the job of enforcing contiguous version numbers for reducible streams. I figured that I could get away the cheat for a system stream. It helped for my use-case but indeed I should probably be flogged for doing so.

That also reminds me, the event store should probably allow writing streams without versioning for streams the don't require strict order such as these types of sub-streams.

Rotorsoft commented 6 days ago

Answered in #84