Log domain events #2817
Log domain events #2817
Conversation
nelsonkopliku
added
enhancement
nelsonkopliku
force-pushed
the
log-domain-events
branch
4 times, most recently
from
4f9501b
to
13690ff
Compare
nelsonkopliku
force-pushed
the
log-domain-events
branch
from
13690ff
to
b3e1bed
Compare
| alias Trento.ActivityLog.ActivityLog | ||
| alias Trento.Infrastructure.Commanded.EventHandlers.ActivityLogEventHandler | ||
|
|
||
| test "should log a domain event" do |
There was a problem hiding this comment.
Pretty useless test 🙈, as we don't have anything really interesting happening here, except from calling the ActivityLogger. I can get rid of this.
| metadata: metadata | ||
| }) | ||
|
|
||
| :ok |
There was a problem hiding this comment.
A doubt I have here is: if for any reason logging a specific interesting domain event fails, we'd be continuing silently.
If we want to be more reliable and have the event handler retry, we might want to not always return :ok.
That means having ActivityLogger.log_activity possibly return an :ok/:error tuple.
I guess we can have this improvement as a followup.
| |> Enum.filter(fn | ||
| ["Trento", resource_type, "Events", _] | ||
| when resource_type in [ | ||
| "Hosts", | ||
| "Clusters", | ||
| "SapSystems", | ||
| "Databases" | ||
| ] -> |
There was a problem hiding this comment.
This expectation on the name structure of modules that contain events can be turned into a Credo check, for better enforcing consistency.
nelsonkopliku
force-pushed
the
log-domain-events
branch
2 times, most recently
from
f50947e
to
a17a387
Compare
nelsonkopliku
force-pushed
the
log-domain-events
branch
from
387465d
to
4814ab6
Compare
|
Hey @gagandeepb @balanza the whole macros stuff have been revisited and eventually removed. |
| for {event, expected_activity_type} <- events do | ||
| metadata = | ||
| event | ||
| |> Jason.encode!() | ||
| |> Jason.decode!() |
There was a problem hiding this comment.
Why do you encode and decode again?
There was a problem hiding this comment.
because what gets stored is an arrow map, while the event itself is an atom map
%{
"foo" => "bar"
}
# vs
%{
foo: "bar"
}Any suggestion on different approaches?
There was a problem hiding this comment.
Maybe something this should work/help:
Map.new(map_with_string_keys, fn {k, v} -> {String.to_exisiting_atom(k), v} end)There was a problem hiding this comment.
tried, not working tho 😅
protocol Enumerable not implemented for %Trento.Hosts.Events.HostRegistered (and any other event struct I guess)
Description
This PR extends Activity Logging capabilities so that all domain events can be tracked as activity log entries.
How was this tested?
Automated tests.