-
Notifications
You must be signed in to change notification settings - Fork 69
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
chore: Add option to hide tokens from ring statuspage #633
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,9 +30,12 @@ var defaultPageTemplate = template.Must(template.New("webpage").Funcs(template.F | |
}).Parse(defaultPageContent)) | ||
|
||
type httpResponse struct { | ||
Ingesters []ingesterDesc `json:"shards"` | ||
Now time.Time `json:"now"` | ||
ShowTokens bool `json:"-"` | ||
Ingesters []ingesterDesc `json:"shards"` | ||
Now time.Time `json:"now"` | ||
// ShowTokens indicates whether the Show Tokens button is clicked. | ||
ShowTokens bool `json:"-"` | ||
// DisableTokens hides the concept of tokens entirely in the page, across all elements. | ||
DisableTokens bool `json:"-"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I switched the name to I still feel |
||
} | ||
|
||
type ingesterDesc struct { | ||
|
@@ -57,12 +60,14 @@ type ringAccess interface { | |
type ringPageHandler struct { | ||
r ringAccess | ||
heartbeatTimeout time.Duration | ||
disableTokens bool | ||
} | ||
|
||
func newRingPageHandler(r ringAccess, heartbeatTimeout time.Duration) *ringPageHandler { | ||
func newRingPageHandler(r ringAccess, heartbeatTimeout time.Duration, disableTokens bool) *ringPageHandler { | ||
return &ringPageHandler{ | ||
r: r, | ||
heartbeatTimeout: heartbeatTimeout, | ||
disableTokens: disableTokens, | ||
} | ||
} | ||
|
||
|
@@ -132,9 +137,10 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { | |
tokensParam := req.URL.Query().Get("tokens") | ||
|
||
renderHTTPResponse(w, httpResponse{ | ||
Ingesters: ingesters, | ||
Now: now, | ||
ShowTokens: tokensParam == "true", | ||
Ingesters: ingesters, | ||
Now: now, | ||
ShowTokens: tokensParam == "true", | ||
DisableTokens: h.disableTokens, | ||
}, defaultPageTemplate, req) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
package ring | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"net/http" | ||
"net/http/httptest" | ||
"regexp" | ||
"strings" | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestRingPageHandler_handle(t *testing.T) { | ||
now := time.Now() | ||
ring := fakeRingAccess{ | ||
desc: &Desc{ | ||
Ingesters: map[string]InstanceDesc{ | ||
"1": { | ||
Zone: "zone-a", | ||
State: ACTIVE, | ||
Addr: "addr-a", | ||
Timestamp: now.Unix(), | ||
Tokens: []uint32{1000000, 3000000, 6000000}, | ||
}, | ||
"2": { | ||
Zone: "zone-b", | ||
State: ACTIVE, | ||
Addr: "addr-b", | ||
Timestamp: now.Unix(), | ||
Tokens: []uint32{2000000, 4000000, 5000000, 7000000}, | ||
}, | ||
}, | ||
}, | ||
} | ||
handler := newRingPageHandler(&ring, 10*time.Second, false) | ||
|
||
t.Run("displays instance info", func(t *testing.T) { | ||
recorder := httptest.NewRecorder() | ||
handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) | ||
|
||
assert.Equal(t, http.StatusOK, recorder.Code) | ||
assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) | ||
|
||
assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
"<td>", "1", "</td>", | ||
"<td>", "zone-a", "</td>", | ||
"<td>", "ACTIVE", "</td>", | ||
"<td>", "addr-a", "</td>", | ||
}, `\s*`))), recorder.Body.String()) | ||
|
||
assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
Comment on lines
+52
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I skipped asserting a couple columns here, namely the ones containing heartbeat timestamps and such. Asserting on these full-precision times felt like a very high vector for test intermittency - plus, it's not really the focus of this PR. |
||
"<td>", "3", "</td>", | ||
"<td>", "100%", "</td>", | ||
}, `\s*`))), recorder.Body.String()) | ||
|
||
assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
"<td>", "2", "</td>", | ||
"<td>", "zone-b", "</td>", | ||
"<td>", "ACTIVE", "</td>", | ||
"<td>", "addr-b", "</td>", | ||
}, `\s*`))), recorder.Body.String()) | ||
|
||
assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
"<td>", "4", "</td>", | ||
"<td>", "100%", "</td>", | ||
}, `\s*`))), recorder.Body.String()) | ||
}) | ||
|
||
t.Run("displays Show Tokens button by default", func(t *testing.T) { | ||
recorder := httptest.NewRecorder() | ||
handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) | ||
|
||
assert.Equal(t, http.StatusOK, recorder.Code) | ||
assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) | ||
|
||
assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
`<input type="button" value="Show Tokens" onclick="window.location.href = '\?tokens=true'"/>`, | ||
}, `\s*`))), recorder.Body.String()) | ||
}) | ||
|
||
t.Run("displays tokens when Show Tokens is enabled", func(t *testing.T) { | ||
recorder := httptest.NewRecorder() | ||
handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring?tokens=true", nil)) | ||
|
||
assert.Equal(t, http.StatusOK, recorder.Code) | ||
assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) | ||
|
||
assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
`<input type="button" value="Hide Tokens" onclick="window.location.href = '\?tokens=false' "/>`, | ||
}, `\s*`))), recorder.Body.String()) | ||
|
||
assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
"<h2>", "Instance: 1", "</h2>", | ||
"<p>", "Tokens:<br/>", "1000000", "3000000", "6000000", "</p>", | ||
}, `\s*`))), recorder.Body.String()) | ||
|
||
assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
"<h2>", "Instance: 2", "</h2>", | ||
"<p>", "Tokens:<br/>", "2000000", "4000000", "5000000", "7000000", "</p>", | ||
}, `\s*`))), recorder.Body.String()) | ||
}) | ||
|
||
tokenDisabledHandler := newRingPageHandler(&ring, 10*time.Second, true) | ||
|
||
t.Run("hides token columns when tokens are disabled", func(t *testing.T) { | ||
recorder := httptest.NewRecorder() | ||
tokenDisabledHandler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) | ||
|
||
assert.Equal(t, http.StatusOK, recorder.Code) | ||
assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) | ||
|
||
assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
"<th>", "Tokens", "</th>", | ||
"<th>", "Ownership", "</th>", | ||
}, `\s*`))), recorder.Body.String()) | ||
|
||
assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
"<td>", "3", "</td>", | ||
"<td>", "100%", "</td>", | ||
}, `\s*`))), recorder.Body.String()) | ||
|
||
assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
"<td>", "4", "</td>", | ||
"<td>", "100%", "</td>", | ||
}, `\s*`))), recorder.Body.String()) | ||
}) | ||
|
||
t.Run("hides Show Tokens button when tokens are disabled", func(t *testing.T) { | ||
recorder := httptest.NewRecorder() | ||
tokenDisabledHandler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) | ||
|
||
assert.Equal(t, http.StatusOK, recorder.Code) | ||
assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) | ||
|
||
assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ | ||
`input type="button" value="Show Tokens"`, | ||
}, `\s*`))), recorder.Body.String()) | ||
}) | ||
} | ||
|
||
type fakeRingAccess struct { | ||
desc *Desc | ||
} | ||
|
||
func (f *fakeRingAccess) getRing(context.Context) (*Desc, error) { | ||
return f.desc, nil | ||
} | ||
|
||
func (f *fakeRingAccess) casRing(_ context.Context, _ func(in interface{}) (out interface{}, retry bool, err error)) error { | ||
return nil | ||
} |
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.
drive-by-fix: In a test of this form, each case needs its own isolated recorder. The recorder does not replace its contents when invoked multiple times, but rather appends the request bodies as the cases proceed.
These tests all still pass, but without this they have less coverage than they appear to.