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

feat: upgrade monolog/monolog to v3 #332

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .envrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
use flake
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ log/html.log
composer.lock
/.php-cs-fixer.cache
.phpunit.result.cache
.direnv
6 changes: 3 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
"minimum-stability": "dev",
"prefer-stable": true,
"require": {
"php": ">=7.4",
"php": ">=8.1",
"ext-curl": "*",
"ext-tidy": "*",
"fossar/htmlawed": "^1.2.8",
"guzzlehttp/psr7": "^2.0",
"j0k3r/graby-site-config": "^1.0.147",
"j0k3r/httplug-ssrf-plugin": "^2.0",
"j0k3r/php-readability": "^1.2.9",
"monolog/monolog": "^1.18.0|^2.3",
"j0k3r/php-readability": "dev-master",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any reason you are bumping readability?

Copy link
Owner

Choose a reason for hiding this comment

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

Because of psr/log, it's detailed in the PR description

Copy link
Author

Choose a reason for hiding this comment

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

Because readability 1x only supports psr/log 1.

Copy link
Author

Choose a reason for hiding this comment

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

Should I try 2.0.2?

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe you can put ^2.0 it'll be enough for the psr/log v3

"monolog/monolog": "^3",
"php-http/client-common": "^2.5",
"php-http/discovery": "^1.14",
"php-http/httplug": "^2.2",
Expand Down
25 changes: 25 additions & 0 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 44 additions & 0 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
description = "Dev environment";

outputs = { self, nixpkgs }:
let pkgs = nixpkgs.legacyPackages.x86_64-linux;
in {
defaultPackage.x86_64-linux =
pkgs.mkShell {
buildInputs = with pkgs; [
(php82.withExtensions
({ all, ... }: with all; [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if @j0k3r wants to include Nix packaging. I could continue to maintain it for the time being as I use something like this locally.


It might be sufficient to use enabled attribute to load the default extensions:

https://github.com/fossar/selfoss/blob/b0eb43d80bedde5da3204b842511afe245d8c17c/flake.nix#L37-L40

And make the PHP version parametric:

https://github.com/fossar/guzzle-transcoder/blob/816585c89c87268d1188d2d9696b49ce73541ff9/flake.nix#L29

Copy link
Owner

Choose a reason for hiding this comment

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

If you can maintain it, I'm ok.
I didn't what Flake was.
Maybe put a comment at the very top of flake.nix with a link to sth that explain what it does/is.

bz2
curl
dom
filter
fileinfo
gd
iconv
imagick
intl
mbstring
openssl
pdo
pdo_mysql
pdo_sqlite
session
sodium
sqlite3
tidy
tokenizer
xdebug
xmlwriter
# yaml
zip
zlib
])
)
php82Packages.composer
libjpeg
libpng
libyaml
jtojnar marked this conversation as resolved.
Show resolved Hide resolved
]; };
};
}
32 changes: 17 additions & 15 deletions src/Monolog/Formatter/GrabyFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
namespace Graby\Monolog\Formatter;

use Monolog\Formatter\HtmlFormatter;
use Monolog\Level;
use Monolog\LogRecord;

/**
* Formats incoming records into an HTML table.
Expand All @@ -18,32 +20,32 @@ class GrabyFormatter extends HtmlFormatter
/**
* Formats a log record.
*
* @param array $record A record to format
* @param LogRecord $record A record to format
*
* @return string The formatted record
*/
public function format(array $record): string
public function format(LogRecord $record): string
{
$output = '<table cellspacing="1" width="100%" class="monolog-output">';

$output .= $this->addRowWithLevel($record['level'], 'Time', $record['datetime']->format($this->dateFormat));
$output .= $this->addRowWithLevel($record['level'], 'Message', (string) $record['message']);
$output .= $this->addRowWithLevel($record->level, 'Time', $record->datetime->format($this->dateFormat));
$output .= $this->addRowWithLevel($record->level, 'Message', $record->message);

if ($record['context']) {
if ($record->context) {
$embeddedTable = '<table cellspacing="1" width="100%">';
foreach ($record['context'] as $key => $value) {
$embeddedTable .= $this->addRowWithLevel($record['level'], $key, $this->convertToString($value));
foreach ($record->context as $key => $value) {
$embeddedTable .= $this->addRowWithLevel($record->level, $key, $this->convertToString($value));
}
$embeddedTable .= '</table>';
$output .= $this->addRowWithLevel($record['level'], 'Context', $embeddedTable, false);
$output .= $this->addRowWithLevel($record->level, 'Context', $embeddedTable, false);
}
if ($record['extra']) {
if ($record->extra) {
$embeddedTable = '<table cellspacing="1" width="100%">';
foreach ($record['extra'] as $key => $value) {
$embeddedTable .= $this->addRowWithLevel($record['level'], $key, $this->convertToString($value));
foreach ($record->extra as $key => $value) {
$embeddedTable .= $this->addRowWithLevel($record->level, $key, $this->convertToString($value));
}
$embeddedTable .= '</table>';
$output .= $this->addRowWithLevel($record['level'], 'Extra', $embeddedTable, false);
$output .= $this->addRowWithLevel($record->level, 'Extra', $embeddedTable, false);
}

return $output . '</table>';
Expand All @@ -61,18 +63,18 @@ protected function convertToString($data): string
/**
* Creates an HTML table row with background cellon title cell.
*
* @param int $level Error level
* @param Level $level Error level
* @param string $th Row header content
* @param string $td Row standard cell content
* @param bool $escapeTd false if td content must not be html escaped
*/
private function addRowWithLevel(int $level, string $th, string $td = ' ', bool $escapeTd = true): string
private function addRowWithLevel(Level $level, string $th, string $td = ' ', bool $escapeTd = true): string
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will not work with Monolog 2, will it?

Copy link
Owner

Choose a reason for hiding this comment

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

At least there is a specific build to run on lower deps, so it's checked https://github.com/j0k3r/graby/actions/runs/5375860418/jobs/9752256955?pr=332#step:4:115

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I'm getting this error, I think I have monolog 2 installed.

Graby\Monolog\Formatter\GrabyFormatter::addRowWithLevel(): Argument #2 ($th) must be of type string, int gi

{
$th = htmlspecialchars($th, \ENT_NOQUOTES, 'UTF-8');
if ($escapeTd) {
$td = '<pre>' . htmlspecialchars($td, \ENT_NOQUOTES, 'UTF-8') . '</pre>';
}

return "<tr style=\"padding: 4px;spacing: 0;text-align: left;\">\n<th style=\"background:" . $this->logLevels[$level] . "\" width=\"100px\">$th:</th>\n<td style=\"padding: 4px;spacing: 0;text-align: left;background: #eeeeee\">" . $td . "</td>\n</tr>";
return "<tr style=\"padding: 4px;spacing: 0;text-align: left;\">\n<th style=\"background:" . $this->getLevelColor($level) . "\" width=\"100px\">$th:</th>\n<td style=\"padding: 4px;spacing: 0;text-align: left;background: #eeeeee\">" . $td . "</td>\n</tr>";
}
}
5 changes: 3 additions & 2 deletions src/Monolog/Handler/GrabyHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Graby\Monolog\Formatter\GrabyFormatter;
use Monolog\Handler\AbstractProcessingHandler;
use Monolog\Logger;
use Monolog\LogRecord;
use Monolog\Processor\PsrLogMessageProcessor;

/**
Expand Down Expand Up @@ -45,9 +46,9 @@ public function hasRecords($level): bool
return isset($this->recordsByLevel[$level]);
}

protected function write(array $record): void
protected function write(LogRecord $record): void
jtojnar marked this conversation as resolved.
Show resolved Hide resolved
{
$this->recordsByLevel[$record['level']][] = $record;
$this->recordsByLevel[$record->level->value][] = $record;
$this->records[] = $record;
}
}