Just catching up with add_to_log() replacement - Seriously?

Just catching up with add_to_log() replacement - Seriously?

napisao/la Howard Miller -
Number of replies: 17
Slika Core developers Slika Documentation writers Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers

Let's get this straight? Am I missing something. The nice simple call 'add_to_log' has been replaced with...

- create a class with 7 methods in it

- instantiate the class (having looked up the namespace, which I've forgotten)

- call the trigger method (because we don't really have logs anymore) to make it happen

....for ever log instance?

I don't want to trigger events. I just want to see that someone has looked at my block/report/whatever. 

The net result is that I will just remove the logging from my plugins because this is way too much hassle. Is it just me? Why couldn't 'add_to_log()' have been maintained as a wrapper around this for people who actually just wanted to log something...

Rant over tongueout

Prosjek ocjena:Useful (1)
In reply to Howard Miller

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la Paul Holden -
Slika Core developers Slika Moodle HQ Slika Moodle Workplace team Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers Slika Testers
This topic has already been done to death on these forums, yet here we are - our monthly rant against the Events API širom raširenih očiju

Really, is it that much work? It shouldn't take you more than a couple of minutes as you already seem to have figured out what you need to do...
In reply to Paul Holden

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la Howard Miller -
Slika Core developers Slika Documentation writers Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers

Sorry - I did look but I missed that. I'll go and have a look.

I guess my point is that (right now today) I couldn't care less about the Events API. I don't want to generate an event. I want to write something in the log. While I can vaguely see how we got here it's complication for it's own sake. 

Writing to a log (as a housekeeping function) should be dead simple. Two minutes, is 1 minute 50 seconds too long!!

In reply to Howard Miller

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la Davo Smith -
Slika Core developers Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers

I think the basic idea is that, if it's interesting enough to add to the Moodle logs, then there's a chance that someone will want to trigger something else when it happens.

Given that, it's helpful for all 'log' entries to also be events that can be hooked in to (I have come across many situations, writing Moodle plugins, whereby hooking into an 'add_to_log' call would have been an ideal way of avoiding modifying core code or forking a 3rd-party plugin).

In reply to Davo Smith

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la Howard Miller -
Slika Core developers Slika Documentation writers Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers

Yep - I guess that's what we are being asked to buy into. "You WILL create events!!!".

But... people are lazy. If it's not about getting the job done on time/budget then devs won't bother. (Almost) no specifications for third-party plugins will have "must create events" in them. Devs will just leave out the logging/events because it becomes something else to go wrong. 

If that's what was expected then it should have been *much* simpler so there is little effort required.

In reply to Howard Miller

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la sam marshall -
Slika Core developers Slika Peer reviewers Slika Plugin developers

It's not like third-party plugins already had proper logging even when it was just an add_to_log call. Hell, core Moodle doesn't have proper logging - in fact the most critical instances (the type where a member of staff has broken a website by changing some setting and you need to find out who it is so they don't break it again after it is fixed) tend to be missing.

And when I'm slagging off third-party plugins above, I'm largely referring to OU internal ones that we developed. smiješak Unless somebody paid particular attention, we very often forgot to add logging, even though we have quite good compliance with code guidelines and suchlike overall.

Anyhow: I don't think the new API is a particular problem for developing new code. It doesn't take long to do and it's an opportunity to think about what you need to log (for example do you want two different log events, or the same event with different parameters). Also it means the log events are properly document (i.e. look in /classes/event, you can see them all - vs look in db/log.php to find it doesn't exist or hasn't been updated and contains incorrect information anyhow).

It is tedious for existing code, though. I have probably migrated more than 20 events now so I should know...

Good news is that in some theoretical future when we're not all far too busy, we might be able to use the new log API to improve logging performance. smiješak I think writing log entries to database is one of the biggest bottlenecks to Moodle scalability, because it's one of the biggest causes of database writes, and db writes are really the key limit to scaling in most setups.

--sam

In reply to sam marshall

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la Howard Miller -
Slika Core developers Slika Documentation writers Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers

That's true - who writes logs to the database anyway namiguje

Oh - and MY plugins have logging in them. I love logs. Well... when I remember veliki osmijeh

In reply to Howard Miller

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la David Mudrák -
Slika Core developers Slika Documentation writers Slika Moodle HQ Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers Slika Plugins guardians Slika Testers Slika Translators
I think Davo described the reasoning behind this very well. What does lead you to "I don't want to fire an event, I just want to log"? If something happened in your plugin that is worth of logging, you probably consider that as an interesting/important moment/action for later analysis or so. Why should not be other areas of the Moodle codebase (be it core or other plugins) be given a chance to react to that moment, too? I personally see big benefits of the new events driven model that can be used for plugins interoperability. And I already saw some plugins making use of it in very elegant way.
In reply to David Mudrák

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la Howard Miller -
Slika Core developers Slika Documentation writers Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers

I understand from your perspective anyway. You write core code. 

I write weird plugins for clients under time and money pressure. Honestly, nobody is going to pay me to write elegant event hooks that will almost certainly never be used. 

The result is that old code that is now being updated for 2.7 etc. is having the log lines deleted. Don't shoot the messenger namiguje

In reply to Howard Miller

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la sam marshall -
Slika Core developers Slika Peer reviewers Slika Plugin developers
If your paying clients don't require logging, then I should think it's fine to remove it. Why log stuff people don't need anyhow? It can always be added later.

Just for info, all the OU's plugins (public and internal) are having the logs updated to the new system for our 2.7 release in December. We have about 200 plugins, although a large majority of them don't do any logging (sometimes for good reason as they're back-end or whatever). I wonder how many event classes we've added... (We aren't finished yet. I have some of mine still left...)

--sam




In reply to David Mudrák

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la Joseph Rézeau -
Slika Core developers Slika Particularly helpful Moodlers Slika Plugin developers Slika Testers Slika Translators

David "And I already saw some plugins making use of it in very elegant way."

Names, please.namiguje

Joseph

In reply to David Mudrák

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la sam marshall -
Slika Core developers Slika Peer reviewers Slika Plugin developers

I think one of the complications with the current system is that it is not suitable for plugin interoperability in general - it's basically only suitable for reacting to user actions.

For general plugin communication you'd want things like being able to communicate very efficiently between plugins in a manner which is *not* logged (because it doesn't correspond to a user action and you might make hundreds of such calls in a request). So far as I know there isn't a good generic mechanism for that yet; it's MDL-44078 and related discussion.

But the current system does have some good possibilities for plugins to observe log events - or would do if we had more events in useful places. smiješak For example, I have some situations where I need the system to do something after a restore finishes to fix specific details - unfortunately there isn't a 'restore complete' event yet (afaik).

--sam



In reply to Howard Miller

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la Joseph Rézeau -
Slika Core developers Slika Particularly helpful Moodlers Slika Plugin developers Slika Testers Slika Translators

Regarding events logging in add-ons, I have a question I asked myself in the course of updating the Questionnaire add-on to the new logging API, and might be asked for any add-on (or indeed core plugin) that uses log events.

As I was adding events, I obviously started with the legacy ones. Then I added some extra ones here and there, where I thought they might be useful/needed. But some admin or teacher using a plugin might think differently. They might a) never use some events I decided to include or b) need to log other events which I did not envisage.

Of course, for case a), it would be possible to put the list of events in the plugin settings (at site level) where an admin would then be able to decide which events in the list they want to be used. Does any core or add-on plugin does that at the moment?

Case b) is not so easy or maybe just not feasible.

Joseph

In reply to Joseph Rézeau

Re: Just catching up with add_to_log() replacement - Seriously?

napisao/la David Mudrák -
Slika Core developers Slika Documentation writers Slika Moodle HQ Slika Particularly helpful Moodlers Slika Peer reviewers Slika Plugin developers Slika Plugins guardians Slika Testers Slika Translators
I don't think this is a good way of thinking about this. Neither admin nor teacher "use" the event. If the event has no subscribers, it is simply logged and done. If those making reports are not interested in it, they will just not include it in the report. There is no need to give up on data just because we think we will never use it. Events data are too valuable, or may be in the future. The sooner you start gathering them, the higher value your database has.