-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Remove monotonic count from ignored types in no duplicate assertion #9463
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the investigation! This solution is the best option in my opinion. We should use this assert_no_duplicate_metrics
method in more integration though
@@ -75,8 +75,6 @@ def test_assert_no_duplicate_message(aggregator): | |||
[ | |||
dict(type='count', name='metric.count', value=1, tags=['aa'], hostname='1'), | |||
dict(type='count', name='metric.count', value=1, tags=['aa'], hostname='1'), | |||
dict(type='monotonic_count', name='metric.monotonic_count', value=1, tags=['aa'], hostname='1'), | |||
dict(type='monotonic_count', name='metric.monotonic_count', value=1, tags=['aa'], hostname='1'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a rate
case in a duplicate metric
section above
@@ -418,7 +418,7 @@ def assert_no_duplicate_metrics(self): | |||
- hostname | |||
""" | |||
# metric types that intended to be called multiple times are ignored | |||
ignored_types = [self.COUNT, self.MONOTONIC_COUNT, self.COUNTER] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double submission of a monotonic_count
is allowed and has a defined behaviour, however it's easily a source of error. While there is no integration submitting a monotonic_count twice on purpose, let's raise an error on these cases.
…9463) * removed monotonic count from ignored types in no-duplicate assertion * added test cases for duplicate rate/monotonic count metric
What does this PR do?
Motivation
AI-1553
Additional Notes
Ideas explored to fail aggregator when this occurs:
assert_metric
in tests:check
method is called twice in integration tests- maybe this can be asserted at the check level in tests throughassert_no_duplicate_*
?dd_agent_check
fixture populates aggregator with json check output inreplay_check_run
, which would result in one rate value despite duplicates (when called withrate
)Review checklist (to be filled by reviewers)
changelog/
andintegration/
labels attached