Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix OTLP Count Metric Serialization #856

Merged
merged 6 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions newrelic/core/otlp_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@


def otlp_encode(payload):
if type(payload) is dict:
if type(payload) is dict: # pylint: disable=C0123
_logger.warning(
"Using OTLP integration while protobuf is not installed. This may result in larger payload sizes and data loss."
)
Expand Down Expand Up @@ -162,7 +162,9 @@ def stats_to_otlp_metrics(metric_data, start_time, end_time):
separate the types and report multiple metrics, one for each type.
"""
for name, metric_container in metric_data:
if any(isinstance(metric, CountStats) for metric in metric_container.values()):
# Types are checked here using type() instead of isinstance, as CountStats is a subclass of TimeStats.
# Imporperly checking with isinstance will lead to count metrics being encoded and reported twice.
if any(type(metric) is CountStats for metric in metric_container.values()): # pylint: disable=C0123
# Metric contains Sum metric data points.
yield Metric(
name=name,
Expand All @@ -177,11 +179,11 @@ def stats_to_otlp_metrics(metric_data, start_time, end_time):
attributes=create_key_values_from_iterable(tags),
)
for tags, value in metric_container.items()
if isinstance(value, CountStats)
if type(value) is CountStats # pylint: disable=C0123
],
),
)
if any(isinstance(metric, TimeStats) for metric in metric_container.values()):
if any(type(metric) is TimeStats for metric in metric_container.values()): # pylint: disable=C0123
# Metric contains Summary metric data points.
yield Metric(
name=name,
Expand All @@ -194,7 +196,7 @@ def stats_to_otlp_metrics(metric_data, start_time, end_time):
attributes=create_key_values_from_iterable(tags),
)
for tags, value in metric_container.items()
if isinstance(value, TimeStats)
if type(value) is TimeStats # pylint: disable=C0123
]
),
)
Expand Down
35 changes: 30 additions & 5 deletions tests/agent_features/test_dimensional_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,17 @@
validate_transaction_metrics,
)

import newrelic.core.otlp_utils
from newrelic.api.application import application_instance
from newrelic.api.background_task import background_task
from newrelic.api.transaction import (
record_dimensional_metric,
record_dimensional_metrics,
)
from newrelic.common.metric_utils import create_metric_identity

import newrelic.core.otlp_utils
from newrelic.core.config import global_settings
from newrelic.packages import six


try:
# python 2.x
reload
Expand All @@ -55,7 +53,7 @@ def otlp_content_encoding(request):
_settings.debug.otlp_content_encoding = request.param
reload(newrelic.core.otlp_utils)
assert newrelic.core.otlp_utils.otlp_content_setting == request.param, "Content encoding mismatch."

yield

_settings.debug.otlp_content_encoding = prev
Expand Down Expand Up @@ -177,7 +175,7 @@ def _test():
("Metric.NotPresent", None, None),
],
)
def test_dimensional_metric_payload():
def test_dimensional_metrics_payload():
@background_task(name="test_dimensional_metric_payload")
def _test():
record_dimensional_metrics(
Expand All @@ -197,3 +195,30 @@ def _test():
app = application_instance()
core_app = app._agent.application(app.name)
core_app.harvest()


@reset_core_stats_engine()
@validate_dimensional_metric_payload(
summary_metrics=[
("Metric.Summary", None, 1),
("Metric.Count", None, None), # Should NOT be present
],
count_metrics=[
("Metric.Count", None, 1),
("Metric.Summary", None, None), # Should NOT be present
],
)
def test_dimensional_metrics_no_duplicate_encodings():
@background_task(name="test_dimensional_metric_payload")
def _test():
record_dimensional_metrics(
[
("Metric.Summary", 1),
("Metric.Count", {"count": 1}),
]
)

_test()
app = application_instance()
core_app = app._agent.application(app.name)
core_app.harvest()
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def _bind_params(method, payload=(), *args, **kwargs):
if not count:
if metric in sent_summary_metrics:
data_points = data_points_to_dict(sent_summary_metrics[metric]["summary"]["data_points"])
assert tags not in data_points, "(%s, %s) Found." % (metric, tags and dict(tags))
assert tags not in data_points, "(%s, %s) Unexpected but found." % (metric, tags and dict(tags))
else:
assert metric in sent_summary_metrics, "%s Not Found. Got: %s" % (
metric,
Expand Down Expand Up @@ -153,7 +153,7 @@ def _bind_params(method, payload=(), *args, **kwargs):
if not count:
if metric in sent_count_metrics:
data_points = data_points_to_dict(sent_count_metrics[metric]["sum"]["data_points"])
assert tags not in data_points, "(%s, %s) Found." % (metric, tags and dict(tags))
assert tags not in data_points, "(%s, %s) Unexpected but found." % (metric, tags and dict(tags))
else:
assert metric in sent_count_metrics, "%s Not Found. Got: %s" % (
metric,
Expand Down