Replies: 2 comments 3 replies
-
I would personally keep things simple at this stage, specially if the current approach is working well. However, the point you've raised about the admin events performing badly with JPA is valid and makes me question whether we should store these events along with the other events in the first place. I take it that the frequency of the admin events is a lot lower than the regular events, so perhaps having those in a separate table would make more sense in the JPA case? |
Beta Was this translation helpful? Give feedback.
-
While I see the benefit for a tree store with entities like users, I don't see such a benefit for storing events. IMHO an event should be stored as one element (maybe JSON?) without the option to split it across stores. I wonder if that might help to speed up the retrieval. Looking at the admin UI, I see there "login events" and "admin events". For "login events", there is the option to have an expiration time. In the UI, the two have different filtering options; for the "admin events" the are the "resource path", "resource types" and "operations type". I could also imagine setups with different storage types and different performance and retention characteristics for each. That being said: I'd say they should be separate on the logical layer, and deployments should be able to choose the storages for each type of event differently. If it makes sense for a specific storage implementation (JPA, Infinispan, etc.) to handle both types of events very similar, they could use the same code by implementing maybe two interfaces in the provider (?) as long as the deployment can choose if they want to use of only for one type of event or both types of events. |
Beta Was this translation helpful? Give feedback.
-
In this discussion, I would like to discuss all the details about storing events in the new storage.
What we have now
Two types of events
We have two types of events called
Event
andAdminEvent
.The purpose of
Event
is to audit all details about the user's behaviour, e.g. login attempts (failed/successful), registration, idp login, etc. More details can be found in docs.The purpose of
AdminEvent
is to audit all changes in Keycloak configuration (for example, when the administrator changes something in the Admin Console) e.g.Two interfaces
For managing/storing events we have two interfaces:
EventListenerProvider
andEventStoreProvider
.Implementations that implement only the
EventListenerProvider
interface are just listening to events while,EventStoreProvider
is able to both, store and provide events. Obtaining events from the store is used, for example, in the Admin console or the Account console.We have more implementations of listener e.g. JBossLoggingEventListenerProvider or EmailEventListenerProvider.
Currently, there is only one implementation of
EventStoreProvider
:JpaEventStoreProvider
.New storage questions/ideas
1
As part of the new storage work, we are replacing the old logic that was adding new ways of providing/storing data from/to Keycloak (e.g.
UserStorageProvider
) with the concept of the Tree storage, where different storage providers can be composed into a tree which is used when storing/obtaining data.The tree storage can be also leveraged for events. However, it may be a little more complicated to implement as with the new store one will need to implement the CRUD interface (for listeners, it should be possible to implement only create) and then there will be a configuration for the tree. However, the configuration would be more flexible, for example, one storage can store only some fields while the other stores everything and so on.
If we are interested in this option, I can try to investigate this and add more details.
2
Both entities represent an event at some point with some details, mostly strings. It should be possible to merge these objects to one entity together with some identifier whether it is
Event
orAdminEvent
. All event details can be represented byMap<String, String>
.Note this change will not mean any change outside the storage. We would still have
Event
andAdminEvent
. What it means is basically there will be only one entity and one table for both models. Outside the storage, this change will be transient.Conclusion
The current implementation of event listeners works well, we provide some listeners out of the box, and I would say there are many community implementations, therefore I am not sure whether it is worth changing this. Although, it would have some benefits to be able to use the tree store for configuring event processing. The simplest thing at the moment would be to leave EventListeners as is.
What we should really think about is how to design
MapEventStoreProvider
because we had some problems when storing a big amount of events in the database. The Admin console event functionality was basically unusable when a big number of events were present in the database, therefore we should think this through properly to make the new store ready for big traffic.Thank you for all your comments/suggestions.
Beta Was this translation helpful? Give feedback.
All reactions