Skip to content

Commit

Permalink
fix positive and negative matches on labels (netdata#17290)
Browse files Browse the repository at this point in the history 
* fix positive and negative matches

* fix typo

* fix unit tests

* unittest for rrdlabels pattern lists
  • Loading branch information
@ktsaou
ktsaou authored Mar 29, 2024
1 parent d39c204 commit 1293c2a
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 35 deletions.
8 changes: 4 additions & 4 deletions src/database/contexts/api_v1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,13 @@ static inline int rrdinstance_to_json_callback(const DICTIONARY_ITEM *item, void
if(before && (!ri->first_time_s || before < ri->first_time_s))
return 0;

if(t_parent->chart_label_key && !rrdlabels_match_simple_pattern_parsed(ri->rrdlabels, t_parent->chart_label_key,
'\0', NULL))
if(t_parent->chart_label_key && rrdlabels_match_simple_pattern_parsed(ri->rrdlabels, t_parent->chart_label_key,
'\0', NULL) != SP_MATCHED_POSITIVE)
return 0;

if(t_parent->chart_labels_filter && !rrdlabels_match_simple_pattern_parsed(ri->rrdlabels,
if(t_parent->chart_labels_filter && rrdlabels_match_simple_pattern_parsed(ri->rrdlabels,
t_parent->chart_labels_filter, ':',
NULL))
NULL) != SP_MATCHED_POSITIVE)
return 0;

time_t first_time_s = ri->first_time_s;
Expand Down
2 changes: 1 addition & 1 deletion src/database/contexts/api_v2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ static FTS_MATCH rrdcontext_to_json_v2_full_text_search(struct rrdcontext_to_jso

size_t label_searches = 0;
if(unlikely(ri->rrdlabels && rrdlabels_entries(ri->rrdlabels) &&
rrdlabels_match_simple_pattern_parsed(ri->rrdlabels, q, ':', &label_searches))) {
rrdlabels_match_simple_pattern_parsed(ri->rrdlabels, q, ':', &label_searches) == SP_MATCHED_POSITIVE)) {
ctl->q.fts.searches += label_searches;
ctl->q.fts.char_searches += label_searches;
matched = FTS_MATCHED_LABEL;
Expand Down
4 changes: 2 additions & 2 deletions src/database/contexts/query_target.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -732,12 +732,12 @@ static inline bool query_instance_matches_labels(
SIMPLE_PATTERN *labels_sp)
{

if (chart_label_key_sp && !rrdlabels_match_simple_pattern_parsed(ri->rrdlabels, chart_label_key_sp, '\0', NULL))
if (chart_label_key_sp && rrdlabels_match_simple_pattern_parsed(ri->rrdlabels, chart_label_key_sp, '\0', NULL) != SP_MATCHED_POSITIVE)
return false;

if (labels_sp) {
struct pattern_array *pa = pattern_array_add_simple_pattern(NULL, labels_sp, ':');
bool found = pattern_array_label_match(pa, ri->rrdlabels, ':', NULL, rrdlabels_match_simple_pattern_parsed);
bool found = pattern_array_label_match(pa, ri->rrdlabels, ':', NULL);
pattern_array_free(pa);
return found;
}
Expand Down
125 changes: 103 additions & 22 deletions src/database/rrdlabels.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ int rrdlabels_walkthrough_read(RRDLABELS *labels, int (*callback)(const char *na
lfe_start_read(labels, lb, ls)
{
ret = callback(string2str(lb->index.key), string2str(lb->index.value), ls, data);
if (ret < 0)
if (ret != 0)
break;
}
lfe_done(labels);
Expand Down Expand Up @@ -1125,9 +1125,12 @@ static int simple_pattern_match_name_only_callback(const char *name, const char

// we return -1 to stop the walkthrough on first match
t->searches++;
if(simple_pattern_matches(t->pattern, name)) return -1;
SIMPLE_PATTERN_RESULT rc = simple_pattern_matches_extract(t->pattern, name, NULL, 0);

return 0;
if(rc == SP_MATCHED_NEGATIVE)
return -1;

return rc == SP_MATCHED_POSITIVE;
}

static int simple_pattern_match_name_and_value_callback(const char *name, const char *value, RRDLABEL_SRC ls __maybe_unused, void *data) {
Expand All @@ -1154,13 +1157,15 @@ static int simple_pattern_match_name_and_value_callback(const char *name, const
*dst = '\0';

t->searches++;
if(simple_pattern_matches_length_extract(t->pattern, tmp, dst - tmp, NULL, 0) == SP_MATCHED_POSITIVE)
SIMPLE_PATTERN_RESULT rc = simple_pattern_matches_length_extract(t->pattern, tmp, dst - tmp, NULL, 0);

if(rc == SP_MATCHED_NEGATIVE)
return -1;

return 0;
return rc == SP_MATCHED_POSITIVE ? 1 : 0;
}

bool rrdlabels_match_simple_pattern_parsed(RRDLABELS *labels, SIMPLE_PATTERN *pattern, char equal, size_t *searches) {
SIMPLE_PATTERN_RESULT rrdlabels_match_simple_pattern_parsed(RRDLABELS *labels, SIMPLE_PATTERN *pattern, char equal, size_t *searches) {
if (!labels) return false;

struct simple_pattern_match_name_value t = {
Expand All @@ -1174,7 +1179,10 @@ bool rrdlabels_match_simple_pattern_parsed(RRDLABELS *labels, SIMPLE_PATTERN *pa
if(searches)
*searches = t.searches;

return (ret == -1)?true:false;
if(ret < 0)
return SP_MATCHED_NEGATIVE;

return (ret > 0)?SP_MATCHED_POSITIVE:SP_NOT_MATCHED;
}

bool rrdlabels_match_simple_pattern(RRDLABELS *labels, const char *simple_pattern_txt) {
Expand All @@ -1191,11 +1199,11 @@ bool rrdlabels_match_simple_pattern(RRDLABELS *labels, const char *simple_patter
}
}

bool ret = rrdlabels_match_simple_pattern_parsed(labels, pattern, equal, NULL);
SIMPLE_PATTERN_RESULT ret = rrdlabels_match_simple_pattern_parsed(labels, pattern, equal, NULL);

simple_pattern_free(pattern);

return ret;
return ret == SP_MATCHED_POSITIVE;
}


Expand Down Expand Up @@ -1388,8 +1396,7 @@ bool pattern_array_label_match(
struct pattern_array *pa,
RRDLABELS *labels,
char eq,
size_t *searches,
bool (*callback_function)(RRDLABELS *, SIMPLE_PATTERN *, char, size_t *))
size_t *searches)
{
if (!pa || !labels)
return true;
Expand All @@ -1398,14 +1405,23 @@ bool pattern_array_label_match(
Word_t Index = 0;
bool first_then_next = true;
while ((Pvalue = JudyLFirstThenNext(pa->JudyL, &Index, &first_then_next))) {
// for each label key in the patterns array

struct pattern_array_item *pai = *Pvalue;
bool match = false;
for (Word_t i = 1; !match && i <= pai->size; i++) {
SIMPLE_PATTERN_RESULT match = SP_NOT_MATCHED ;
for (Word_t i = 1; i <= pai->size; i++) {
// for each pattern in the label key pattern list

if (!(Pvalue = JudyLGet(pai->JudyL, i, PJE0)) || !*Pvalue)
continue;
match = callback_function(labels, (SIMPLE_PATTERN *)(*Pvalue), eq, searches);

match = rrdlabels_match_simple_pattern_parsed(labels, (SIMPLE_PATTERN *)(*Pvalue), eq, searches);

if(match != SP_NOT_MATCHED)
break;
}
if (!match)

if (match != SP_MATCHED_POSITIVE)
return false;
}
return true;
Expand Down Expand Up @@ -1481,6 +1497,7 @@ void pattern_array_free(struct pattern_array *pa)

string_freez((STRING *)Index);
(void) JudyLDel(&(pa->JudyL), Index, PJE0);
freez(pai);
Index = 0;
}
freez(pa);
Expand Down Expand Up @@ -1669,7 +1686,7 @@ static int rrdlabels_walkthrough_index_read(RRDLABELS *labels, int (*callback)(c
lfe_start_read(labels, lb, ls)
{
ret = callback(string2str(lb->index.key), string2str(lb->index.value), ls, index, data);
if (ret < 0)
if (ret != 0)
break;
index++;
}
Expand Down Expand Up @@ -1705,25 +1722,25 @@ static int rrdlabels_unittest_pattern_check()

bool match;
struct pattern_array *pa = pattern_array_add_key_value(NULL, "_module", "wrong_module", '=');
match = pattern_array_label_match(pa, labels, '=', NULL, rrdlabels_match_simple_pattern_parsed);
match = pattern_array_label_match(pa, labels, '=', NULL);
// This should not match: _module in ("wrong_module")
if (match)
rc++;

pattern_array_add_key_value(pa, "_module", "disk_detection", '=');
match = pattern_array_label_match(pa, labels, '=', NULL, rrdlabels_match_simple_pattern_parsed);
match = pattern_array_label_match(pa, labels, '=', NULL);
// This should match: _module in ("wrong_module","disk_detection")
if (!match)
rc++;

pattern_array_add_key_value(pa, "key1", "wrong_key1_value", '=');
match = pattern_array_label_match(pa, labels, '=', NULL, rrdlabels_match_simple_pattern_parsed);
match = pattern_array_label_match(pa, labels, '=', NULL);
// This should not match: _module in ("wrong_module","disk_detection") AND key1 in ("wrong_key1_value")
if (match)
rc++;

pattern_array_add_key_value(pa, "key1", "value1", '=');
match = pattern_array_label_match(pa, labels, '=', NULL, rrdlabels_match_simple_pattern_parsed);
match = pattern_array_label_match(pa, labels, '=', NULL);
// This should match: _module in ("wrong_module","disk_detection") AND key1 in ("wrong_key1_value", "value1")
if (!match)
rc++;
Expand All @@ -1734,13 +1751,13 @@ static int rrdlabels_unittest_pattern_check()
sp = simple_pattern_create("key3=*phant", SIMPLE_PATTERN_DEFAULT_WEB_SEPARATORS, SIMPLE_PATTERN_EXACT, true);
pattern_array_add_lblkey_with_sp(pa, "key3", sp);

match = pattern_array_label_match(pa, labels, '=', NULL, rrdlabels_match_simple_pattern_parsed);
match = pattern_array_label_match(pa, labels, '=', NULL);
// This should match: _module in ("wrong_module","disk_detection") AND key1 in ("wrong_key1_value", "value1") AND key2 in ("cat* !d*") AND key3 in ("*phant")
if (!match)
rc++;

rrdlabels_add(labels, "key3", "now_fail", RRDLABEL_SRC_CONFIG);
match = pattern_array_label_match(pa, labels, '=', NULL, rrdlabels_match_simple_pattern_parsed);
match = pattern_array_label_match(pa, labels, '=', NULL);
// This should not match: _module in ("wrong_module","disk_detection") AND key1 in ("wrong_key1_value", "value1") AND key2 in ("cat* !d*") AND key3 in ("*phant")
if (match)
rc++;
Expand Down Expand Up @@ -1832,6 +1849,69 @@ static int rrdlabels_unittest_migrate_check()
return rc;
}

struct pattern_array *trim_and_add_key_to_values(struct pattern_array *pa, const char *key, STRING *input);
static int rrdlabels_unittest_check_pattern_list(RRDLABELS *labels, const char *pattern, bool expected) {
fprintf(stderr, "rrdlabels_match_simple_pattern(labels, \"%s\") ... ", pattern);

STRING *str = string_strdupz(pattern);
struct pattern_array *pa = trim_and_add_key_to_values(NULL, NULL, str);

bool ret = pattern_array_label_match(pa, labels, '=', NULL);

fprintf(stderr, "%s, got %s expected %s\n", (ret == expected)?"OK":"FAILED", ret?"true":"false", expected?"true":"false");

string_freez(str);
pattern_array_free(pa);

return (ret == expected)?0:1;
}

static int rrdlabels_unittest_host_chart_labels() {
fprintf(stderr, "\n%s() tests\n", __FUNCTION__);

int errors = 0;

RRDLABELS *labels = rrdlabels_create();
rrdlabels_add(labels, "_hostname", "hostname1", RRDLABEL_SRC_CONFIG);
rrdlabels_add(labels, "_os", "linux", RRDLABEL_SRC_CONFIG);
rrdlabels_add(labels, "_distro", "ubuntu", RRDLABEL_SRC_CONFIG);

// match a single key
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=*", true);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=!*", false);

// conflicting keys (some positive, some negative)
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=* _os=!*", false);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=!* _os=*", false);

// the user uses a key that is not there
errors += rrdlabels_unittest_check_pattern_list(labels, "_not_a_key=*", false);
errors += rrdlabels_unittest_check_pattern_list(labels, "_not_a_key=!*", false);
errors += rrdlabels_unittest_check_pattern_list(labels, "_not_a_key=* _hostname=* _os=*", false);
errors += rrdlabels_unittest_check_pattern_list(labels, "_not_a_key=!* _hostname=* _os=*", false);

// positive and negative matches on the same key
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=!*invalid* !*bad* *name*", true);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=*name* !*invalid* !*bad*", true);

// positive and negative matches on the same key with catch all
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=!*invalid* !*bad* *", true);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=* !*invalid* !*bad*", true);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=!*invalid* !*name* *", false);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=* !*invalid* !*name*", true);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=*name* !*", true);

errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=!*name* _os=l*", false);
errors += rrdlabels_unittest_check_pattern_list(labels, "_os=l* hostname=!*name*", false);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=*name* _hostname=*", true);
errors += rrdlabels_unittest_check_pattern_list(labels, "_hostname=*name* _os=l*", true);
errors += rrdlabels_unittest_check_pattern_list(labels, "_os=l* _hostname=*name*", true);

rrdlabels_destroy(labels);

return errors;
}

static int rrdlabels_unittest_check_simple_pattern(RRDLABELS *labels, const char *pattern, bool expected) {
fprintf(stderr, "rrdlabels_match_simple_pattern(labels, \"%s\") ... ", pattern);

Expand Down Expand Up @@ -1924,6 +2004,7 @@ int rrdlabels_unittest(void) {
errors += rrdlabels_unittest_sanitization();
errors += rrdlabels_unittest_add_pairs();
errors += rrdlabels_unittest_simple_pattern();
errors += rrdlabels_unittest_host_chart_labels();
errors += rrdlabels_unittest_double_check();
errors += rrdlabels_unittest_migrate_check();
errors += rrdlabels_unittest_pattern_check();
Expand Down
5 changes: 2 additions & 3 deletions src/database/rrdlabels.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ int rrdlabels_walkthrough_read(RRDLABELS *labels, int (*callback)(const char *na
void rrdlabels_log_to_buffer(RRDLABELS *labels, BUFFER *wb);
bool rrdlabels_match_simple_pattern(RRDLABELS *labels, const char *simple_pattern_txt);

bool rrdlabels_match_simple_pattern_parsed(RRDLABELS *labels, SIMPLE_PATTERN *pattern, char equal, size_t *searches);
SIMPLE_PATTERN_RESULT rrdlabels_match_simple_pattern_parsed(RRDLABELS *labels, SIMPLE_PATTERN *pattern, char equal, size_t *searches);
int rrdlabels_to_buffer(RRDLABELS *labels, BUFFER *wb, const char *before_each, const char *equal, const char *quote, const char *between_them,
bool (*filter_callback)(const char *name, const char *value, RRDLABEL_SRC ls, void *data), void *filter_data,
void (*name_sanitizer)(char *dst, const char *src, size_t dst_size),
Expand All @@ -69,8 +69,7 @@ bool pattern_array_label_match(
struct pattern_array *pa,
RRDLABELS *labels,
char eq,
size_t *searches,
bool (*callback_function)(RRDLABELS *, SIMPLE_PATTERN *, char, size_t *));
size_t *searches);
struct pattern_array *pattern_array_add_simple_pattern(struct pattern_array *pa, SIMPLE_PATTERN *pattern, char sep);
struct pattern_array *
pattern_array_add_key_simple_pattern(struct pattern_array *pa, const char *key, SIMPLE_PATTERN *pattern);
Expand Down
6 changes: 3 additions & 3 deletions src/health/health_prototypes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ static char *simple_pattern_trim_around_equal(const char *src) {
return store;
}

static struct pattern_array *trim_and_add_key_to_values(struct pattern_array *pa, const char *key, STRING *input) {
struct pattern_array *trim_and_add_key_to_values(struct pattern_array *pa, const char *key, STRING *input) {
char *tmp = simple_pattern_trim_around_equal(string2str(input));
pa = health_config_add_key_to_values(pa, key, tmp);
freez(tmp);
Expand Down Expand Up @@ -483,7 +483,7 @@ static bool prototype_matches_host(RRDHOST *host, RRD_ALERT_PROTOTYPE *ap) {
return false;

if (host->rrdlabels && ap->match.host_labels_pattern &&
!pattern_array_label_match(ap->match.host_labels_pattern, host->rrdlabels, '=', NULL, rrdlabels_match_simple_pattern_parsed))
!pattern_array_label_match(ap->match.host_labels_pattern, host->rrdlabels, '=', NULL))
return false;

return true;
Expand All @@ -501,7 +501,7 @@ static bool prototype_matches_rrdset(RRDSET *st, RRD_ALERT_PROTOTYPE *ap) {
return false;

if (st->rrdlabels && ap->match.chart_labels_pattern &&
!pattern_array_label_match(ap->match.chart_labels_pattern, st->rrdlabels, '=', NULL, rrdlabels_match_simple_pattern_parsed))
!pattern_array_label_match(ap->match.chart_labels_pattern, st->rrdlabels, '=', NULL))
return false;

return true;
Expand Down

0 comments on commit 1293c2a

Please sign in to comment.