mirror of https://github.com/grafana/grafana
Alerting: In migration improve deduplication of title and group (#78351)
* Alerting: In migration improve deduplication of title and group This change improves alert titles generated in the legacy migration that occur when we need to deduplicate titles. Now when duplicate titles are detected we will first attempt to append a sequential index, falling back to a random uid if none are unique within 10 attempts. This should cause shorter and more easily readable deduplicated titles in most cases. In addition, groups are no longer deduplicated. Instead we set them to a combination of truncated dashboard name and humanized alert frequency. This way, alerts from the same dashboard share a group if they have the same evaluation interval. In the event that truncation causes overlap, it won't be a big issue as all alerts will still be in a group with the correct evaluation interval.pull/78830/head
Matthew Jacobson
2 years ago
committed by
GitHub
parent
773e0680c5
commit
2b51f0e263
@ -0,0 +1,104 @@ |
||||
package models |
||||
|
||||
import ( |
||||
"fmt" |
||||
"strings" |
||||
|
||||
"github.com/grafana/grafana/pkg/util" |
||||
) |
||||
|
||||
// MaxDeduplicationAttempts is the maximum number of attempts to try to deduplicate a string using any
|
||||
// individual method, such as sequential index suffixes or uids.
|
||||
const MaxDeduplicationAttempts = 10 |
||||
|
||||
// Deduplicator is a utility for deduplicating strings. It keeps track of the strings it has seen and appends a unique
|
||||
// suffix to strings that have already been seen. It can optionally truncate strings before appending the suffix to
|
||||
// ensure that the resulting string is not longer than maxLen.
|
||||
// This implementation will first try to deduplicate via a sequential index suffix of the form " #2", " #3", etc.
|
||||
// If after MaxIncrementDeduplicationAttempts attempts it still cannot find a unique string, it will generate a new
|
||||
// unique uid and append that to the string.
|
||||
type Deduplicator struct { |
||||
set map[string]int |
||||
caseInsensitive bool |
||||
maxLen int |
||||
uidGenerator func() string |
||||
} |
||||
|
||||
// NewDeduplicator creates a new deduplicator.
|
||||
// caseInsensitive determines whether the string comparison should be case-insensitive.
|
||||
// maxLen determines the maximum length of deduplicated strings. If the deduplicated string would be longer than
|
||||
// maxLen, it will be truncated.
|
||||
func NewDeduplicator(caseInsensitive bool, maxLen int) *Deduplicator { |
||||
return &Deduplicator{ |
||||
set: make(map[string]int), |
||||
caseInsensitive: caseInsensitive, |
||||
maxLen: maxLen, |
||||
uidGenerator: util.GenerateShortUID, |
||||
} |
||||
} |
||||
|
||||
// Deduplicate returns a unique string based on the given base string. If the base string has not already been seen by
|
||||
// this deduplicator, it will be returned as-is. If the base string has already been seen, a unique suffix will be
|
||||
// appended to the base string to make it unique.
|
||||
func (s *Deduplicator) Deduplicate(base string) (string, error) { |
||||
if s.maxLen > 0 && len(base) > s.maxLen { |
||||
base = base[:s.maxLen] |
||||
} |
||||
cnt, ok := s.contains(base) |
||||
if !ok { |
||||
s.add(base, 0) |
||||
return base, nil |
||||
} |
||||
|
||||
// Start at 2, so we get a, a_2, a_3, etc.
|
||||
for i := 2 + cnt; i < 2+cnt+MaxDeduplicationAttempts; i++ { |
||||
dedup := s.appendSuffix(base, fmt.Sprintf(" #%d", i)) |
||||
if _, ok := s.contains(dedup); !ok { |
||||
s.add(dedup, 0) |
||||
return dedup, nil |
||||
} |
||||
} |
||||
|
||||
// None of the simple suffixes worked, so we generate a new uid. We try a few times, just in case, but this should
|
||||
// almost always create a unique string on the first try.
|
||||
for i := 0; i < MaxDeduplicationAttempts; i++ { |
||||
dedup := s.appendSuffix(base, fmt.Sprintf("_%s", s.uidGenerator())) |
||||
if _, ok := s.contains(dedup); !ok { |
||||
s.add(dedup, 0) |
||||
return dedup, nil |
||||
} |
||||
} |
||||
|
||||
return "", fmt.Errorf("failed to deduplicate %q", base) |
||||
} |
||||
|
||||
// contains checks whether the given string has already been seen by this deduplicator.
|
||||
func (s *Deduplicator) contains(u string) (int, bool) { |
||||
dedup := u |
||||
if s.caseInsensitive { |
||||
dedup = strings.ToLower(dedup) |
||||
} |
||||
if s.maxLen > 0 && len(dedup) > s.maxLen { |
||||
dedup = dedup[:s.maxLen] |
||||
} |
||||
cnt, seen := s.set[dedup] |
||||
return cnt, seen |
||||
} |
||||
|
||||
// appendSuffix appends the given suffix to the given base string. If the resulting string would be longer than maxLen,
|
||||
// the base string will be truncated.
|
||||
func (s *Deduplicator) appendSuffix(base, suffix string) string { |
||||
if s.maxLen > 0 && len(base)+len(suffix) > s.maxLen { |
||||
return base[:s.maxLen-len(suffix)] + suffix |
||||
} |
||||
return base + suffix |
||||
} |
||||
|
||||
// add adds the given string to the deduplicator.
|
||||
func (s *Deduplicator) add(uid string, cnt int) { |
||||
dedup := uid |
||||
if s.caseInsensitive { |
||||
dedup = strings.ToLower(dedup) |
||||
} |
||||
s.set[dedup] = cnt |
||||
} |
||||
@ -0,0 +1,149 @@ |
||||
package models |
||||
|
||||
import ( |
||||
"fmt" |
||||
"strings" |
||||
"testing" |
||||
|
||||
"github.com/stretchr/testify/require" |
||||
|
||||
"github.com/grafana/grafana/pkg/util" |
||||
) |
||||
|
||||
func TestDeduplicator(t *testing.T) { |
||||
tc := []struct { |
||||
name string |
||||
maxLen int |
||||
caseInsensitive bool |
||||
input []string |
||||
expected []string |
||||
expectedState map[string]struct{} |
||||
}{ |
||||
{ |
||||
name: "when case insensitive, it deduplicates case-insensitively", |
||||
caseInsensitive: true, |
||||
input: []string{"a", "A", "B", "b", "a", "A", "b", "B"}, |
||||
expected: []string{"a", "A #2", "B", "b #2", "a #3", "A #4", "b #3", "B #4"}, |
||||
}, |
||||
{ |
||||
name: "when case sensitive, it deduplicates case-sensitively", |
||||
caseInsensitive: false, |
||||
input: []string{"a", "A", "B", "b", "a", "A", "b", "B"}, |
||||
expected: []string{"a", "A", "B", "b", "a #2", "A #2", "b #2", "B #2"}, |
||||
}, |
||||
{ |
||||
name: "when maxLen is 0, it does not truncate", |
||||
maxLen: 0, |
||||
input: []string{strings.Repeat("a", 1000), strings.Repeat("a", 1000)}, |
||||
expected: []string{strings.Repeat("a", 1000), strings.Repeat("a", 1000) + " #2"}, |
||||
}, |
||||
{ |
||||
name: "when maxLen is > 0, it truncates - caseInsensitive", |
||||
caseInsensitive: true, |
||||
maxLen: 10, |
||||
input: []string{strings.Repeat("A", 15), strings.Repeat("a", 15), strings.Repeat("A", 15)}, |
||||
expected: []string{strings.Repeat("A", 10), strings.Repeat("a", 7) + " #2", strings.Repeat("A", 7) + " #3"}, |
||||
}, |
||||
{ |
||||
name: "when maxLen is > 0, it truncates - caseSensitive", |
||||
maxLen: 10, |
||||
input: []string{strings.Repeat("A", 15), strings.Repeat("a", 15), strings.Repeat("A", 15)}, |
||||
expected: []string{strings.Repeat("A", 10), strings.Repeat("a", 10), strings.Repeat("A", 7) + " #2"}, |
||||
}, |
||||
{ |
||||
name: "when truncate causes collision, it deduplicates - caseInsensitive", |
||||
caseInsensitive: true, |
||||
maxLen: 10, |
||||
input: []string{strings.Repeat("A", 15), strings.Repeat("a", 10), strings.Repeat("b", 15), strings.Repeat("B", 10)}, |
||||
expected: []string{strings.Repeat("A", 10), strings.Repeat("a", 7) + " #2", strings.Repeat("b", 10), strings.Repeat("B", 7) + " #2"}, |
||||
}, |
||||
{ |
||||
name: "when truncate causes collision, it deduplicates - caseSensitive", |
||||
maxLen: 10, |
||||
input: []string{strings.Repeat("A", 15), strings.Repeat("A", 10)}, |
||||
expected: []string{strings.Repeat("A", 10), strings.Repeat("A", 7) + " #2"}, |
||||
}, |
||||
{ |
||||
name: "when deduplicate causes collision, it deduplicates - caseInsensitive", |
||||
caseInsensitive: true, |
||||
maxLen: 10, |
||||
input: []string{"A", "a", "a #2", "b", "B", "B #2"}, |
||||
expected: []string{"A", "a #2", "a #2 #2", "b", "B #2", "B #2 #2"}, |
||||
}, |
||||
{ |
||||
name: "when deduplicate causes collision, it deduplicates - caseSensitive", |
||||
maxLen: 10, |
||||
input: []string{"a", "a", "a #2", "b", "b", "b #2"}, |
||||
expected: []string{"a", "a #2", "a #2 #2", "b", "b #2", "b #2 #2"}, |
||||
}, |
||||
{ |
||||
name: "when deduplicate causes collision, it finds next available increment - caseInsensitive", |
||||
caseInsensitive: true, |
||||
maxLen: 10, |
||||
input: []string{"a", "A #2", "a #3", "A #4", "a #5", "A #6", "a #7", "A #8", "a #9", "A #10", "a"}, |
||||
expected: []string{"a", "A #2", "a #3", "A #4", "a #5", "A #6", "a #7", "A #8", "a #9", "A #10", "a #11"}, |
||||
}, |
||||
{ |
||||
name: "when deduplicate causes collision, it finds next available increment - caseSensitive", |
||||
maxLen: 10, |
||||
input: []string{"a", "a #2", "a #3", "a #4", "a #5", "a #6", "a #7", "a #8", "a #9", "a #10", "a"}, |
||||
expected: []string{"a", "a #2", "a #3", "a #4", "a #5", "a #6", "a #7", "a #8", "a #9", "a #10", "a #11"}, |
||||
}, |
||||
{ |
||||
name: "when deduplicate causes collision enough times, it deduplicates with uid - caseInsensitive", |
||||
caseInsensitive: true, |
||||
maxLen: 10, |
||||
input: []string{"a", "A #2", "a #3", "A #4", "a #5", "A #6", "a #7", "A #8", "a #9", "A #10", "a #11", "A"}, |
||||
expected: []string{"a", "A #2", "a #3", "A #4", "a #5", "A #6", "a #7", "A #8", "a #9", "A #10", "a #11", "A_uid-1"}, |
||||
}, |
||||
{ |
||||
name: "when deduplicate causes collision enough times, it deduplicates with uid - caseSensitive", |
||||
maxLen: 10, |
||||
input: []string{"a", "a #2", "a #3", "a #4", "a #5", "a #6", "a #7", "a #8", "a #9", "a #10", "a #11", "a"}, |
||||
expected: []string{"a", "a #2", "a #3", "a #4", "a #5", "a #6", "a #7", "a #8", "a #9", "a #10", "a #11", "a_uid-1"}, |
||||
}, |
||||
} |
||||
|
||||
for _, tt := range tc { |
||||
t.Run(tt.name, func(t *testing.T) { |
||||
inc := 0 |
||||
mockUidGenerator := func() string { |
||||
inc++ |
||||
return fmt.Sprintf("uid-%d", inc) |
||||
} |
||||
dedup := Deduplicator{ |
||||
set: make(map[string]int), |
||||
caseInsensitive: tt.caseInsensitive, |
||||
maxLen: tt.maxLen, |
||||
uidGenerator: mockUidGenerator, |
||||
} |
||||
out := make([]string, 0, len(tt.input)) |
||||
for _, in := range tt.input { |
||||
d, err := dedup.Deduplicate(in) |
||||
require.NoError(t, err) |
||||
out = append(out, d) |
||||
} |
||||
require.Equal(t, tt.expected, out) |
||||
}) |
||||
} |
||||
} |
||||
|
||||
func Test_shortUIDCaseInsensitiveConflicts(t *testing.T) { |
||||
s := Deduplicator{ |
||||
set: make(map[string]int), |
||||
caseInsensitive: true, |
||||
} |
||||
|
||||
// 10000 uids seems to be enough to cause a collision in almost every run if using util.GenerateShortUID directly.
|
||||
for i := 0; i < 10000; i++ { |
||||
s.add(util.GenerateShortUID(), 0) |
||||
} |
||||
|
||||
// check if any are case-insensitive duplicates.
|
||||
deduped := make(map[string]struct{}) |
||||
for k := range s.set { |
||||
deduped[strings.ToLower(k)] = struct{}{} |
||||
} |
||||
|
||||
require.Equal(t, len(s.set), len(deduped)) |
||||
} |
||||
Loading…
Reference in new issue