newrelic-python-agent
newrelic-python-agent copied to clipboard
NewRelicContextFormatter update to support adding a stack trace
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
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.
I've several pushed commits for both @fritzdj and our team to review. Here's the summary of my changes.
- Python 2 issues in the code and tests fixed with transparent changes.
- The public APIs for
log_record_to_dict()andformat_exc_infowere originally changed in a backwards incompatible manner.- These have been changed to be class methods of the
NewRelicContextFormatterto maintain compatibility, and exported as top level names in thenewrelic.api.logmodule for the same reason. - As part of this change,
stack_trace_limitwas added as a function argument to both of these APIs and will be supplied automatically by calls toformat()if using the Formatter as an object, or can be supplied otherwise.
- These have been changed to be class methods of the
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.
Thanks @TimPansino, these changes look good to me! Good catches on the breaking change to the public API and Python 2 compatibility issues.
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...