grobian / carbon-c-relay

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

Validation doesn't work for tagged metrics #410

Open ihard opened 4 years ago

ihard commented 4 years ago

build from latest master With

match *
    validate ^[0-9.e+-]+\ [0-9.e+-]+$ else drop
    ;

test metric

echo "test_metric;instance=hostname1;env=dev 30 `date +%s`" >/dev/tcp/127.0.0.1/2003

go to dev null if comment section

#match *
#    validate ^[0-9.e+-]+\ [0-9.e+-]+$ else drop
#    ;

the metric successfully enters the storage and is visible in Grafana

grobian commented 4 years ago

your validation rule requires a space, isn't it to be expected that no metric will match that?

ihard commented 4 years ago

Doesn't the validation expression only check the value? The verification expression is taken from the documentation. Metrics without tags pass verification successfully.

echo "test.metric 30 `date +%s`" >/dev/tcp/127.0.0.1/2003
grobian commented 4 years ago

Right you are, I'm obviously not remembering well any more. I stand corrected.

grobian commented 4 years ago

I'm not sure if you use tags or not, but I have a hunch that if you add ; to the allowed chars setting, it will work as expected. Tag support basically hides the tag from the routing model, but also breaks validation in this way.

ihard commented 4 years ago

It really helped, thank you. It wasn't quite obvious. I use tags in metrics and I was confused by this description of the release:

3.3 (23-03-2018)
dispatcher tags support was added, it is only activated when the
; character is not in the list of allowed characters (-c flag).
Note that by default this character is not allowed, hence tags support
enabled.

With the -c -_=; option, tagged metrics work in version 3.6.

grobian commented 4 years ago

I think eventually this is very similar to #398: we need something to validate tags I think.