How to translate observer pattern for audit logs?

Hello again!

In Rails, I would just make an ActiveRecord Observer that observers ActiveRecord::Base (everything). Then in ApplicationController (or middleware), have a before filter that sets a global (really a thread local) User.current var. Then the observer can match up that user to the changes and write to the log.

How to do this in Elixir/Phoenix/Ecto?

Thanks for the help!

2 Likes

In Rails, I would just make an ActiveRecord Observer that observers ActiveRecord::Base (everything)

vs.

Then in ApplicationController (or middleware), have a before filter that sets a global (really a thread local) User.current var.

I can’t decide which sentence better describes the sad state of Ruby on Rails programming and architecture. Moreover, this is a hack that has been so deeply taught into our brains that we want to move it to Elixir.

The problem of audit logs cannot be reliably solved the way you describe, even in Rails. Logging should be built into your application if you want a trail of who did when what, you should explicitly save it.

In Ecto 1.0 you have callbacks. And you probably can build what you described above. But don’t. Ecto 2.0 does not have callbacks.

Instead, if you want to track your database writes, you could use transactions with Ecto.Multi. Basically after each update, remove, edit etc. you have another operation that saves the operation performed before to database. Check out this code for reference:

https://github.com/hexpm/hex_web/search?utf8=✓&q=Multi

I am pretty sure @wojtekmach or @ericmj can tell you if this works for Hex or there are some problems resulting from using this approach.

8 Likes

@hubertlepicki from someone who has never really used Rails and does not know a lot about Ruby or Rails could you explain what makes those two sentences bad?

2 Likes

OK, it is a hack. For once, you try to work around constraints of the framework by breaking it’s basic rules. Secondly, you introduce a global, accessible from anywhere during few/resp life cycle. Finally, you hook into callback mechanism. Your callbacks will be shoot now weather you want it or not, also from background jobs, places where you did not set the user variable globally etc. This means more if clauses, more bugs, etc.

Basically I’ve learned to prefer behavior that is easily visible/readable to deeply hidden callback functions, when possible.

3 Likes

I’m not aware of any problems so @ericmj is a better person to ask - especially for any problems and potential operational or maintenance difficulties. I ported the auditing code from ecto 1 to ecto 2 and it was very smooth transition.

My design goal when building the auditing subsystem was to make things explicit, so we have to explicitly audit each controller action we care about (we explicitly pass the data we care about too). On one hand it may seem like a lot of unnecessary boilerplate, but in reality it’s just one/two additional lines per controller action - no big deal. If we don’t want something audited we’re just not using it in the controller. Also, in Ecto 2 using Ecto.Multi is dead easy for this kinds of things. We have some conveniences to make the usage in controller be small and straightforward: controller_helpers.ex, audit_log.ex. I’m sure there’re better solutions to this problem, but as far as I’m aware this seems to be working OK.

I know that people also have success with pushing auditing all the way to DB using e.g. https://wiki.postgresql.org/wiki/Audit_trigger_91plus

3 Likes

@wojtekmach I have been using auditing on the database level, with some success, but in the end we decided to move to similar approach hex_web does. Less magic == better.

Auditing on database level does have disadvantages of:

  • difficulty of passing variables, i.e. current_user down to database requires some tricks
  • difficulty of filtering when you do not need some stuff to be audited.

Better add one line to each action and be done with it.

3 Likes

People have said why it’s bad, I’ll try to give a counterpoint:

$ rake routes | wc -l
1166

$ find app/models/ -name "*.rb" | wc -l
464

$ find app/workers/ -name "*.rb" | wc -l
164

(And that’s not counting all the stuff we have factored into gems or separate apps/repos.)

The technique I mentioned is easy and requires very little code in comparison to putting the logging logic into all the controllers/workers/service objects/etc (or even auditing them to see which ones need it). The real world is brutal sometimes. :024: Also, once it’s done, it’s done. If you add any more controllers/workers/models in the future, you don’t have to do anything or worry about it.

As with most things, it’s a tradeoff. While I agree with most of what @hubertlepicki says, sometimes the tradeoff is so worth it!

Thanks for all the replies; will check out Ecto.Multi!

3 Likes

You can wrap your Repo module function calls to require a user. Then it doesn’t even take any extra lines in each of your controllers or other locations.

5 Likes