Improved the Raven handler when using a Buffer handler

[fabpot]

The Raven handler should behave a lot like a mail handler when used in a Buffer handler. This PR addresses this use case.

[stof]

why setting a second formatter ? If someone wants to provide a different formatting for batched messages, it is already possible as your batch method uses formatBatch

[fabpot]

How?

[stof]

a formatter has 2 methods: format and formatBatch. If you want a special logic when formatting batch (in this case, using a different line format), you can provide a different implementation of the formatter. So IMO, your batch handling should use the formatter set in the handler instead of a second one set as logFormatter.

[fabpot]

So, you mean that it would be better to create a special formatter for Raven extending the line formatter but with a different pattern for format and formatBatch?

[stof]

if we really need a different format for the single and batch formattings, I would say yes

[fabpot]

We definitely want a different format for the single and the batch formatting. For the former, we only want a minimal amount of information as it is used as the title of the report, but for the logs, we want as much information as possible. And the line formatter fits well for both purposes.

Creating another formatter just for that use case seems wrong to me.

So, I think what I propose is actually better. ping @Seldaek

[Seldaek]

I'm not really against having a second formatter if there are two formats needed, but I am not sure that picking the last record as the main one is right. This sounds very dependent on the exact use case and not very generic. I am especially worried about using the level of the last record to determine whether to log or not. In theory all records should be filtered according to their level first, and then maybe you could take the last one as the one for the "subject" line.

[pvanliefland]

I also wanted a nice buffered logging for Sentry so I tried @fabpot's version.

@Seldaek you're right : for me, picking the last record does not work, as the last record is a debug entry (kernel.terminate). Filtering all the records based on their level seems to work.

As for the subject line, I ended up taking the record with the highest severity, as it is usually the problem I want to log.

Apart from that, Fabien's version works like a charm.

Here is my slightly modified writeBatch method:

public function handleBatch(array $records)
{
    $level = $this->level;

    $records = array_filter($records, function($record) use($level) {
        return $record['level'] >= $level;
    });

    $record = array_reduce($records, function($highest, $record) {
        if($record['level'] >= $highest['level']) {
            $highest = $record;

            return $highest;
        }
    });

    // the other ones are added as a context item
    $logs = array();
    foreach ($records as $r) {
        $logs[] = $this->processRecord($r);
    }

    if ($logs) {
        $record['context']['logs'] = (string) $this->getLogFormatter()->formatBatch($logs);
    }

    $this->handle($record);
}

[Seldaek]

@pvanliefland can you send a pull request against @fabpot's repo to update this pull request? Or @fabpot just copies the changes? Or someone says if they disagree, but it seems like a better way to me?

[pvanliefland]

Done. Tried this approach with 2 different projects, looks ok to me.

[fabpot]

👍

[Seldaek]

Alright, merged. Just FYI I renamed the *LogFormatter stuff to *BatchFormatter.