"extra" parameters should be unpacked on MakeRecord and not nested

[jelther]

First of all, congratulations for this library. It's awesome!

So, I was trying to use loguru with Opencensus Azure Log Handlers to send logs to Application Insights Service.

OpenCensus Example Usage

On this export, the documentation is very clear stating that passing parameters through extra will make they be pushed as custom dimensions to the service.

Unfortunately, when passing these arguments with loguru, I saw that the extra arguments are not unpacked as they should be:

loguru/loguru/_simple_sinks.py

Line 56 in 0940f92

{"extra": record["extra"]},

At this line, the "extra" for dict "record" is referenced as a dict of dicts so when we call logging.getLogger().makeRecord() the extra is not unpacked correctly.

As you can see on logging original makeRecord() (here) the extra parameter should be a dict and not a dict of dicts.

Of course, a customSink will make this feasible but it should be nice that loguru could reproduce the same scenarios as the original logging library.

[Delgan]

Hi! Thank you for the nice words!

Your suggestion seems to make a lot of sense. In fact, it was implemented like that some time ago. The extra dict was directly assigned to the extra attribute of the record. Easy.

Unfortunately, not everything is that simple. The problem is that the standard logging prohibits the use of certain values and raise an exception if they are present in the extra dict.

This has caused troubles to users on several occasions. For example, it was not possible to call logger.bind(name="foobar").info("Test") while using a standard logging Handler. Indeed, name would otherwise overwrite the corresponding standard Record attribute and Python would complain about that.

This is problematic because name is commonly used to create bound logger. For this reason, it was decided in #271 to no longer set extra=extra but rather extra={"extra": extra}. It's a bit convoluted, but it's the only solution found to avoid unfortunate error messages.

The only solution I see for your use case is to subclass the Handler and perform the appropriate transformation before re-sending it to the AzureLogHandler. 😕

class AzureProxyHandler(logging.Handler):
    def __init__(self):
        super().__init__()
        self._handler = AzureLogHandler()

    def emit(self, record):
        record.custom_dimensions = record.extra.get("custom_dimensions", None)
        self._handler.emit(record)

logger.add(AzureProxyHandler())

[jelther]

@Delgan , appreciate your response.
Just after I submitted this issue I look at the previous commits and realized that it was the way I described before.

So, totally got your point for making this modification to bypass the python limitation. I just wonder if this will impact on other handlers as well.

I'll try to subclass the handler the way you mentioned.

I was going through docs and what would be your suggestion if I were to use a subclass for the standard sink ? Is that the way it should be used ?

Thanks!

[jelther]

Just as a note, the custom_dimensions should be a property of LogRecord and not a key value of extra dictionary:

def emit(self, record):
		record.custom_dimensions = record.extra.get("custom_dimensions", None)
		self._handler.emit(record)

This worked as expected.

[Delgan]

So, totally got your point for making this modification to bypass the python limitation. I just wonder if this will impact on other handlers as well.

That's what I was wondering too. But I don't think there are many libraries that depend on the configuration this attribute.

I was going through docs and what would be your suggestion if I were to use a subclass for the standard sink ? Is that the way it should be used ?

Which "standard sink" are you referring to? Loguru does not expose a class that can be inherited. Still you can subclass logging.Handler or AzureLogHandler and pass it to logger.add(). As you realized, it will work fine. Another possibility is to create a custom class with a write() method.

[jelther]

@Delgan ,got it. I thought I could subclass the sink classes but I'm afraid I got it wrong.

Thanks for your suggestion.it worked very well.

I'll close this issue.

[Lewistrick]

I was trying to implement the code above but it still didn't write the properties in extra to Azure Application Insights. This seems to be because the extra attribute of record has an dictionary inside it also called extra. Changing the emit method to the following did work for me:

    def emit(self, record):
        extra = record.extra["extra"]
        record.custom_dimensions = extra.get("custom_dimensions", None)
        self._handler.emit(record)

Logging a message is done like this (so I'm not creating another extra key inside the extra argument):

logger.info(
    "Hello, world!",
    extra={
        "custom_dimensions": {
            "key_1": "value_1",
            "key_2": "value_2",
        },
    },
)

I hope it's OK to post this just as an FYI because this was the only concrete info I could find on combining loguru and App Insights, and maybe I'll help others (or future me) with this.