Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support tags in line protocol #4

Closed
grzkv opened this issue Jan 24, 2020 · 4 comments
Closed

Support tags in line protocol #4

grzkv opened this issue Jan 24, 2020 · 4 comments

Comments

@grzkv
Copy link
Member

grzkv commented Jan 24, 2020

Add support for the extended line protocol with tags (see https://graphite.readthedocs.io/en/latest/tags.html). Currently, we support only the record in the form

path.is.here 123.45 12345678

Adding tags support will include supporting records in the format

path.is.here;tag1=val1;tag2=val2;tag3=val3 123.45 12345678

The tags will not influence routing in the first version. The record validation should be handled with care. If it will make sense for performance or other reasons, tag support may be made optional with a config flag.

@grzkv grzkv added the good first issue Good for newcomers label Jan 24, 2020
@jwkohnen
Copy link
Member

I am working on this.

I was reading a bit of code of graphite-project/carbon and found that they handle the tag with the name name special: It is the metric path without leading ~ (sanitized name as tag name) and it is always the first tag while the others are sorted asciibetically. Now that I've implemented this I am wondering if we should even bother with that in nanotube or just send it over at let the other end deal with these details?

In other words: How far should we do validation at this (nanotube's) stage of the pipeline?

Another aspect of this question: I found ambiguous information about what characters are allowed as tag names and values. (I am slightly bothered that graphite calls key=value pairs tags and not labels like the rest of the world does, but whatever...) In some place they demand "ASCII" characters except special characters like !=~; etc. which means a lot of "garbage" is allowed like \g\t\r\xff\x00 which is weird. In the validation code of carbon there is no filtering whatsoever though (short of the special characters) and somewhere it is mentioned that any UTF-8 character is allowed but "not well tested". In fact, the lack of validation suggests that all kinds of nonsense like \u200b\ufffd is allowed.

Now, should we pass through everything? Or should we restrict to ASCII-only (that's what I've implementd now) or restrict to printables (ascii? UTF-8?)?

@grzkv
Copy link
Member Author

grzkv commented Jan 30, 2020

I was reading a bit of code of graphite-project/carbon and found that they handle the tag with the name special: It is the metric path without leading ~ (sanitized name as tag name) and it is always the first tag while the others are sorted asciibetically. Now that I've implemented this I am wondering if we should even bother with that in nanotube or just send it over at let the other end deal with these details?

I'd say there is no need to bother with anything like this now. Let's get a simple implementation first and then build on top of it.

In other words: How far should we do validation at this (nanotube's) stage of the pipeline?

If tags are enabled, I'd say we have to do the full record validation. I.e. we should check that the metric name is indeed composed of a valid path and a valid set of tags.

Now, should we pass through everything? Or should we restrict to ASCII-only (that's what I've implementd now) or restrict to printables (ascii? UTF-8?)?

We should use the same set of allowed symbols for tags as we do for paths, otherwise, we will make it confusing. Our current implementation will only allow working with single-byte characters anyway. Reading Unicode strings into characters is computationally costly, and we don't do it.

@jwkohnen jwkohnen removed their assignment Apr 29, 2020
@grzkv grzkv removed the good first issue Good for newcomers label Jul 20, 2020
@grzkv
Copy link
Member Author

grzkv commented Jul 20, 2020

I am postponing this to the point when we see an actual need for it.

@grzkv grzkv added the wontfix label Jul 20, 2020
@grzkv grzkv removed the wontfix label Oct 18, 2021
@grzkv
Copy link
Member Author

grzkv commented Mar 9, 2022

Currently, we do not have a Graphite storage that would support tags on scale. Since this project aims to primarily serve high-load and high-scale setups, this issue is closed for now.

@grzkv grzkv closed this as completed Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants