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
Alerting: No longer index state history log streams by instance labels #65474
Conversation
… of straight logQL
Hello @alexweav!
Please, if the current pull request addresses a bug fix, label it with the |
type Selector struct { | ||
// Label to Select | ||
Label string | ||
Op Operator | ||
// Value that is expected | ||
Value string | ||
} | ||
|
||
func NewSelector(label, op, value string) (Selector, error) { | ||
if !isValidOperator(op) { | ||
return Selector{}, fmt.Errorf("'%s' is not a valid query operator", op) | ||
} | ||
return Selector{Label: label, Op: Operator(op), Value: value}, nil | ||
} | ||
|
||
func selectorString(selectors []Selector) string { | ||
if len(selectors) == 0 { | ||
return "{}" | ||
} | ||
// Build the query selector. | ||
query := "" | ||
for _, s := range selectors { | ||
query += fmt.Sprintf("%s%s%q,", s.Label, s.Op, s.Value) | ||
} | ||
// Remove the last comma, as we append one to every selector. | ||
query = query[:len(query)-1] | ||
return "{" + query + "}" | ||
} | ||
|
||
func isValidOperator(op string) bool { | ||
switch op { | ||
case "=", "!=", "=~", "!~": | ||
return true | ||
} | ||
return false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copied verbatim from loki_http.go
t.Run("selector string", func(t *testing.T) { | ||
selectors := []Selector{{"name", "=", "Bob"}, {"age", "=~", "30"}} | ||
expected := "{name=\"Bob\",age=~\"30\"}" | ||
result := selectorString(selectors) | ||
require.Equal(t, expected, result) | ||
|
||
selectors = []Selector{} | ||
expected = "{}" | ||
result = selectorString(selectors) | ||
require.Equal(t, expected, result) | ||
}) | ||
|
||
t.Run("new selector", func(t *testing.T) { | ||
selector, err := NewSelector("label", "=", "value") | ||
require.NoError(t, err) | ||
require.Equal(t, "label", selector.Label) | ||
require.Equal(t, Eq, selector.Op) | ||
require.Equal(t, "value", selector.Value) | ||
|
||
selector, err = NewSelector("label", "invalid", "value") | ||
require.Error(t, err) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copied from below. not modified
require.Equal(t, exp, res[0].Stream) | ||
}) | ||
|
||
t.Run("groups streams based on combined labels", func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed any more. the type system now enforces this automatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-65474-to-v9.4.x origin/v9.4.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x a416100abc5f649ada57201cb855d79116f40c8e
# Push it to GitHub
git push --set-upstream origin backport-65474-to-v9.4.x
git switch main
# Remove the local backport branch
git branch -D backport-65474-to-v9.4.x Then, create a pull request where the |
#65474) * Remove private labels * No longer index by instance labels * Labels are now invariant, only build them once * Remove bucketing since everything is in a single stream * Refactor statesToStreams to only return a single unified log stream * Don't query on labels that no longer exist * Move selector logic to loki layer, genericize client to work in terms of straight logQL * Add support for line-level label filters in query * Combine existing selector tests for better parallelism * Tests for logQL construction * Underscore instead of dot for unwrapping labels in logql (cherry picked from commit a416100)
#65563) Alerting: No longer index state history log streams by instance labels (#65474) * Remove private labels * No longer index by instance labels * Labels are now invariant, only build them once * Remove bucketing since everything is in a single stream * Refactor statesToStreams to only return a single unified log stream * Don't query on labels that no longer exist * Move selector logic to loki layer, genericize client to work in terms of straight logQL * Add support for line-level label filters in query * Combine existing selector tests for better parallelism * Tests for logQL construction * Underscore instead of dot for unwrapping labels in logql (cherry picked from commit a416100)
What is this feature?
This is a continuation of #65403 where we stop merging instance labels into the log stream labels. Going forward, log streams correspond 1:1 to rules.
Queries for labels will rely on the functionality added in #65403, they now filter the log line itself rather than labels.
Why do we need this feature?
Which issue(s) does this PR fix?:
n/a
Special notes for your reviewer:
Consider reviewing this PR commit-by-commit
Please check that: