Skip to content

bake monolog into the container and add installer plugin to configure - #33

Open
ufundo wants to merge 1 commit into
civicrm:mainfrom
ufundo:monolog
Open

bake monolog into the container and add installer plugin to configure#33
ufundo wants to merge 1 commit into
civicrm:mainfrom
ufundo:monolog

Conversation

@ufundo

@ufundo ufundo commented May 8, 2025

Copy link
Copy Markdown
Contributor
  • add a copy of the Monolog extension into to the civicrm container
  • add an installer plugin to:
    a) enable Monolog on install;
    b) configure it to disable the standard monolog configs, and add a single config which directs all logs to stdout => so they show in the docker logs

The monolog release is a slightly forked version . This is because the main branch currently restricts stdout logs to when you are using CLI - whereas we want web requests to send their logs through to stdout also. I also did a manual package release with the monolog composer dependencies.

Hopefully we can get that merged and then use an "official" release. But it should work as is.

@ufundo

ufundo commented May 8, 2025

Copy link
Copy Markdown
Contributor Author

ping @michaelmcandrew @JoeMurray

return;
}

\Civi\Setup::log()->info(sprintf('[%s] Handle %s', basename(__FILE__), 'installDatabase'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when an attempt to log an info message occurs prior to initiation of logging infrastructure? Perhaps we should hold off until successful install has completed.

@ufundo ufundo May 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is standard boilerplate from all of the installer plugins, and is using a bespoke logger I think. It is printed to screen if you install with cv, not sure if it goes anywhere on web install.


// create new config to pass all to stdout
\Civi\Api4\Monolog::create(FALSE)
->addValue('name', 'docker_std_out')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm Bikeshedding here, but the name is stdout not std_out.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std_out is used throughout the existing monolog extension code... so I was following that I guess?
https://lab.civicrm.org/extensions/monolog/-/blob/master/Civi/MonoLog/MonologManager.php?ref_type=heads#L122

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

Successfully merging this pull request may close these issues.

2 participants