-
Notifications
You must be signed in to change notification settings - Fork 2.8k
ci: Exclude HTTP 5xx metrics from comparisons #7671
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -26,6 +26,41 @@ | |||||||
| # } | ||||||||
| } | ||||||||
|
|
||||||||
| METRIC_EXCLUSION_RULES = { | ||||||||
| # excluding HTTP 5xx responses as these can be flaky | ||||||||
| 'http_5xx_errors': { | ||||||||
| 'condition': 'label_match', | ||||||||
| 'label': 'http_response_status_code', | ||||||||
| 'pattern': r'^5\d{2}$', | ||||||||
| }, | ||||||||
|
|
||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
| def should_exclude_metric(metric_name, labels): | ||||||||
| """ | ||||||||
| Determines if a metric should be excluded from comparison based on configured rules. | ||||||||
|
|
||||||||
| Args: | ||||||||
| metric_name: The name of the metric | ||||||||
| labels: Dictionary of labels for the metric | ||||||||
|
|
||||||||
| Returns: | ||||||||
| tuple: (should_exclude: bool, reason: str or None) | ||||||||
| """ | ||||||||
| for rule_name, rule_config in METRIC_EXCLUSION_RULES.items(): | ||||||||
| condition = rule_config['condition'] | ||||||||
|
|
||||||||
| if condition == 'label_match': | ||||||||
| label = rule_config['label'] | ||||||||
| pattern = rule_config['pattern'] | ||||||||
| if label in labels and re.match(pattern, labels[label]): | ||||||||
| return True | ||||||||
|
|
||||||||
|
|
||||||||
| return False | ||||||||
|
|
||||||||
|
|
||||||||
| def suppress_transient_labels(metric_name, labels): | ||||||||
| """ | ||||||||
| Suppresses transient labels in metrics based on configured patterns. | ||||||||
|
|
@@ -58,9 +93,12 @@ def parse_metrics(content): | |||||||
| for family in text_string_to_metric_families(content): | ||||||||
| for sample in family.samples: | ||||||||
| labels = dict(sample.labels) | ||||||||
| #simply pop undesirable metric labels | ||||||||
| labels.pop('service_instance_id',None) | ||||||||
|
|
||||||||
| should_exclude= should_exclude_metric(sample.name, labels) | ||||||||
| if should_exclude: | ||||||||
|
||||||||
| should_exclude= should_exclude_metric(sample.name, labels) | |
| if should_exclude: | |
| if should_exclude_metric(sample.name, labels): |
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.
it would be good to count these exclusions in stats
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.
it would be good to count these exclusions in stats
Like the ones posted in PR, if some stats are excluded?
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.
yes (although I am not sure excluding full metrics is a good thing, per the other comment).
Uh oh!
There was an error while loading. Please reload this page.