search

grafana / carbon-relay-ng

Fast carbon relay+aggregator with admin interfaces for making changes online - production ready
Other
467 stars 151 forks source link

Carbon-relay-ng accepts badly formatted metrics with strict mode enabled #137

Open pawellesniewski opened 8 years ago

pawellesniewski commented 8 years ago

Hi! I really like carbon-relay-ng daemon, which has replaced my previous carbon-relay basic installation. Unfortunatelly i discovered that it accepts badly formatted metrics even with "strict" legacy metric validation level. So far i disovered only two special cases:

First - metric with "NaN" instead of value:

[~>> echo "test.metric NaN `date +%s`" | nc x.x.x.x 2003
16:18:16.864778 ▶ INFO  table sending to route: test.metric NaN 1476713896
16:18:16.864928 ▶ INFO  route default sending to dest y.y.y.y:2003: test.metric NaN 1476713896
16:18:16.865058 ▶ INFO  dest y.y.y.y:2003 test.metric NaN 1476713896 received from In -> nonBlockingSend
16:18:16.865149 ▶ INFO  conn y.y.y.y:2003 HandleData: writing test.metric NaN 1476713896
16:18:16.945107 ▶ INFO  bufWriter y_y_y_y_2003 flush-writing to tcp test.metric NaN 1476713896
16:18:16.945462 ▶ INFO  bufWriter y_y_y_y_2003 flush-writing to tcp

Second - metric with "new line" character:

[~>> echo -e "test.metric 1 `date +%s`\n" | nc x.x.x.x 2003
16:18:35.739095 ▶ INFO  table sending to route: test.metric 1 1476713915
16:18:35.741651 ▶ INFO  route default sending to dest y.y.y.y:2003: test.metric 1 1476713915
16:18:35.742258 ▶ INFO  dest y.y.y.y:2003 test.metric 1 1476713915 received from In -> nonBlockingSend
16:18:35.742595 ▶ INFO  conn y.y.y.y:2003 HandleData: writing test.metric 1 1476713915
16:18:35.945254 ▶ INFO  bufWriter y_y_y_y_2003 flush-writing to tcp test.metric 1 1476713915
16:18:35.946116 ▶ INFO  bufWriter y_y_y_y_2003 flush-writing to tcp

which is then badly formatted on receiving site.

Dieterbe commented 8 years ago

hmm the validation only validates metric keys. you're right that we should also validate the other fields and format of the message itself.

Dieterbe commented 7 years ago

wait i'm mistaken. it's not a missing feature, we do have the validation. we validate that we have 3 fields and that they are a float and a unix timestamp, at least that's what should happen. but from your report it looks like it's broken.

anyway when i have some time i will reproduce. should be easy to find and fix . (feel free to beat me to it with a PR though? :))