Skip to content

Commit 3179be0

Browse files
authored
Sample numbers and bools in tags and metrics (#2521)
Signed-off-by: Bob Weinand <[email protected]>
1 parent 60100d9 commit 3179be0

File tree

3 files changed

+160
-45
lines changed

3 files changed

+160
-45
lines changed

ext/ddshared.c

+55-15
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,62 @@ void ddshared_minit(void) {
1717
ddtrace_php_version = Z_STR_P(zend_get_constant_str(ZEND_STRL("PHP_VERSION")));
1818
}
1919

20-
bool dd_rule_matches(zval *pattern, zval *prop, int rulesFormat) {
20+
bool dd_glob_rule_is_wildcards_only(zval *pattern) {
21+
if (Z_TYPE_P(pattern) != IS_STRING || Z_STRLEN_P(pattern) == 0) {
22+
return false;
23+
}
24+
char *p = Z_STRVAL_P(pattern);
25+
while (*p == '*') {
26+
++p;
27+
}
28+
return *p == 0;
29+
}
30+
31+
bool dd_rule_matches(zval *pattern, zval *prop, int rulesFormat) {
2132
if (Z_TYPE_P(pattern) != IS_STRING) {
2233
return false;
2334
}
24-
if (Z_TYPE_P(prop) != IS_STRING) {
25-
return true; // default case unset or null must be true, everything else is too then...
35+
zend_string *str;
36+
if (Z_TYPE_P(prop) == IS_STRING) {
37+
str = zend_string_copy(Z_STR_P(prop));
38+
} else {
39+
if (Z_TYPE_P(prop) == IS_TRUE) {
40+
#if PHP_VERSION_ID < 80200
41+
str = zend_string_init("true", 4, 0);
42+
#else
43+
str = ZSTR_KNOWN(ZEND_STR_TRUE);
44+
#endif
45+
} else if (Z_TYPE_P(prop) == IS_FALSE) {
46+
#if PHP_VERSION_ID < 80200
47+
str = zend_string_init("false", 5, 0);
48+
#else
49+
str = ZSTR_KNOWN(ZEND_STR_FALSE);
50+
#endif
51+
} else if (Z_TYPE_P(prop) == IS_LONG) {
52+
str = zend_long_to_str(Z_LVAL_P(prop));
53+
} else if (Z_TYPE_P(prop) == IS_DOUBLE) {
54+
zend_long to_long = zend_dval_to_lval(Z_DVAL_P(prop));
55+
if (Z_DVAL_P(prop) == (double)to_long) {
56+
str = zend_long_to_str(to_long);
57+
} else {
58+
return dd_glob_rule_is_wildcards_only(pattern);
59+
}
60+
} else {
61+
return Z_STRLEN_P(pattern) == 0 || dd_glob_rule_is_wildcards_only(pattern);
62+
}
2663
}
2764

65+
bool result;
2866
if (rulesFormat == DD_TRACE_SAMPLING_RULES_FORMAT_GLOB) {
29-
return dd_glob_rule_matches(pattern, Z_STR_P(prop));
30-
}
31-
else {
32-
return zai_match_regex(Z_STR_P(pattern), Z_STR_P(prop));
67+
result = dd_glob_rule_matches(pattern, str);
68+
} else {
69+
result = zai_match_regex(Z_STR_P(pattern), str);
3370
}
71+
zend_string_release(str);
72+
return result;
3473
}
3574

36-
bool dd_glob_rule_matches(zval *pattern, zend_string* value) {
75+
bool dd_glob_rule_matches(zval *pattern, zend_string *value) {
3776
if (Z_TYPE_P(pattern) != IS_STRING) {
3877
return false;
3978
}
@@ -42,18 +81,15 @@ bool dd_glob_rule_matches(zval *pattern, zend_string* value) {
4281
char *s = ZSTR_VAL(value);
4382

4483
int wildcards = 0;
45-
int patternLength = 0;
46-
int stringLength = ZSTR_LEN(value);
4784
while (*p) {
4885
if (*(p++) == '*') {
4986
++wildcards;
5087
}
51-
patternLength++;
5288
}
5389

5490
// If there are no wildcards, no need to go through the whole string if pattern is shorter than the input string
55-
// Indeed wildcards (ie '*') can replace multiple characters while '?' canonly replace one
56-
if (wildcards == 0 && patternLength < stringLength) {
91+
// Indeed wildcards (i.e. '*') can replace multiple characters while '?' can only replace one
92+
if (wildcards == 0 && Z_STRLEN_P(pattern) < ZSTR_LEN(value)) {
5793
return false;
5894
}
5995

@@ -71,10 +107,14 @@ bool dd_glob_rule_matches(zval *pattern, zend_string* value) {
71107
free_alloca(backtrack_points, use_heap);
72108
return !*p;
73109
}
74-
if (*s == *p || *p == '?') {
110+
// equal or case-insensitive match
111+
if (*s == *p || *p == '?' || ((*s | ' ') == (*p | ' ') && (*p | ' ') >= 'a' && (*p | ' ') <= 'z')) {
75112
++s, ++p;
76113
} else if (*p == '*') {
77-
backtrack_points[backtrack_idx++] = ++p;
114+
do {
115+
++p;
116+
} while (*p == '*');
117+
backtrack_points[backtrack_idx++] = p;
78118
backtrack_points[backtrack_idx++] = s;
79119
} else {
80120
do {

ext/priority_sampling/priority_sampling.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,12 @@ static bool dd_check_sampling_rule(zend_array *rule, ddtrace_span_data *span) {
8282
if ((rule_pattern = zend_hash_str_find(rule, ZEND_STRL("tags"))) && Z_TYPE_P(rule_pattern) == IS_ARRAY) {
8383
zend_array *tag_rules = Z_ARR_P(rule_pattern);
8484
zend_array *meta = ddtrace_property_array(&span->property_meta);
85+
zend_array *metrics = ddtrace_property_array(&span->property_metrics);
8586
zend_string *tag_name;
8687
ZEND_HASH_FOREACH_STR_KEY_VAL(tag_rules, tag_name, rule_pattern) {
8788
if (tag_name) {
8889
zval *value;
89-
if (!(value = zend_hash_find(meta, tag_name))) {
90+
if (!(value = zend_hash_find(meta, tag_name)) && !(value = zend_hash_find(metrics, tag_name))) {
9091
return false;
9192
}
9293
if (!dd_rule_matches(rule_pattern, value, get_DD_TRACE_SAMPLING_RULES_FORMAT())) {

tests/ext/priority_sampling/021-rule-name-as-glob-with-question-mark.phpt

+103-29
Original file line numberDiff line numberDiff line change
@@ -19,50 +19,90 @@ $tests = [
1919
["f*o*e", "fooname", true],
2020
["f*o*m?", "fooname", true],
2121
["f*x*m?", "fooname", false],
22-
22+
["fooname", "FOONAME", true],
23+
["fooNAME", "FOOname", true],
24+
["fooNAME", "FOOname", true],
25+
["[ooNAM]", "{OOnam}", false],
26+
["{ooNAM}", "[OOnam]", false],
27+
["*", 1.2, true],
28+
["****", 1.2, true],
29+
["", 1.2, false],
30+
["1.2", 1.2, false],
31+
["", 1, false],
32+
["1", 1, true],
33+
["1", 1.0, true],
34+
["1*", 1.0, true],
35+
["1*", 1, true],
36+
["1*", 10, true],
37+
["1*", 0, false],
38+
["true", true, true],
39+
["truee", true, false],
40+
["*", true, true],
41+
["FALSE", false, true],
42+
["FALS", false, false],
43+
["", null, true],
44+
["*", null, true],
45+
["?", null, false],
2346
];
2447

2548
foreach ($tests as list($pattern, $name, $matches)) {
26-
ini_set("datadog.trace.sampling_rules", '[{"name":"' . $pattern . '","sample_rate":0.7},{"sample_rate": 0.3}]');
49+
if (\is_string($name)) {
50+
ini_set("datadog.trace.sampling_rules", '[{"name":"' . $pattern . '","sample_rate":0.7},{"sample_rate": 0.3}]');
2751

28-
$root = DDTrace\root_span();
29-
$root->name = $name;
52+
$root = DDTrace\root_span();
53+
$root->name = $name;
3054

31-
DDTrace\get_priority_sampling();
55+
DDTrace\get_priority_sampling();
3256

33-
if ($root->metrics["_dd.rule_psr"] == ($matches ? 0.7 : 0.3)) {
34-
echo "As expected, $pattern " . ($matches ? "matches" : "doesn't match") . " $name (name)\n";
35-
} else {
36-
echo "$pattern " . ($matches ? "should have matched" : "shouldn't have matched") . " $name (service). Metrics found were: \n";
37-
var_dump($root->metrics);
38-
}
57+
if ($root->metrics["_dd.rule_psr"] == ($matches ? 0.7 : 0.3)) {
58+
echo "As expected, $pattern " . ($matches ? "matches" : "doesn't match") . " $name (name)\n";
59+
} else {
60+
echo "$pattern " . ($matches ? "should have matched" : "shouldn't have matched") . " $name (service). Metrics found were: \n";
61+
var_dump($root->metrics);
62+
}
3963

40-
ini_set("datadog.trace_sampling_rules", '[{"service":"' . $pattern . '","sample_rate":0.7},{"sample_rate": 0.3]');
64+
ini_set("datadog.trace.sampling_rules", '[{"service":"' . $pattern . '","sample_rate":0.7},{"sample_rate": 0.3}]');
4165

42-
$root = \DDTrace\root_span();
43-
$root->service = $name;
66+
$root = \DDTrace\root_span();
67+
$root->service = $name;
4468

45-
\DDTrace\get_priority_sampling();
69+
\DDTrace\get_priority_sampling();
4670

47-
if ($root->metrics["_dd.rule_psr"] == ($matches ? 0.7 : 0.3)) {
48-
echo "As expected, $pattern " . ($matches ? "matches" : "doesn't match") . " $name (service)\n";
49-
} else {
50-
echo "$pattern " . ($matches ? "should have matched" : "shouldn't have matched") . " $name (service). Metrics found were: \n";
51-
var_dump($root->metrics);
52-
}
71+
if ($root->metrics["_dd.rule_psr"] == ($matches ? 0.7 : 0.3)) {
72+
echo "As expected, $pattern " . ($matches ? "matches" : "doesn't match") . " $name (service)\n";
73+
} else {
74+
echo "$pattern " . ($matches ? "should have matched" : "shouldn't have matched") . " $name (service). Metrics found were: \n";
75+
var_dump($root->metrics);
76+
}
5377

54-
ini_set("datadog.trace_sampling_rules", '[{"resource":"' . $pattern . '","sample_rate":0.7},{"sample_rate": 0.3]');
78+
ini_set("datadog.trace.sampling_rules", '[{"resource":"' . $pattern . '","sample_rate":0.7},{"sample_rate": 0.3}]');
5579

56-
$root = \DDTrace\root_span();
57-
$root->resource = $name;
80+
$root = \DDTrace\root_span();
81+
$root->resource = $name;
5882

59-
\DDTrace\get_priority_sampling();
83+
\DDTrace\get_priority_sampling();
6084

61-
if ($root->metrics["_dd.rule_psr"] == ($matches ? 0.7 : 0.3)) {
62-
echo "As expected, $pattern " . ($matches ? "matches" : "doesn't match") . " $name (resource)\n";
85+
if ($root->metrics["_dd.rule_psr"] == ($matches ? 0.7 : 0.3)) {
86+
echo "As expected, $pattern " . ($matches ? "matches" : "doesn't match") . " $name (resource)\n";
87+
} else {
88+
echo "$pattern " . ($matches ? "should have matched" : "shouldn't have matched") . " $name (resource). Metrics found were: \n";
89+
var_dump($root->metrics);
90+
}
6391
} else {
64-
echo "$pattern " . ($matches ? "should have matched" : "shouldn't have matched") . " $name (resource). Metrics found were: \n";
65-
var_dump($root->metrics);
92+
ini_set("datadog.trace.sampling_rules", '[{"tags":{"foo":"' . $pattern . '"},"sample_rate":0.7},{"sample_rate": 0.3}]');
93+
94+
$root = \DDTrace\root_span();
95+
$root->meta["foo"] = $name;
96+
97+
\DDTrace\get_priority_sampling();
98+
99+
$name = var_export($name, true);
100+
if ($root->metrics["_dd.rule_psr"] == ($matches ? 0.7 : 0.3)) {
101+
echo "As expected, $pattern " . ($matches ? "matches" : "doesn't match") . " $name (tag)\n";
102+
} else {
103+
echo "$pattern " . ($matches ? "should have matched" : "shouldn't have matched") . " $name (tag). Metrics found were: \n";
104+
var_dump($root->metrics);
105+
}
66106
}
67107
}
68108
?>
@@ -104,3 +144,37 @@ As expected, f*o*m? matches fooname (resource)
104144
As expected, f*x*m? doesn't match fooname (name)
105145
As expected, f*x*m? doesn't match fooname (service)
106146
As expected, f*x*m? doesn't match fooname (resource)
147+
As expected, fooname matches FOONAME (name)
148+
As expected, fooname matches FOONAME (service)
149+
As expected, fooname matches FOONAME (resource)
150+
As expected, fooNAME matches FOOname (name)
151+
As expected, fooNAME matches FOOname (service)
152+
As expected, fooNAME matches FOOname (resource)
153+
As expected, fooNAME matches FOOname (name)
154+
As expected, fooNAME matches FOOname (service)
155+
As expected, fooNAME matches FOOname (resource)
156+
As expected, [ooNAM] doesn't match {OOnam} (name)
157+
As expected, [ooNAM] doesn't match {OOnam} (service)
158+
As expected, [ooNAM] doesn't match {OOnam} (resource)
159+
As expected, {ooNAM} doesn't match [OOnam] (name)
160+
As expected, {ooNAM} doesn't match [OOnam] (service)
161+
As expected, {ooNAM} doesn't match [OOnam] (resource)
162+
As expected, * matches 1.2 (tag)
163+
As expected, **** matches 1.2 (tag)
164+
As expected, doesn't match 1.2 (tag)
165+
As expected, 1.2 doesn't match 1.2 (tag)
166+
As expected, doesn't match 1 (tag)
167+
As expected, 1 matches 1 (tag)
168+
As expected, 1 matches 1.0 (tag)
169+
As expected, 1* matches 1.0 (tag)
170+
As expected, 1* matches 1 (tag)
171+
As expected, 1* matches 10 (tag)
172+
As expected, 1* doesn't match 0 (tag)
173+
As expected, true matches true (tag)
174+
As expected, truee doesn't match true (tag)
175+
As expected, * matches true (tag)
176+
As expected, FALSE matches false (tag)
177+
As expected, FALS doesn't match false (tag)
178+
As expected, matches NULL (tag)
179+
As expected, * matches NULL (tag)
180+
As expected, ? doesn't match NULL (tag)

0 commit comments

Comments
 (0)