mirror of https://github.com/grafana/loki
Fix a bug with metric queries and label_format. (#3263)
* Fix a bug whith metric queries and label_format. In my previous PR where I introduced hints, I forgot to implement label format label required. This would cause label format label to be skipped during parsing. I've also took the opportunity to improve the code. Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Refactor and add more tests. Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>pull/3271/head
Cyril Tovena
4 years ago
committed by
GitHub
parent
287244d110
commit
e8cce09de2
@ -0,0 +1,86 @@ |
||||
package log |
||||
|
||||
import ( |
||||
"strings" |
||||
) |
||||
|
||||
var noParserHints = &parserHint{} |
||||
|
||||
// ParserHint are hints given to LogQL parsers.
|
||||
// This is specially useful for parser that extract implicitly all possible label keys.
|
||||
// This is used only within metric queries since it's rare that you need all label keys.
|
||||
// For example in the following expression:
|
||||
//
|
||||
// sum by (status_code) (rate({app="foo"} | json [5m]))
|
||||
//
|
||||
// All we need to extract is the status_code in the json parser.
|
||||
type ParserHint interface { |
||||
// Tells if a label with the given key should be extracted.
|
||||
ShouldExtract(key string) bool |
||||
// Tells if there's any hint that start with the given prefix.
|
||||
// This allows to speed up key searching in nested structured like json.
|
||||
ShouldExtractPrefix(prefix string) bool |
||||
// Tells if we should not extract any labels.
|
||||
// For example in :
|
||||
// sum(rate({app="foo"} | json [5m]))
|
||||
// We don't need to extract any labels from the log line.
|
||||
NoLabels() bool |
||||
} |
||||
|
||||
type parserHint struct { |
||||
noLabels bool |
||||
requiredLabels []string |
||||
} |
||||
|
||||
func (p *parserHint) ShouldExtract(key string) bool { |
||||
if len(p.requiredLabels) == 0 { |
||||
return true |
||||
} |
||||
for _, l := range p.requiredLabels { |
||||
if l == key { |
||||
return true |
||||
} |
||||
} |
||||
return false |
||||
} |
||||
|
||||
func (p *parserHint) ShouldExtractPrefix(prefix string) bool { |
||||
if len(p.requiredLabels) == 0 { |
||||
return true |
||||
} |
||||
for _, l := range p.requiredLabels { |
||||
if strings.HasPrefix(l, prefix) { |
||||
return true |
||||
} |
||||
} |
||||
|
||||
return false |
||||
} |
||||
|
||||
func (p *parserHint) NoLabels() bool { |
||||
return p.noLabels |
||||
} |
||||
|
||||
// newParserHint creates a new parser hint using the list of labels that are seen and required in a query.
|
||||
func newParserHint(requiredLabelNames, groups []string, without, noLabels bool, metricLabelName string) *parserHint { |
||||
if len(groups) > 0 { |
||||
requiredLabelNames = append(requiredLabelNames, groups...) |
||||
} |
||||
if metricLabelName != "" { |
||||
requiredLabelNames = append(requiredLabelNames, metricLabelName) |
||||
} |
||||
requiredLabelNames = uniqueString(requiredLabelNames) |
||||
if noLabels { |
||||
if len(requiredLabelNames) > 0 { |
||||
return &parserHint{requiredLabels: requiredLabelNames} |
||||
} |
||||
return &parserHint{noLabels: true} |
||||
} |
||||
// we don't know what is required when a without clause is used.
|
||||
// Same is true when there's no grouping.
|
||||
// no hints available then.
|
||||
if without || len(groups) == 0 { |
||||
return noParserHints |
||||
} |
||||
return &parserHint{requiredLabels: requiredLabelNames} |
||||
} |
||||
@ -0,0 +1,118 @@ |
||||
// uses log_test package to avoid circular dependency between log and logql package.
|
||||
package log_test |
||||
|
||||
import ( |
||||
"testing" |
||||
|
||||
"github.com/prometheus/prometheus/pkg/labels" |
||||
"github.com/stretchr/testify/require" |
||||
|
||||
"github.com/grafana/loki/pkg/logql" |
||||
) |
||||
|
||||
var jsonLine = []byte(`{ |
||||
"remote_user": "foo", |
||||
"upstream_addr": "10.0.0.1:80", |
||||
"protocol": "HTTP/2.0", |
||||
"request": { |
||||
"time": "30.001", |
||||
"method": "POST", |
||||
"host": "foo.grafana.net", |
||||
"uri": "/rpc/v2/stage", |
||||
"size": "101" |
||||
}, |
||||
"response": { |
||||
"status": 204, |
||||
"latency_seconds": "30.001" |
||||
} |
||||
}`) |
||||
|
||||
func Test_ParserHints(t *testing.T) { |
||||
lbs := labels.Labels{{Name: "app", Value: "nginx"}} |
||||
|
||||
t.Parallel() |
||||
for _, tt := range []struct { |
||||
expr string |
||||
expectOk bool |
||||
expectVal float64 |
||||
expectLbs string |
||||
}{ |
||||
{ |
||||
`rate({app="nginx"} | json | response_status = 204 [1m])`, |
||||
true, |
||||
1.0, |
||||
`{app="nginx", protocol="HTTP/2.0", remote_user="foo", request_host="foo.grafana.net", request_method="POST", request_size="101", request_time="30.001", request_uri="/rpc/v2/stage", response_latency_seconds="30.001", response_status="204", upstream_addr="10.0.0.1:80"}`, |
||||
}, |
||||
{ |
||||
`sum without (request_host,app) (rate({app="nginx"} | json | __error__="" | response_status = 204 [1m]))`, |
||||
true, |
||||
1.0, |
||||
`{protocol="HTTP/2.0", remote_user="foo", request_method="POST", request_size="101", request_time="30.001", request_uri="/rpc/v2/stage", response_latency_seconds="30.001", response_status="204", upstream_addr="10.0.0.1:80"}`, |
||||
}, |
||||
{ |
||||
`sum by (request_host,app) (rate({app="nginx"} | json | __error__="" | response_status = 204 [1m]))`, |
||||
true, |
||||
1.0, |
||||
`{app="nginx", request_host="foo.grafana.net"}`, |
||||
}, |
||||
{ |
||||
`sum(rate({app="nginx"} | json | __error__="" | response_status = 204 [1m]))`, |
||||
true, |
||||
1.0, |
||||
`{}`, |
||||
}, |
||||
{ |
||||
`sum(rate({app="nginx"} | json [1m]))`, |
||||
true, |
||||
1.0, |
||||
`{}`, |
||||
}, |
||||
{ |
||||
`sum(rate({app="nginx"} | json | unwrap response_latency_seconds [1m]))`, |
||||
true, |
||||
30.001, |
||||
`{}`, |
||||
}, |
||||
{ |
||||
`sum(rate({app="nginx"} | json | response_status = 204 | unwrap response_latency_seconds [1m]))`, |
||||
true, |
||||
30.001, |
||||
`{}`, |
||||
}, |
||||
{ |
||||
`sum by (request_host,app)(rate({app="nginx"} | json | response_status = 204 and remote_user = "foo" | unwrap response_latency_seconds [1m]))`, |
||||
true, |
||||
30.001, |
||||
`{app="nginx", request_host="foo.grafana.net"}`, |
||||
}, |
||||
{ |
||||
`rate({app="nginx"} | json | response_status = 204 | unwrap response_latency_seconds [1m])`, |
||||
true, |
||||
30.001, |
||||
`{app="nginx", protocol="HTTP/2.0", remote_user="foo", request_host="foo.grafana.net", request_method="POST", request_size="101", request_time="30.001", request_uri="/rpc/v2/stage", response_status="204", upstream_addr="10.0.0.1:80"}`, |
||||
}, |
||||
{ |
||||
`sum without (request_host,app)(rate({app="nginx"} | json | response_status = 204 | unwrap response_latency_seconds [1m]))`, |
||||
true, |
||||
30.001, |
||||
`{protocol="HTTP/2.0", remote_user="foo", request_method="POST", request_size="101", request_time="30.001", request_uri="/rpc/v2/stage", response_status="204", upstream_addr="10.0.0.1:80"}`, |
||||
}, |
||||
} { |
||||
tt := tt |
||||
t.Run(tt.expr, func(t *testing.T) { |
||||
t.Parallel() |
||||
expr, err := logql.ParseSampleExpr(tt.expr) |
||||
require.NoError(t, err) |
||||
ex, err := expr.Extractor() |
||||
require.NoError(t, err) |
||||
v, lbsRes, ok := ex.ForStream(lbs).Process(jsonLine) |
||||
var lbsResString string |
||||
if lbsRes != nil { |
||||
lbsResString = lbsRes.String() |
||||
} |
||||
require.Equal(t, tt.expectOk, ok) |
||||
require.Equal(t, tt.expectVal, v) |
||||
require.Equal(t, tt.expectLbs, lbsResString) |
||||
}) |
||||
} |
||||
} |
||||
Loading…
Reference in new issue