go-graphite / carbon-clickhouse

Graphite metrics receiver with ClickHouse as storage
MIT License
187 stars 47 forks source link

Tags uploader breaking change. #116

Closed lexx-bright closed 1 year ago

lexx-bright commented 1 year ago

After 5257f8e6ec977b5fc3d777f9044061d2e87d8b00, Tag1 and Tags fields are uploaded unescaped. @msaf1980 stated this was intentional https://github.com/go-graphite/carbon-clickhouse/blob/1a337757495e47cfeef1f72dac362386273cb1b3/uploader/tagged.go#L85 But this change breaks queries using special symbols.

echo 'test_metric;minus=-;plus=+;percent=%;underscore=_;colon=:;hash=#;forward=/' 0 $(date +%s) | nc 127.0.0.1 2003

Before
┌─Tag1─────────────────┬─Tags──────────────────────────────────────────────────────────────────────────────────────────────────┐
│ __name__=test_metric │ ['__name__=test_metric','colon=:','forward=/','hash=#','minus=-','percent=%','plus=+','underscore=_'] │
│ colon=:              │ ['__name__=test_metric','colon=:','forward=/','hash=#','minus=-','percent=%','plus=+','underscore=_'] │
│ forward=/            │ ['__name__=test_metric','colon=:','forward=/','hash=#','minus=-','percent=%','plus=+','underscore=_'] │
│ hash=#               │ ['__name__=test_metric','colon=:','forward=/','hash=#','minus=-','percent=%','plus=+','underscore=_'] │
│ minus=-              │ ['__name__=test_metric','colon=:','forward=/','hash=#','minus=-','percent=%','plus=+','underscore=_'] │
│ percent=%            │ ['__name__=test_metric','colon=:','forward=/','hash=#','minus=-','percent=%','plus=+','underscore=_'] │
│ plus=+               │ ['__name__=test_metric','colon=:','forward=/','hash=#','minus=-','percent=%','plus=+','underscore=_'] │
│ underscore=_         │ ['__name__=test_metric','colon=:','forward=/','hash=#','minus=-','percent=%','plus=+','underscore=_'] │
└──────────────────────┴───────────────────────────────────────────────────────────────────────────────────────────────────────┘
After
┌─Tag1─────────────────┬─Tags────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ __name__=test_metric │ ['__name__=test_metric','colon=%3A','forward=%2F','hash=%23','minus=-','percent=%25','plus=%2B','underscore=_'] │
│ colon=%3A            │ ['__name__=test_metric','colon=%3A','forward=%2F','hash=%23','minus=-','percent=%25','plus=%2B','underscore=_'] │
│ forward=%2F          │ ['__name__=test_metric','colon=%3A','forward=%2F','hash=%23','minus=-','percent=%25','plus=%2B','underscore=_'] │
│ hash=%23             │ ['__name__=test_metric','colon=%3A','forward=%2F','hash=%23','minus=-','percent=%25','plus=%2B','underscore=_'] │
│ minus=-              │ ['__name__=test_metric','colon=%3A','forward=%2F','hash=%23','minus=-','percent=%25','plus=%2B','underscore=_'] │
│ percent=%25          │ ['__name__=test_metric','colon=%3A','forward=%2F','hash=%23','minus=-','percent=%25','plus=%2B','underscore=_'] │
│ plus=%2B             │ ['__name__=test_metric','colon=%3A','forward=%2F','hash=%23','minus=-','percent=%25','plus=%2B','underscore=_'] │
│ underscore=_         │ ['__name__=test_metric','colon=%3A','forward=%2F','hash=%23','minus=-','percent=%25','plus=%2B','underscore=_'] │
└──────────────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

curl -s '127.0.0.1:19999/render/?' --data-urlencode 'target=seriesByTag("plus=+")' --data-urlencode "format=json" --data-urlencode "until=now" --data-urlencode "from=-1h"
[]

INFO [render] query {"request_id": "be360a700560749ff375fbd3fdd3725f", "carbonapi_uuid": "c73243da-db30-4212-bc36-6f383a45da8e", "query": "SELECT Path FROM default.graphite_tagged_new  WHERE (Tag1='plus=+') AND (Date >='2022-11-16' AND Date <= '2022-11-16') GROUP BY Path FORMAT TabSeparatedRaw", "read_rows": "16", "read_bytes": "1836", "written_rows": "0", "written_bytes": "0", "total_rows_to_read": "16", "query_id": "be360a700560749ff375fbd3fdd3725f::7786c48987549a52", "time": 0.024265706}

curl -s '127.0.0.1:19999/render/?' --data-urlencode 'target=seriesByTag("plus=%2B")' --data-urlencode "format=json" --data-urlencode "until=now" --data-urlencode "from=-1h"
[{"target":"test_metric;colon=%3A;forward=%2F;hash=%23;minus=-;percent=%25;plus=%2B;underscore=_","datapoints":[[null,1668597900],[null,1668597960],[null,1668598020],[null,1668598080],[null,1668598140],[null,1668598200],[null,1668598260],[null,1668598320],[null,1668598380],[null,1668598440],[null,1668598500],[0,1668598560],[null,1668598620],[null,1668598680],[null,1668598740],[0,1668598800],[0,1668598860],[null,1668598920],[null,1668598980],[null,1668599040],[null,1668599100],[null,1668599160],[null,1668599220],[null,1668599280],[null,1668599340],[null,1668599400],[null,1668599460],[null,1668599520],[null,1668599580],[null,1668599640],[null,1668599700],[null,1668599760],[null,1668599820],[null,1668599880],[null,1668599940],[null,1668600000],[null,1668600060],[null,1668600120],[null,1668600180],[null,1668600240],[null,1668600300],[null,1668600360],[null,1668600420],[null,1668600480],[null,1668600540],[null,1668600600],[null,1668600660],[null,1668600720],[null,1668600780],[null,1668600840],[null,1668600900],[null,1668600960],[null,1668601020],[null,1668601080],[null,1668601140],[null,1668601200],[null,1668601260],[null,1668601320],[null,1668601380],[null,1668601440]],"tags":{"colon":"%3A","forward":"%2F","hash":"%23","minus":"-","name":"test_metric","percent":"%25","plus":"%2B","underscore":"_"}}]

INFO [render] query {"request_id": "896fb921edf2c68118c621176bff3e65", "carbonapi_uuid": "c4c8dc7b-3186-459b-9e44-44eaace5b2e5", "query": "SELECT Path FROM default.graphite_tagged_new  WHERE (Tag1='plus=%2B') AND (Date >='2022-11-16' AND Date <= '2022-11-16') GROUP BY Path FORMAT TabSeparatedRaw", "written_bytes": "0", "total_rows_to_read": "16", "read_rows": "16", "read_bytes": "1836", "written_rows": "0", "query_id": "896fb921edf2c68118c621176bff3e65::aeeefffd65f47a56", "time": 0.008576678}
msaf1980 commented 1 year ago

In our evironment we don't use this symbols, and no tests for them. Need to be fixed, release will be canceled.

msaf1980 commented 1 year ago

@lexx-bright Can you test against current master ? All tests (unit and e2e) added and unescape work as expected. But additional check before release is not bad.

lexx-bright commented 1 year ago

Carbon-clickhouse seems to be fixed. Raised a related issue https://github.com/go-graphite/graphite-clickhouse/issues/207 for graphite-clickhouse.

Hipska commented 1 year ago

Also have a look at the building package actions, it says build successful, but if you look at the details of step "Push packages to the stable repo" it says it could not upload package as it already existed. Which makes sense, new package name should be carbon-clickhouse-0.11.3-2.x86_64.rpm

msaf1980 commented 1 year ago

Yes, some changes in release cycle is needed. But may be before next release, now simple increment version.

Hipska commented 1 year ago

This seems not to be fixed completely, as now tag values with spaces in them are stored with a +. This breaks existing graphite queries and conflicts with tags where value actually have a + sign.

Made new issue for that: #124