Skip to content

Commit

Permalink
item.get cache won't be hit if tags order are different (#1745)
Browse files Browse the repository at this point in the history
* cache won't be hit if tags order are not the same

* unit test for get items with tags
  • Loading branch information
iqbalaydrus authored Dec 4, 2023
1 parent 0f144f6 commit dfe360b
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 0 deletions.
8 changes: 8 additions & 0 deletions pkg/zabbix/methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package zabbix

import (
"context"
"sort"
"strconv"
"strings"

Expand Down Expand Up @@ -402,6 +403,13 @@ func (ds *Zabbix) GetAllItems(ctx context.Context, hostids []string, appids []st
}
tagsParams = append(tagsParams, tagParam)
}
// tags order should be handled for higher cache hit ratio
sort.Slice(tagsParams, func(i, j int) bool {
if tagsParams[i]["tag"] != tagsParams[j]["tag"] {
return tagsParams[i]["tag"] < tagsParams[j]["tag"]
}
return tagsParams[i]["value"] < tagsParams[j]["value"]
})
params["tags"] = tagsParams
params["evaltype"] = 2
}
Expand Down
53 changes: 53 additions & 0 deletions pkg/zabbix/zabbix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package zabbix

import (
"context"
"github.com/alexanderzobnin/grafana-zabbix/pkg/settings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -88,3 +89,55 @@ func TestNonCachedQuery(t *testing.T) {
result, _ = resp.String()
assert.Equal(t, "testNew", result)
}

func TestItemTagCache(t *testing.T) {
zabbixClient, _ := MockZabbixClient(
basicDatasourceInfo,
`{"result":[{"itemid":"1","name":"test1"}]}`,
200,
)
// tag filtering is on >= 54 version
zabbixClient.version = 64
zabbixClient.settings.AuthType = settings.AuthTypeToken
zabbixClient.api.SetAuth("test")
items, err := zabbixClient.GetAllItems(
context.Background(),
nil,
nil,
"num",
false,
"Application: test, interface: test",
)

assert.NoError(t, err)
if assert.Len(t, items, 1) {
item := items[0]
assert.Equal(t, "1", item.ID)
assert.Equal(t, "test1", item.Name)
}

zabbixClient, _ = MockZabbixClientResponse(
zabbixClient,
// intentionally different response to test if the cache hits
`{"result":[{"itemid":"2","name":"test2"}]}`,
200,
)
zabbixClient.api.SetAuth("test")
items, err = zabbixClient.GetAllItems(
context.Background(),
nil,
nil,
"num",
false,
// change tag order
"interface: test, Application: test",
)

assert.NoError(t, err)
if assert.Len(t, items, 1) {
item := items[0]
// test if it still uses cached response
assert.Equal(t, "1", item.ID)
assert.Equal(t, "test1", item.Name)
}
}

0 comments on commit dfe360b

Please sign in to comment.