newrelic / newrelic-python-agent

New Relic Python Agent
https://docs.newrelic.com/docs/agents/python-agent
Apache License 2.0
176 stars 99 forks source link

NewRelicContextFormatter update to support adding a stack trace #1168

Closed fritzdj closed 2 months ago

fritzdj commented 3 months ago

Overview

Currently the NewRelicContextFormatter implementation doesn't support including the stack trace. It would be convenient if there was an option because it would allow stack frames to be included as an attribute for an log in NR (instead of them being broken up by frame, which is the default if structured logs are not used). This PR is adding opt-in support for including stack traces.

Related Issue

https://github.com/newrelic/newrelic-python-agent/issues/1169

CLAassistant commented 3 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

:white_check_mark: TimPansino
:white_check_mark: fritzdj
:x: mergify[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

TimPansino commented 3 months ago

I've several pushed commits for both @fritzdj and our team to review. Here's the summary of my changes.

I think this will achieve the most flexible compatibility possible with existing customer uses, while making this feature possible. It's worth noting that in all cases the default behavior will remain unchanged, where no stack traces are attached.

fritzdj commented 3 months ago

Thanks @TimPansino, these changes look good to me! Good catches on the breaking change to the public API and Python 2 compatibility issues.

rafaelclp commented 2 months ago

We extend the NewRelicContextFormatter to add more data to the log record dict:

class OurNewRelicContextFormatter(NewRelicContextFormatter):
    @classmethod
    def log_record_to_dict(cls, record) -> dict[str, Any]:
        log_dict = super().log_record_to_dict(record)
        # ... add more stuff to log_dict here ...
        return log_dict

When we updated to newrelic==9.12, we started getting errors because log_record_to_dict "takes 2 positional arguments but 3 were given".

We already fixed this from our side, by taking in *args, **kwargs and redirecting them to the super call, but wondering if we are the only ones affected...