Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows an alter an event and allows to stop sending it as well #18

Open
rthideaway opened this issue Jan 29, 2018 · 12 comments
Open

Allows an alter an event and allows to stop sending it as well #18

rthideaway opened this issue Jan 29, 2018 · 12 comments

Comments

@rthideaway
Copy link

rthideaway commented Jan 29, 2018

We would need a feature to extend an event array and also have an ability to completely prevent sending to the log service. Mostly because of environments which are not able to send the events.

At the moment I patched it with this patch. Allowing modules to alter an event and also send a flag to prevent sending completely.

@RoySegall
Copy link

You should do something like:

if (!empty($event['send']) && !$event['send']) {
  // Do something awesome.
}

Because there could be a case where the send key does not exist in the array.

@rthideaway
Copy link
Author

@RoySegall first of all thanks for pointing that out! I have it wrong, true, but I think the following would be enough for this particular case:

if (empty($event['send'])) {
  continue;
}

Simply if $event['send'] is not set to something positive, then we skip sending event to the log service.

@RoySegall
Copy link

So, in that case, we need to make sure that all the events will have the send property set to TRUE because now, the item is not initialized anywhere and that mean that all the events will be skipped by default and we don't want that, right? 😉

@rthideaway
Copy link
Author

rthideaway commented Jan 30, 2018

@RoySegall I hope the link to my patch isn't broken, but actually all the events already have send to TRUE:

+    $event['send'] = TRUE;
+    $module_handler = \Drupal::moduleHandler();
+    $module_handler->alter('logs_http_event', $event);
+
+    // If event does not want be sent, continue with the next one.
+    if (empty($event['send'])) {
+      continue;
+    }

@RoySegall
Copy link

I think that's OK but there are 2 missing:

  1. We need a test to make sure the functionality will work
  2. Make this as a PR so we could merge this properly.

Except that, @ordavidil Any thoughts on this one?

@rthideaway
Copy link
Author

@RoySegall PR: #20
I'll try to cover this with a test once I have a bit of spare time. Possibly during weekend.

@RoySegall
Copy link

looks awesome. Let's wait for tests.

@fago
Copy link

fago commented Feb 7, 2018

I just wondered why this is not done by overriding the "enabled" flag in the config via settings.php? This should achieve the same?

@rthideaway
Copy link
Author

rthideaway commented Feb 7, 2018

@fago If you would have config split per environment then yes. But if you have one config for all environments and you want to disable sending messages only on develop, then you have to keep your logs enabled and just disable sending to server. That was idea behind that flag.

@rthideaway
Copy link
Author

rthideaway commented Feb 7, 2018

@fago my bad, so you mean to override config in settings.php with
$config['logs_http.settings']['enabled'] = TRUE;
That should work yes :D But I still think that flag may be useful in some cases when you want disable sending messages in some specific case. Like ignoring specific "Info" message.

@nikunjkotecha
Copy link

nikunjkotecha commented Oct 25, 2018

for a commerce based project it is really important to keep all the messages around checkout and even for other projects it is possible that we want to keep messages of the all the types only for particular scenario and not for each and every message (for instance, info messages when user login but no messages around successful cron jobs)

+1 to this issue and IMO even solution is generic enough and would allow custom modules to control what to log and what not to log.

@scottsawyer
Copy link

Yep, this would be awesome, then I could create a module that would allow filtering logged events based on type and / or severity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants