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

Adds backtick for the quoted string token lexer. #2095

Merged
merged 4 commits into from
May 19, 2020

Conversation

cyriltovena
Copy link
Contributor

This allows to pass unescaped string to Loki.

For example we now support this :

{name="cassandra"} |~ `error=\w+`
{name!~`mysql-\d+`}

As you can see no escape is required when using ` quoted strings.

Fixes #2016 #2061

Signed-off-by: Cyril Tovena cyril.tovena@gmail.com

* Fix unit conversion in stats log.

🤦

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

* Fix the tests because you know ! you know ?

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

wip

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
This allows to pass unescaped string to Loki.

For example we now support this :

`` {name="cassandra"} |~  `error=\w+` ``
`` {name!~`mysql-\d+`} ``

As you can see no escape is required when using ` quoted strings.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #2095 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2095      +/-   ##
==========================================
+ Coverage   61.24%   61.26%   +0.02%     
==========================================
  Files         146      146              
  Lines       11187    11187              
==========================================
+ Hits         6851     6854       +3     
+ Misses       3788     3787       -1     
+ Partials      548      546       -2     
Impacted Files Coverage Δ
pkg/logql/lex.go 100.00% <100.00%> (ø)
pkg/promtail/targets/tailer.go 73.86% <0.00%> (ø)
pkg/promtail/targets/filetarget.go 70.48% <0.00%> (+1.80%) ⬆️

@@ -20,6 +20,25 @@ func TestParse(t *testing.T) {
exp Expr
err error
}{
{
// raw string
in: "count_over_time({foo=~`bar\\w+`}[12h] |~ `error\\`)",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be:

"count_over_time({foo=~`bar\w+`}[12h] |~ `error\`)",

With only a single backlash because it's inside the new backticks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, unless you have an alternative we can't escape backtick here, basically foo |~ error`` is interpreted as "foo |~" + identifier(error) + empty raw string . So I need to use " quoted query. and that requires escape. I call this go quote inception. Not sure I explained this correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, ok, i was more just asking if this was a mistake or if it was a requirement of how to get the value in the test... no worries LGTM

Copy link
Collaborator

@slim-bean slim-bean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cyriltovena cyriltovena merged commit f9a2c43 into grafana:master May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"\." -> "invalid char escape"
3 participants