context vs extras arguments

[colinscape]

Heya @erickhun !

Thanks for putting this together! I had a few thoughts on this that I wasn't sure how best to share, so thought I'd try out a GH issue! 😀

First up - all the 'logging' methods that are exposed have a context as well as a extras parameter. None of the examples show use of the extras parameter though, which seems to suggest that they are maybe from more 'internal' use. Is that right?

If it is right, I think we should remove the extras parameter from the exposed methods, to keep things simple.

Also, I see that the Datadog trace and span ids are being inserted into the context[dd] parameters. Since this is the parameter passed by the caller, this feels a bit odd, since there is the (small) potential for these to clash. It would make more sense from my side that the Datadog parameters get added into the extras parameter. What do you think?

I don't know how the context and extras are treated differently by Monolog, so this might not be the most appropriate thing to do. However, I do think we should have a clear way for this to distinguish between the caller's parameters and the more 'built-in' parameters.

[erickhun]

yeah @colinscape , @esclapes also bring up the exact same issue for the extra field . I've added the extra field after reading that documentation : https://docs.datadoghq.com/logs/log_collection/php/?tab=phpmonolog#adding-more-context

As I understand That'd be some "extra" information that would not make sense to put in acontext .

// Add various generic context
$record['extra']['key'] = 'value';

I've tried to come up with examples but can't 🤷‍♂️ So yeah we should certainly drop it if that doesn't make sense

[colinscape]

It looks from the implementation that everything gets put together anyway: https://github.com/bufferapp/php-bufflog/blob/master/src/php-bufflog/BuffLog.php#L94-L102

        $output = [
            "message"   => $message,
            "level"     => $level,
            "datetime"  => date(\DateTime::ATOM),
            // we could use timestamp if we need ms precision (but it isn't readable) https://docs.datadoghq.com/logs/processing/#reserved-attributes
            // 'timestamp' => round(microtime(true) * 1000),
            "context"   => $context,
            "extra"     => $extra
        ];

From that, it doesn't look like there's any great difference - it's mainly the 'scope' of the information - I guess any parameters passed in the call will be more specific to the situation at hand (eg result of a function call), whereas the extra information would be the other stuff that might be useful for later analysis (eg, user id, trace id, span id, etc) and which would likely be shared across many of the logging calls from this service in this instance.

[erickhun]

[erickhun]

done with #6

[esclapes]

I think that was a good call @erickhun, just for reference, I found this in monolog docs:

At first glance context and extra look very similar, and they are in the sense that they both carry arbitrary data that is related to the log message somehow. The main difference is that context can be supplied in user land (it is the 3rd parameter to Logger::addRecord()) whereas extra is internal only and can be filled by processors. The reason processors write to extra and not to context is to prevent overriding any user provided data in context.

It may be monolog specific, but it may be useful to keep it in mind if we consider move our convention to a monolog formatter

[erickhun]

yes @esclapes great find.

I was reading how Monolog enrich information, and it use processors, that use by default the extra fields : some examples here: https://github.com/Seldaek/monolog/tree/master/src/Monolog/Processor

I'm wondering if the information we're trying to add are only users related, so only using context 🤔