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

Match rule regexp doesn't process last tag when tag support is activated. #398

Open
lexx-bright opened this issue Dec 27, 2019 · 5 comments

Comments

@lexx-bright
Copy link
Contributor

When tag support is activated match rule regexp processing is performed up until last ";". So if metrics look like metric.example;tag1=value1;tag2=value2;tag3=value3 you can't route based on tag3=value3, because only "metric.example;tag1=value1;tag2=value2" substring is taken to account.

@grobian
Copy link
Owner

grobian commented Dec 28, 2019

hmm, ok, I didn't know this was possible

grobian added a commit that referenced this issue Dec 28, 2019
a logic error would only exlude the last tag defined in the metric,
instead of all of them, which obviously leads to wrong routing (or
non-matches)
@grobian
Copy link
Owner

grobian commented Dec 28, 2019

I think you want something to match the tags, which currently doesn't exist. What you experienced is actually a bug, the routing for the metrics was messed up. Tags by the current design are forwarded, but not handled in any way. If you give a description of what you need, I might be able to implement something.

@lexx-bright
Copy link
Contributor Author

I want to be able route metrics based on tags. For example:

match “env=prod”
send to ch_prod
stop
;
match “env=test”
send to ch_test
stop
;
And it does work until env tag is final.

@lexx-bright
Copy link
Contributor Author

Could you share your concerns about tags to be a part of a metric name? As a workaround I've added ";=" to the list of allowed symbols and got desired behavior. But could it potentially lead to some problems?

@grobian
Copy link
Owner

grobian commented Dec 30, 2019

The main concern is routing matters for consistent-hash based clusters. Metric a.b;foo=1 should be routed to the same node as a.b;foo=2, because routing should only take into account a.b. This is what the code does, basically ignore the part after ;, but still forward it to the destination. In the current code this goes wrong if there are multiple ';'s, because it picks the last, instead of the first. As a result, you can match all tags but the last.
What I think you need is something like

match * tag env=(dev|pre) send to dev_cluster stop;
match * tag env=prod customer=A send to prodA_cluster stop;
match * tag env=prod send to prod_cluster stop;

Now I just made up that syntax. Idea is that you can match the tag names, and their values via regex, to control the routing. Alternative is something like:

match-full env=prod send to prod_cluster stop;

where it would consider the full input string up to the first space, as you've been doing now. The latter approach is likely easier to implement on this end.

msaf1980 pushed a commit to msaf1980/carbon-c-relay that referenced this issue Jan 20, 2020
a logic error would only exlude the last tag defined in the metric,
instead of all of them, which obviously leads to wrong routing (or
non-matches)
msaf1980 pushed a commit to msaf1980/carbon-c-relay that referenced this issue Jan 22, 2020
a logic error would only exlude the last tag defined in the metric,
instead of all of them, which obviously leads to wrong routing (or
non-matches)
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