diff --git a/CHANGELOG.md b/CHANGELOG.md index 1318e23e54..2e61325822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ We use *breaking* word for marking changes that are not backward compatible (rel - [#2728](https://github.com/thanos-io/thanos/pull/2728) Query: Fixed panics when using larger number of replica labels with short series label sets. - [#2787](https://github.com/thanos-io/thanos/pull/2787) Update Prometheus mod to pull in prometheus/prometheus#7414. - [#2807](https://github.com/thanos-io/thanos/pull/2807) Store: decreased memory allocations while querying block's index. +- [#2809](https://github.com/thanos-io/thanos/pull/2809) Query: `/api/v1/stores` now guarantees to return a string in the `lastError` field. ### Changed diff --git a/pkg/query/storeset.go b/pkg/query/storeset.go index b2db21dc05..654cee494b 100644 --- a/pkg/query/storeset.go +++ b/pkg/query/storeset.go @@ -5,6 +5,7 @@ package query import ( "context" + "encoding/json" "fmt" "sort" "strings" @@ -47,10 +48,26 @@ type RuleSpec interface { Addr() string } +// stringError forces the error to be a string +// when marshalled into a JSON. +type stringError struct { + originalErr error +} + +// MarshalJSON marshals the error into a string form. +func (e *stringError) MarshalJSON() ([]byte, error) { + return json.Marshal(e.originalErr.Error()) +} + +// Error returns the original underlying error. +func (e *stringError) Error() string { + return e.originalErr.Error() +} + type StoreStatus struct { Name string `json:"name"` LastCheck time.Time `json:"lastCheck"` - LastError error `json:"lastError"` + LastError *stringError `json:"lastError"` LabelSets []storepb.LabelSet `json:"labelSets"` StoreType component.StoreAPI `json:"-"` MinTime int64 `json:"minTime"` @@ -510,7 +527,7 @@ func (s *StoreSet) updateStoreStatus(store *storeRef, err error) { status = *prev } - status.LastError = err + status.LastError = &stringError{originalErr: err} if err == nil { status.LastCheck = time.Now() diff --git a/pkg/query/storeset_test.go b/pkg/query/storeset_test.go index 273a2e086c..58813b2dce 100644 --- a/pkg/query/storeset_test.go +++ b/pkg/query/storeset_test.go @@ -5,6 +5,7 @@ package query import ( "context" + "encoding/json" "fmt" "math" "net" @@ -12,6 +13,7 @@ import ( "time" "github.com/fortytw2/leaktest" + "github.com/pkg/errors" "github.com/thanos-io/thanos/pkg/component" "github.com/thanos-io/thanos/pkg/store" "github.com/thanos-io/thanos/pkg/store/storepb" @@ -660,7 +662,7 @@ func TestQuerierStrict(t *testing.T) { testutil.Equals(t, 2, len(storeSet.stores), "two static clients must remain available") testutil.Equals(t, curMin, storeSet.stores[staticStoreAddr].minTime, "minimum time reported by the store node is different") testutil.Equals(t, curMax, storeSet.stores[staticStoreAddr].maxTime, "minimum time reported by the store node is different") - testutil.NotOk(t, storeSet.storeStatuses[staticStoreAddr].LastError) + testutil.NotOk(t, storeSet.storeStatuses[staticStoreAddr].LastError.originalErr) } func TestStoreSet_Update_Rules(t *testing.T) { @@ -778,3 +780,28 @@ func TestStoreSet_Update_Rules(t *testing.T) { }) } } + +type weirdError struct { + originalErr error +} + +// MarshalJSON marshals the error and returns weird results. +func (e *weirdError) MarshalJSON() ([]byte, error) { + return json.Marshal(map[string]string{}) +} + +// Error returns the original, underlying string. +func (e *weirdError) Error() string { + return e.originalErr.Error() +} + +func TestStringError(t *testing.T) { + weirdError := &weirdError{originalErr: errors.New("test")} + properErr := &stringError{originalErr: weirdError} + storestatusMock := map[string]error{} + storestatusMock["weird"] = weirdError + storestatusMock["proper"] = properErr + b, err := json.Marshal(storestatusMock) + testutil.Ok(t, err) + testutil.Equals(t, []byte(`{"proper":"test","weird":{}}`), b, "expected to get proper results") +}