Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

Characters supported in tag values #1216

Closed
qqbb18 opened this issue Feb 5, 2019 · 13 comments
Closed

Characters supported in tag values #1216

qqbb18 opened this issue Feb 5, 2019 · 13 comments
Assignees

Comments

@qqbb18
Copy link

qqbb18 commented Feb 5, 2019

We would like to gain more insight into what printable ASCII characters (32-126) are supported in tag values on Grafana/MetricTank.
Currently, our system only directly supports a limited character set: a-zA-Z0-9-_./%, and blocks other characters (or only allows other special characters via encoding them into HEX format).
 However, the tag values in the timeseries data into our system might contain substantially more characters than this list. 
Therefore we plan to support additional ASCII characters in tag values, given that they will not cause issues to any existing Grafana/MetricTank functionality, such as query, computation, etc. 
We have conducted some basic tests on MetricTank and Grafana by putting each character to different positions of a tag value: leading, trailing and somewhere in the middle. 
After combining the test results with other limitations, we have an initial proposal for the characters supported by our system, which is listed below:



0 - 9 allow everywhere in a tag value

a - z allow everywhere in a tag value

A - Z allow everywhere in a tag value




! # % & - / : < = > @ _ . allow everywhere in a tag value


(space) ~ allow only in the middle of a tag value

; Always block in a tag value because this is not supported by MetricTank

' Always block in a tag value because this is causes request error (unexpected character) when used on Grafana

" $ ( ) * + , ? [ ] \ ^ ` { } | Always block in a tag value because they might cause ambiguity when used in regex queries



Please kindly provide suggestions/comments if you feel any of this is not accurate. Moreover, in order to better understand the restrictions on tag values, we would like to confirm with you on the officially supported list of characters. If possible, please provide brief explanations on why a character is not supported. Finally, it is probably worth adding a regression test for this so we can confidently extend the character set that we plan to support in the future. Thanks!

This issue has also been posted to Grafana:
grafana/grafana#15261

@replay
Copy link
Contributor

replay commented Feb 6, 2019

Generally MT is really permissive regarding tag values. The only character that's not allowed is ;. I wrote a unit test to show that and made sure to include all the special characters you mentioned. In some cases it can be a little tricky to figure out how to query something like \ with a regular expression though. Unfortunately, it seems like GitHub is confused and doesn't show all the characters correctly, but if you do git fetch then it gets copied correctly:

565c5d2f

@Dieterbe
Copy link
Contributor

Dieterbe commented Feb 6, 2019

we should probably align with Graphite on this...

@replay
Copy link
Contributor

replay commented Feb 6, 2019

I forgot to mention that actually a tag value could start with ~, but then it's going to be hard to query it because =~ is the regular expression operator

@replay
Copy link
Contributor

replay commented Feb 6, 2019

Graphite is similarly permissive, I'm not sure how well this is tested though: https://github.com/graphite-project/graphite-web/blob/master/webapp/graphite/tags/utils.py#L52

@qqbb18
Copy link
Author

qqbb18 commented Feb 6, 2019

@replay This was exactly what I have found from my tests: the only character not accepted by MT is ;. Thanks for adding a unit test to include all the characters mentioned above! I totally agree that it is tricky to figure out how to test with regex queries.

However I found there are cases some other characters that do not work well with Grafana, for example, ', this always gives request error "unexpected character", but this might be a Grafana/Graphite issue ...

@replay
Copy link
Contributor

replay commented Feb 6, 2019

The issue with ' in the middle of the value is probably related to how Grafana submits the query to Graphite. May I ask why it is necessary for you to use such characters in tag values?

@shalstea
Copy link

shalstea commented Feb 6, 2019

We aren't asking for it per se. We are suggesting that you document and test what you support/don't support. We would love to have all the characters supported but realize that, that isn't feasible.

For us we are sourcing tag values from systems we don't control (hardware device info, etc). They can have any character, if we don't support them directly then we have to encode the values in some way. We provide teams with a way to do the encoding but it requires a character by character copy, examination. Right now we have nothing officially documented / supported from MetricTank/Grafana and this is to help close the gap.. and then we can work around the corner cases on our side.

@replay
Copy link
Contributor

replay commented Mar 18, 2019

As an update to the Graphite (and MT is Graphite compatible) documentation, we got the character support documented in the official docs now: graphite-project/graphite-web#2426

@robert-milan robert-milan mentioned this issue May 21, 2019
27 tasks
@Dieterbe
Copy link
Contributor

Dieterbe commented Jul 8, 2019

@replay status of this?

@replay
Copy link
Contributor

replay commented Jul 10, 2019

On Metrictank side i think this can be considered done with the merging of the following PRs:
#1371
#1348
raintank/schema#28

In Graphite & Carbon there are still related changes which have not been merged yet, but that doesn't really concern this issue:
graphite-project/graphite-web#2458
graphite-project/carbon#858

@robert-milan
Copy link
Contributor

On Metrictank side i think this can be considered done with the merging of the following PRs:
#1371
#1348
raintank/schema#28

^^ Those are all merged

In Graphite & Carbon there are still related changes which have not been merged yet, but that doesn't really concern this issue:
graphite-project/graphite-web#2458
graphite-project/carbon#858

There are ongoing discussions and work related to the graphite-project changes.

@shalstea
Copy link

Can this be closed ?

@replay
Copy link
Contributor

replay commented Dec 12, 2019

I'm waiting for the two PRs in the graphite-project to be closed. let me ping the involved people about those

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants