logger.error() doesn't get logged into Sentry

[Natim]

logger.error() doesn't get logged into Sentry

[underyx]

Hey Remy, thanks for the report! I'm not sure how exactly to understand your request, however. What is logger in this context? The aiohttp-sentry package doesn't expose any sort of logger, as far as I can tell. Glad to help if we can clear this up!

[Natim]

Hello underyx,

I was refering to the Python standard library logging that you can use in your python code to log some things into sentry.

import logging

logger = logging.getLogger()

logger.error("Something went wrong when calling https://api.service.tld/url")

It seems that aiohttp-sentry is handling exceptions that make the app crash, but if you want to notify sentry without crashing (using logger.error for instance) it doesn't log it.

[underyx]

Ah, I see. The aiohttp-sentry integration's responsibility is meant to be just to provide reporting of errors from within aiohttp request handlers. The feature you're asking for seems to be part of the official sentry SDK, so I'd recommend that you configure it there instead. Does this make sense as a solution?

[underyx]

I do see how this could've been confusing though, so I tried to rephrase the package description to make its responsibility more clear: 205fcc4

Thanks for pointing it out!

[Natim]

Actually the way I expected it to work was that it was using aiohttp to call sentry asynchronously within the ioloop, handling errors that might occured within the aiohttp based service.

[underyx]

Ah, gotcha! In that case, and if you're still using the legacy raven Sentry client, you're looking for this project instead: https://github.com/getsentry/raven-aiohttp — this lets you configure the client to report via requests made with aiohttp.

[Natim]

Let me take an example:

from aiohttp_sentry import SentryMiddleware
from aiohttp import web

import logging

logger = logging.getLogger()

routes = web.RouteTableDef()

@routes.get('/')
async def hello(request):
    try:
        with open("file") as f:
             content = f.read()
    except FileNotFoundError:
        logger.exception("Unable to open file")  # Logs into sentry
        return web.Response(text="File not found", status=503)
    return web.Response(text=f"Hello, world: {content}")


@routes.get('/another-example')
async def example(request):
    try:
        with open("file") as f:
             content = f.read()
    except FileNotFoundError:
        raise
    return web.Response(text=f"Hello, world: {content}")

app = web.Application(middlewares=[SentryMiddleware()])
app.add_routes(routes)
web.run_app(app)

This are the two cases I had in mind.

If I understood correctly your are currently supporting only the second one, is that correct?

[underyx]

Correct. aiohttp-sentry handles only the second one.

The first case is supported by raven and sentry-sdk already, so there's no need for an extra package to provide that kind of reporting. The only consideration is that if you're using raven, you can configure its built-in error log reporting to use aiohttp by adding raven-aiohttp to it. I'm not sure how it works with sentry-sdk, but it should be something similar.

[Natim]

Can we take this lib to its next level and make it use aiohttp to communicate with sentry rather than any of the sentry sdk?

[underyx]

I don't think I clearly see yet how it would be beneficial to take over this responsibility from existing tools. Could you maybe provide an example of how the above code example would look if you had aiohttp-sentry take care of reporting about logger.exception("Unable to open file")? That should probably help me understand better. Sorry about making you jump through all these hoops, though! I just really don't see how this would help, yet.

[Natim]

I guess at this point I have almost enought to make a PR 😂

from raven.handlers.logging import SentryHandler
from raven.conf import setup_logging
[...]

@web.middleware
class SentryMiddleware:
    def __init__(self, sentry_kwargs=None, *, install_excepthook=True, loop=None):
        if sentry_kwargs is None:
            sentry_kwargs = {}

        sentry_kwargs = {
            "transport": raven_aiohttp.AioHttpTransport,
            "enable_breadcrumbs": False,
            # by default, do not let raven.Client install its own excepthook
            "install_sys_hook": not install_excepthook,
            **sentry_kwargs,
        }
        self.client = raven.Client(**sentry_kwargs)

        handler = SentryHandler(client=self.client)
        setup_logging(handler)

        if install_excepthook:
            self.update_excepthook(loop)

[underyx]

Ah, okay, now I see! Sorry I took so long to understand 😅

It certainly seems like your suggestion would be valuable for users. But implicitly patching the logging behavior of anyone who uses the middleware sounds dangerous. Let's continue discussing in the PR you just opened!

[Natim]

I wouldn't say dangerous but I understand that you might want to be able to deactivate it.

[underyx]

@Natim by the way, you opened this issue less than a week before my on-site job interview at Datadog in Paris. Could I ask if there was any connection between the two events? :D