Skip to content

Commit

Permalink
Fix Trace Finalizer Crashes (#652)
Browse files Browse the repository at this point in the history
* Patch crashes in various traces with None settings

* Add tests for graphql trace types to unittests

* Add test to ensure traces don't crash in finalizer

* [Mega-Linter] Apply linters fixes

* Bump tests

Co-authored-by: TimPansino <TimPansino@users.noreply.github.com>
Co-authored-by: Lalleh Rafeei <84813886+lrafeei@users.noreply.github.com>
  • Loading branch information
3 people authored Oct 10, 2022
1 parent 9a56f3a commit 65246e7
Show file tree
Hide file tree
Showing 6 changed files with 548 additions and 438 deletions.
5 changes: 3 additions & 2 deletions newrelic/api/database_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ def _log_async_warning(self):

def finalize_data(self, transaction, exc=None, value=None, tb=None):
self.stack_trace = None
self.sql_format = "off"

connect_params = None
cursor_params = None
Expand Down Expand Up @@ -206,8 +207,8 @@ def finalize_data(self, transaction, exc=None, value=None, tb=None):
transaction._explain_plan_count += 1

self.sql_format = (
tt.record_sql if tt.record_sql else ""
) # If tt.record_sql is None, then the empty string will default to sql being obfuscated
tt.record_sql if tt.record_sql else "off"
) # If tt.record_sql is None, then default to sql being off
self.connect_params = connect_params
self.cursor_params = cursor_params
self.sql_parameters = sql_parameters
Expand Down
7 changes: 6 additions & 1 deletion newrelic/api/graphql_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,13 @@ def finalize_data(self, transaction, exc=None, value=None, tb=None):
self._add_agent_attribute("graphql.operation.type", self.operation_type)
self._add_agent_attribute("graphql.operation.name", self.operation_name)

settings = transaction.settings
if settings and settings.agent_limits and settings.agent_limits.sql_query_length_maximum:
limit = transaction.settings.agent_limits.sql_query_length_maximum
else:
limit = 0

# Attach formatted graphql
limit = transaction.settings.agent_limits.sql_query_length_maximum
self.graphql = graphql = self.formatted[:limit]
self._add_agent_attribute("graphql.operation.query", graphql)

Expand Down
36 changes: 24 additions & 12 deletions newrelic/api/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@
import weakref
from collections import OrderedDict

from newrelic.api.application import application_instance
import newrelic.core.database_node
import newrelic.core.error_node
from newrelic.core.log_event_node import LogEventNode
import newrelic.core.root_node
import newrelic.core.transaction_node
import newrelic.packages.six as six
from newrelic.api.application import application_instance
from newrelic.api.time_trace import TimeTrace, get_linking_metadata
from newrelic.common.encoding_utils import (
DistributedTracePayload,
Expand Down Expand Up @@ -63,6 +62,7 @@
)
from newrelic.core.config import DEFAULT_RESERVOIR_SIZE, LOG_EVENT_RESERVOIR_SIZE
from newrelic.core.custom_event import create_custom_event
from newrelic.core.log_event_node import LogEventNode
from newrelic.core.stack_trace import exception_stack
from newrelic.core.stats_engine import CustomMetrics, SampledDataSet
from newrelic.core.thread_utilization import utilization_tracker
Expand Down Expand Up @@ -324,8 +324,12 @@ def __init__(self, application, enabled=None, source=None):
self.enabled = True

if self._settings:
self._custom_events = SampledDataSet(capacity=self._settings.event_harvest_config.harvest_limits.custom_event_data)
self._log_events = SampledDataSet(capacity=self._settings.event_harvest_config.harvest_limits.log_event_data)
self._custom_events = SampledDataSet(
capacity=self._settings.event_harvest_config.harvest_limits.custom_event_data
)
self._log_events = SampledDataSet(
capacity=self._settings.event_harvest_config.harvest_limits.log_event_data
)
else:
self._custom_events = SampledDataSet(capacity=DEFAULT_RESERVOIR_SIZE)
self._log_events = SampledDataSet(capacity=LOG_EVENT_RESERVOIR_SIZE)
Expand Down Expand Up @@ -1473,31 +1477,35 @@ def set_transaction_name(self, name, group=None, priority=None):
self._group = group
self._name = name


def record_log_event(self, message, level=None, timestamp=None, priority=None):
settings = self.settings
if not (settings and settings.application_logging and settings.application_logging.enabled and settings.application_logging.forwarding and settings.application_logging.forwarding.enabled):
if not (
settings
and settings.application_logging
and settings.application_logging.enabled
and settings.application_logging.forwarding
and settings.application_logging.forwarding.enabled
):
return

timestamp = timestamp if timestamp is not None else time.time()
level = str(level) if level is not None else "UNKNOWN"

if not message or message.isspace():
_logger.debug("record_log_event called where message was missing. No log event will be sent.")
return

message = truncate(message, MAX_LOG_MESSAGE_LENGTH)

event = LogEventNode(
timestamp=timestamp,
level=level,
message=message,
attributes=get_linking_metadata(),
attributes=get_linking_metadata(),
)

self._log_events.add(event, priority=priority)


def record_exception(self, exc=None, value=None, tb=None, params=None, ignore_errors=None):
# Deprecation Warning
warnings.warn(
Expand Down Expand Up @@ -1603,6 +1611,8 @@ def _process_node(self, node):

if type(node) is newrelic.core.database_node.DatabaseNode:
settings = self._settings
if not settings:
return
if not settings.collect_traces:
return
if not settings.slow_sql.enabled and not settings.transaction_tracer.explain_enabled:
Expand Down Expand Up @@ -1869,7 +1879,9 @@ def record_log_event(message, level=None, timestamp=None, application=None, prio
"record_log_event has been called but no transaction or application was running. As a result, "
"the following event has not been recorded. message: %r level: %r timestamp %r. To correct "
"this problem, supply an application object as a parameter to this record_log_event call.",
message, level, timestamp,
message,
level,
timestamp,
)
elif application.enabled:
application.record_log_event(message, level, timestamp, priority=priority)
Expand Down
Loading

0 comments on commit 65246e7

Please sign in to comment.