elastic / apm-agent-python

https://www.elastic.co/guide/en/apm/agent/python/current/index.html
BSD 3-Clause "New" or "Revised" License
412 stars 219 forks source link

Logging integration to capture all logged exceptions #1040

Open stephan-erb-by opened 3 years ago

stephan-erb-by commented 3 years ago

Is your feature request related to a problem? Please describe. Instrumenting an existing application can be cumbersome: All places with exception handling need to be extend in order to also forward the errors to APM using client.capture_exception(). However, most of the time such code paths are already well covered with logger.exception(...) or similar.

Describe the solution you'd like The APM library should offer an opt-in feature to automatically forward all logged exceptions to APM. This could be an automatic feature similar to how HTTP libraries get instrumented.

Describe alternatives you've considered The APM logging.LoggingHandler class offers such functionality. However the capturing is hereby bound to a specific output format. It cannot easily be used together with structlog

This feature request is inspired by the integration offered by Sentry https://docs.sentry.io/platforms/python/guides/logging/.

Additional context n/a

basepi commented 3 years ago

Thanks for the request! Ref https://github.com/elastic/apm-agent-python/issues/942

process0 commented 3 years ago

@basepi I may be wrong, but I dont think this feature request is about the APM client shipping logs. I think this request is to integrate with the logging library to call client.capture_exception when an exception is logged.

process0 commented 3 years ago

I use structlog and elasticapm to instrument celery. Heres a quick way to achieve what i think was described above using a structlog processor:

# Forward all Exceptions outside transactions
def check_exec(logger, name, event_dict):
    from elasticapm.traces import execution_context
    from elasticapm import get_client
    apm_client = get_client()
    transaction = execution_context.get_transaction()

    # Assume if we have a transaction then we are capturing exceptions
    # by other means
    exc_type, exc_value, exc_traceback = sys.exc_info()
    if apm_client and not transaction and exc_type is not None:
        apm_client.capture_exception(
            handled=False
        )
    return event_dict

This could easily be put into the elasticapm.handlers.structlog.structlog_processor

basepi commented 3 years ago

Theoretically we're already forwarding unhandled exceptions via our framework integrations. Plus you can use our excepthook to make the capturing behavior even more broad.

But perhaps it's worth adding another place in structlog as in your example. Thanks for the input, it's really valuable!

process0 commented 3 years ago

Theoretically we're already forwarding unhandled exceptions via our framework integrations. Plus you can use our excepthook to make the capturing behavior even more broad.

For celery integration, if an exception happens outside a task, then it isn't forwarded. I didn't see excepthook. I'll try it out.