enrich icon indicating copy to clipboard operation
enrich copied to clipboard

Common: re-validate the event and the contexts after all the enrichments

Open benjben opened this issue 5 years ago • 4 comments

At the moment the event and its contexts are validated at the beginning and then the contexts added by the enrichments are validated, but given that some enrichments can update the fields of the events (JS enrichment) and of its contexts (JS enrichment, PII enrichment), it's possible that enrich emits a non-valid event.

benjben avatar Sep 17 '20 07:09 benjben

I'm wondering if we can consider another path, e.g. being less permissive in what JS enrichment can do (prohibit mutating event). Then we can valildate only specified entities being PII'ed (we know from config) apart from ones already being validate by previous steps. But maybe it doesn't worth it - the benefit I see is marginal performance advantage.

chuwy avatar Sep 17 '20 08:09 chuwy

I would also like that, it would make life way easier when we will make EnrichedEvent a case class, but it would require us to notify all the customers who use JS enrichment to mutate the enriched event to update their logic and also the models based on the output events (i.e. get fields in contexts instead of in canonical event fields)

benjben avatar Sep 17 '20 08:09 benjben

Hey @benjben! Notifying clients is actually a very strong point. I was hoping to add case class in a very near feature, but I didn't realise how brekaing will be the change. How do you think we should proceed with case class migration?

chuwy avatar Sep 17 '20 16:09 chuwy

I was hoping to add case class in a very near feature

Same !!

How do you think we should proceed with case class migration?

This will require some design for JS enrichment. I hope that we won't need to pass a mutable copy of the case class and check the fields that have changed.

benjben avatar Sep 18 '20 07:09 benjben