Event Logging module: new hook to add event types

[Phorum]

I would like to propose an enhancement to the core Event Logging module: add a hook that other modules can call to register their own event types into the EVENT_TYPES structure. The benefit to this is that modules can then choose to place their log types on the Event Logging settings pages, rather than on their own settings pages (well, they could if they want, but at least there would be a choice).

The code to do this is very simple, just put something like the following in ./mods/event_logging/event_logging.php:

{{{
// Allow other modules to register their event types to be controlled by the
// Event Logging settings page.
if (isset($GLOBALS["PHORUM"]["hooks"]["event_logging_types"]))
{
$eventtypes =& $GLOBALS["PHORUM"]["DATA"]["MOD_EVENT_LOGGING"]["EVENT_TYPES"];
$eventtypes = phorum_hook('event_logging_types', $eventtypes);
}
}}}

I have attached a modified event_logging.php.

Using the hook would be very easy within a module:

{{{
function phorum_mod_foo_event_logging_types($data)
{
$data["Module Foo events"] = NULL;
$data["mod_foo_event_1"] = "Cubs win the World Series";
$data["mod_foo_event_2"] = "Hell has frozen over";

return $data;
}
}}}

Reported by: [email protected]
Imported from TRAC: http://trac.phorum.org/ticket/930

[Phorum]

We understand your idea, but we have some issues with it. First of all a minor issue: no hook docs are provided with the patch. Especially since adding the data to the array is not really intuitive, hook docs need to be provided to explain what the hook does and how it works. We could write those docs ourselves of course, no problem with that.

The other issue is more architecture related. The way in which the current system was implemented, makes enabling/disabling part of and the responsibility of the module that wants to log events. To me this makes sense, because an admin that wants to configure a new module, will be looking at the settings screen of that module. When storing the logging features in the event logging settings screen, a pointer is required in the module's settings to tell the admin to go do event logging to configure logging options. That feels counterintuitive to me.

I do understand the idea though. When an admin is configuring his logging settings, then he might expect logging for modules to be part of the event logging module settings. In my intended architecture this is not the case, so he might overlook the log setting.

These two sides of the story are that much equal IMO, that the only complete solution would be that both the module X and the event logging module would provide options for configuring the logging of the module X. The settings should appear in both settings screens.

In the chat I talked about this with Thomas and here's a relevant part of that chat:

{{{
11:17 < maurice> Hrm. one thing that comes to mind:
11:17 < maurice> the settings are stored in the event logging module's settings.
11:17 < maurice> It would be nicer integrated if the external logging functions would also
take an event name.
11:17 < maurice> That event name could check if the setting was enabled.
11:17 < maurice> IMO that would be the proper way to integrate the settings.
11:18 < maurice> Without such integration, all the hook does is adding a config property
to the event logging data.
11:18 < maurice> The calling module would still need to investigate the event logging
module's settings to decide whether or not a log message should be
written.
11:18 <~thomas[mysnip]> right
11:18 < maurice> That makes the hook a bit hacky for me.
11:19 < maurice> Based on this, I'd vote "no", changing the ticket to an idea for
extending the event logging module with an easier way to fully integrate
with another module.
}}}

To not create a hook now that induces some hacky way to link event logging and other modules, I change this ticket to a more general idea ticket for the event logging module to make integration better and easier.

Features that I'd like to see:

• A way to let module X register logging events that they want to log;
• The event logging module would automatically determine the module for which these events are registered and automatically creates a new section in the event logging settings for this;
• The module X can use a simple call from the event logging module to add the event logging settings to the module X's settings screen as well.
• Allowing the module X logging calls to include an event name, which is used by the event logging module to decide - based on the event settings - whether or not the event should be logged.
• The new code should be backward compatible with the current code.

This way, things are structured and settings can be applied on both settings screens.

I will design and implement a system for this in the next update of the event logging module.

By: mmakaay

[Phorum]

Replying to [comment:1 mmakaay]:

Thanks for adding this. I like the proposed features, they perform the objective I was after in the first place (to encourage/support consolidating event settings). One other issue that you are resolving is the actual storage of log setting data, which IMO is better served by the event logging system, not the module itself. Another is that the event logging system itself is the best final word (again IMO) on whether an event should be logged. The module code (or even any other piece of code) is only worried about whether it is reporting a candidate loggable event, not whether or not an Admin has enabled/disabled logging for that event.

The change will result in cleaner and more consistent module code in modules that choose to use logging, and may even encourage more event logging from modules.

-phil.

By: [email protected]

[Phorum]

Replying to [comment:1 mmakaay]:

Point noted regarding documentation of proposed hooks. I will remember that point in any future changes I propose. -phil.

By: [email protected]

[Phorum]

By: ts77