kafka-ops / julie

A solution to help you build automation and gitops in your Apache Kafka deployments. The Kafka gitops!
MIT License
418 stars 113 forks source link

Introduce the concept of an AuditLog for JulieOps #484

Closed purbon closed 2 years ago

purbon commented 2 years ago

close #460

akselh commented 2 years ago

Hi @purbon, I missed this PR because of Easter holidays - but still would like to post some comments. Hope you are ok with that.

It is nice that you have added a way to get the audit log into something else than stdout. However I wonder why it was necessary to duplicate the audit log mechanism that is already established in JulieOps?

I am thinking of the system with the props methods on actions. I think it would be better to base the changes entirely on these methods and if needed extend/refactor the logging in the props methods. And then make if possible to either log this output to stdout, file and/or Kafka topic as required.

Like it is now, if stdout logging of the "new" audit log is configured a duplicated log will be found in stdout; i.e. output from both props() and detailedProps(). Most of the time these are pretty identical output, with the difference of this new resource name output.

Also the duplication of audit logging makes the code harder to understand. The new logging based on detailedProps is actually more explicit than the old one based on toString, so refactoring the old audit log into this would be better.

Out of curiosity, why is the special logging format of resource name needed? Besides the prefix "rn://" it seems to be a custom string based on each action type which contains information already in the log details?