Mainly looking for comments on whether the basics look right for now, before I start adding lots of audit calls to various parts of the codebase.

So, I'm wondering whether we want to keep with the free-form text record.
I would personally prefer a type of entry_provider_type,entry_provider,entry_type and then arguments.
So something like: audit('login', 'authtest', 'login', {'attributes':'test'}) or something.

And then the particular plugin that generated the audit entry would need a function to get a user-displayed string.
This way, we would have the information in a way that it could also be used by admins that for example want all users that used openid in the last week.

A more structured approach sounds fine to me.

You might add a comment over this line explaining why you want to reverse this list.

With this line indented, it seems that the following lines will not work.

I suggest adding docblocks on all the new classes, functions, and methods so they are well explained for future readers.

Consider adding a comment explaining what this method is for, and what the meanings of 1, 2, and 3 are.

It seems overall good, but I think it would really benefit from automated tests (for example, that indentation problem would be caught if there were tests).

Yeah, I fixed that when I spotted it earlier. None of the calls in this PR set timestamp, so they worked. Mildly surprised it passed the lint test (I think I did one before pushing).

Thanks for the comments. I'm most of the way through a rewrite based on Patrick's suggestion above.

Patrick, is this more like what you're thinking?

Would it be an idea to make event_arrs a double-star keyword-argument dict?
That way, functions can do things like self.audit('login', user=username, foo='bar')

But about the general idea: yes, this looks like what I was thinking of.

That seems a sane idea.

I'm fairly happy with this at the moment, although I think the Audit class' fetch_records function will probably need more work when we come to having record search pages (paginated results and the like).

I would prefer it if you don't use a raw SqlQuery, I'm working on getting rid of them in the rest of the code :).
Is there any chance you can either use the standard select type?
If not, I would like it if you could abstract this away in the Query or BaseStore layer.