grobian / carbon-c-relay

Enhanced C implementation of Carbon relay, aggregator and rewriter
Apache License 2.0
380 stars 107 forks source link

quoted expressions stop working properly #457

Closed tantra35 closed 4 months ago

tantra35 commented 1 year ago

seems after https://github.com/grobian/carbon-c-relay/commit/f3ee3f1bc18d29f1321518a1aa8f3292603536ea broken follow expression(it is quoted to escape ;)

match "^local\.nomad-jobs\.[^;]+;.+"

now it doesn't select metrics like this:

local.nomad-jobs.aav01.default.an-abtests-web-back.bytask.cpu.Percent.p99;tg=an-abtests-web;task=django_celery_beat

so metrics went to the wrong destination

grobian commented 1 year ago

Mentioned commit only dealt with printing the expressions, not any matching.

If I try your example, it just matches for me. How did your c-relay get configured? What regex library is it using? (relay -v will show which)

tantra35 commented 1 year ago

Yes you right i alredy realized it

In test stand we use follow configuration

cluster send-localhost
    forward
        localhost:2333
;

match "^local\.nomad-jobs\.[^;]+;.+"
    send to
        send-localhost
    stop
;

match *
    send to
        blackhole
    stop
;

carbon-c-relay -v output

carbon-c-relay v3.7.4 (636551-dirty)
enabled support for: gzip ssl
regular expressions library: libc

on localhost we launch just simple socket server that write to stdout all that is get from socket:

#!/bin/bash
CARBONPORT=2333

echo "Start listening on port tcp ${CARBONPORT} ..."
while read line
do
    echo "$line"
done < <(nc -k -l ${CARBONPORT})
echo "Good bye" 

then we begin put metrics to carbon-c-relay like this:

local.nomad-jobs.aav01.default.an-abtests-web-back.bytask.cpu.Percent.p99;tg=an-abtests-web;task=django_celery_beat 12 1675080983

and doesn't see any output, so metrics goes to "blackhole"

Also i must mention that in version 3.5 this quoted expression works

For expiriments we try to replace(experiments with additional slashes):

"^local\.nomad-jobs\.[^;]+;.+" -> "^local\\\.nomad-jobs\\\.[^;]+;.+"

and this is doesn't help, metrics doesn't selected as expected, and goes to blackhole on test stand

grobian commented 1 year ago
regular expressions library: libc

I'm almost sure this is the problem.

You mention this works on 3.5, can you post the regex library used by that version?

tantra35 commented 1 year ago

Hm, regexp library is the same, one difference is a ubuntu release(in docker container) for 3.5 we use ubuntu 16.04(xenial), and for 3.7 ubuntu 20.04(focal)

# carbon-c-relay -v
carbon-c-relay v3.5 (973e08-dirty)
enabled support for: gzip ssl
regular expressions library: libc
tantra35 commented 1 year ago

Also just replace libc re library to PCRE:

carbon-c-relay v3.7.4 (636551-dirty)
enabled support for: gzip ssl
regular expressions library: PCRE2

without success

grobian commented 1 year ago
carbon-c-relay v3.7.4 (3ce549)
enabled support for: gzip lz4 ssl
regular expressions library: oniguruma

that gives me:

match "^local.nomad-jobs.[^;]+;.+"
    send to blackhole
    stop
    ;

local.nomad-jobs.aav01.default.an-abtests-web-back.bytask.cpu.Percent.p99;tg=an-abtests-web;task=django_celery_beat
match
    ^local.nomad-jobs.[^;]+;.+ (regex) -> local.nomad-jobs.aav01.default.an-abtests-web-back.bytask.cpu.Percent.p99;tg=an-abtests-web;task=django_celery_beat
    blackholed
    stop

e.g. it matches your expression. I don't understand why this would change for you. There are differences between regex libraries for sure (some need escaping, others don't) but if you can run 3.5 next to 3.7.4 and get a different match, it's very likely the c-relay code that makes the difference.

grobian commented 1 year ago

In think https://github.com/grobian/carbon-c-relay/commit/4ecb19330f063ca8d851066834a82f9a5caf0787 is more of a problem to you. It would mean that "^local.nomad-jobs..+ would simply match the same thing, for ;.+ is never exposed to the matching.

(bug/fault in the test routine)

tantra35 commented 1 year ago

Hm, but how now detect metrics with tags? For example in our case we want that tagged metrics goes to the different metrics storage

I think that remove tags part from metric name is wrong decision

grobian commented 1 year ago

Yes, for that case, you'd disable tag support, by adding ; to the set of allowed characters.

It gets more complicated if you want to have consistent-hashing on the metric name (without tags) at the same time.

grobian commented 4 months ago

I think this is a configuration issue