Problem/Motivation

Send all Drupal log to AWS cloudwatch.

Steps to reproduce

Generate one log inside drupal

Proposed resolution

Add settings to enable logging inside cloudwatch when there are a log inside drupal.

API changes

-Add settings to enable and set log group/log stream.
-Add event to alter message/log group name/ log stream name before sending aws

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Musa.thomas created an issue. See original summary.

musa.thomas’s picture

Priority: Normal » Major
musa.thomas’s picture

musa.thomas’s picture

StatusFileSize
new13.32 KB

wrong patch sorry

musa.thomas’s picture

StatusFileSize
new17.28 KB

Some fatal error is trigger on the seettings form when the aws client is not ready.
Add context of log message inside event.

musa.thomas’s picture

jamesgrobertson’s picture

I've added a hook to alter the messages before they get sent to CloudWatch. I had a need to add additional data to the message that wouldn't make sense to log to messages stored in the database. Feedback is welcome!

musa.thomas’s picture

@jamesgrobertson why u need hook_alter inside event ..... strange.... just use the event

arthur_lorenz’s picture

Status: Active » Needs review
StatusFileSize
new19.97 KB
new6.71 KB

I extended patch #5 enable the users to create their own message format providing more metadata.

arthur_lorenz’s picture

StatusFileSize
new11.24 KB
new10.89 KB

Added the new config to the schema.

arthur_lorenz’s picture

StatusFileSize
new20.86 KB

I added a new config to enable/disable a check if the selected log group or stream is valid.

fago’s picture

Status: Needs review » Needs work

thx. the patch looks solid and is a critical feature I was expecting the module to already provide. let's get it added

When reviewing the code I found a small issue though, the following part needs some work on code comments to obey Drupal phpcs

+/**
+ * Event to use
+ *
+ * @SuppressWarnings(PHPMD.CamelCaseParameterName)
+ * @SuppressWarnings(PHPMD.CamelCaseVariableName)
+ */
+class AwsCloudwatchLogsSendEvent extends Event {
+
+  protected $message;
+
+  protected $logGroup;
+
+  protected $logStream;
+
+  protected $context;
+
arthur_lorenz’s picture

StatusFileSize
new21.05 KB

Thx, cleaned up the code and fixed cs issues.

arthur_lorenz’s picture

Status: Needs work » Needs review
aludescher’s picture

StatusFileSize
new21.36 KB
new754 bytes

The sendAws() method tries to create FormattableMarkup messages using the entire $context parameter which sometimes contains keys without a placeholder prefix.
This results in the following error message: Invalid placeholder (%s) with string: "%s".

From \Drupal\Component\Render\FormattableMarkup::placeholderFormat():

Support for keys without a placeholder prefix is deprecated in Drupal 9.1.0 and will be removed in Drupal 10.0.0.

Fix: filter the $context parameter before creating a FormattableMarkup message.

jamesgrobertson’s picture

In #3276441-15: Add feature sending drupal log to aws cloudwatch the message couldn't be altered in the event. I've changed it so that the message that gets sent to CloudWatch is the one returned from the event.

useernamee’s picture

Status: Needs review » Reviewed & tested by the community

@jamesgrobertson changes look good to me, except for the missing changes in the DeleteLogGroupForm which I plan to add manually.

@aludescher would you agree that we add this small change to your patch:

diff -u b/src/Logger/Log.php b/src/Logger/Log.php
--- b/src/Logger/Log.php
+++ b/src/Logger/Log.php
@@ -179,7 +183,16 @@
    $this->putLog->putLog(
      $aws_client,
-      $message_formatted,
+     $event->getMessage(),
      $event->getLogGroup(),
      $event->getLogStream()
    );

This code is already running on system so I'm marking this ticket RTBC.

musa.thomas’s picture

glad to see new maintener and this feature will be released !

  • e8cbae7c committed on 8.x-1.x
    Issue #3276441 by jamesgrobertson, arthur_lorenz, Musa.thomas,...

useernamee’s picture

Status: Reviewed & tested by the community » Fixed

merged.

  • 54b47da0 committed on 8.x-1.x
    Issue #3276441 by jamesgrobertson, arthur_lorenz, Musa.thomas,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.