Tags uploader breaking change.
lexx-bright opened this issue · 6 comments
lexx-bright commented
After 5257f8e, Tag1 and Tags fields are uploaded unescaped.
@msaf1980 stated this was intentional
carbon-clickhouse/uploader/tagged.go
Line 85 in 1a33775
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
In our evironment we don't use this symbols, and no tests for them. Need to be fixed, release will be canceled.
msaf1980 commented
@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
Carbon-clickhouse seems to be fixed. Raised a related issue go-graphite/graphite-clickhouse#207 for graphite-clickhouse.
Hipska commented
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
Yes, some changes in release cycle is needed. But may be before next release, now simple increment version.