Skip to content
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.

Sign up for GitHub

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

Clarify and clean up template contract for status #149

Merged
merged 4 commits into from
Mar 30, 2022
Merged

Clarify and clean up template contract for status #149

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6bd72a9
Clarify and clean up template contract for status
colega Mar 29, 2022
0b963d9
Update CHANGELOG.md
colega Mar 29, 2022
7a42eff
Use only 2 decimals for percentage
colega Mar 30, 2022
d2926ce
Add 'Content-Type: text/html' header
colega Mar 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* [CHANGE] ring/client: It's now possible to set different value than `consul` as default KV store. #120
* [CHANGE] Lifecycler: Default value of lifecycler's `final-sleep` is now `0s` (i.e. no sleep). #121
* [CHANGE] Lifecycler: It's now possible to change default value of lifecycler's `final-sleep`. #121
* [CHANGE] Minor cosmetic changes in ring and memberlist HTTP status templates. #149
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a breaking change? Looks like the output is for humans.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is definitely not breaking, but it's a change. Would you prefer this to be an [ENHANCEMENT]?

* [ENHANCEMENT] Add middleware package. #38
* [ENHANCEMENT] Add the ring package #45
* [ENHANCEMENT] Add limiter package. #41
Expand Down
13 changes: 2 additions & 11 deletions kv/memberlist/kv_init_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,21 +171,13 @@ func (kvs *KVInitService) ServeHTTP(w http.ResponseWriter, req *http.Request) {
if strings.Contains(accept, "application/json") {
w.Header().Set("Content-Type", "application/json")

data, err := json.Marshal(v)
if err != nil {
if err := json.NewEncoder(w).Encode(v); err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}

// We ignore errors here, because we cannot do anything about them.
// Write will trigger sending Status code, so we cannot send a different status code afterwards.
// Also this isn't internal error, but error communicating with client.
_, _ = w.Write(data)
return
}

err := defaultPageTemplate.Execute(w, v)
if err != nil {
if err := defaultPageTemplate.Execute(w, v); err != nil {
pstibrany marked this conversation as resolved.
Show resolved Hide resolved
http.Error(w, err.Error(), http.StatusInternalServerError)
}
}
Expand Down Expand Up @@ -226,7 +218,6 @@ func viewKey(w http.ResponseWriter, store map[string]valueDesc, key string, form
}

func formatValue(w http.ResponseWriter, val interface{}, format string) {

w.WriteHeader(200)
w.Header().Add("content-type", "text/plain")

Expand Down
58 changes: 27 additions & 31 deletions ring/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,36 @@ import (
var defaultPageContent string
var defaultPageTemplate = template.Must(template.New("webpage").Funcs(template.FuncMap{
"mod": func(i, j int) bool { return i%j == 0 },
"humanFloat": func(f float64) string {
return fmt.Sprintf("%.4g", f)
},
"timeOrEmptyString": func(t time.Time) string {
if t.IsZero() {
return ""
}
return t.Format(time.RFC3339Nano)
},
"durationSince": func(t time.Time) string { return time.Since(t).Truncate(time.Millisecond).String() },
}).Parse(defaultPageContent))

type ingesterDesc struct {
ID string `json:"id"`
State string `json:"state"`
Address string `json:"address"`
HeartbeatTimestamp string `json:"timestamp"`
RegisteredTimestamp string `json:"registered_timestamp"`
Zone string `json:"zone"`
Tokens []uint32 `json:"tokens"`
NumTokens int `json:"-"`
Ownership float64 `json:"-"`
}

type httpResponse struct {
Ingesters []ingesterDesc `json:"shards"`
Now time.Time `json:"now"`
ShowTokens bool `json:"-"`
}

type ingesterDesc struct {
ID string `json:"id"`
State string `json:"state"`
Address string `json:"address"`
HeartbeatTimestamp time.Time `json:"timestamp"`
RegisteredTimestamp time.Time `json:"registered_timestamp"`
Zone string `json:"zone"`
Tokens []uint32 `json:"tokens"`
NumTokens int `json:"-"`
Ownership float64 `json:"-"`
}

type ringAccess interface {
casRing(ctx context.Context, f func(in interface{}) (out interface{}, retry bool, err error)) error
getRing(context.Context) (*Desc, error)
Expand Down Expand Up @@ -85,7 +95,7 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) {
}
_, ownedTokens := ringDesc.countTokens()

ingesterIDs := []string{}
var ingesterIDs []string
for id := range ringDesc.Ingesters {
ingesterIDs = append(ingesterIDs, id)
}
Expand All @@ -95,24 +105,17 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) {
var ingesters []ingesterDesc
for _, id := range ingesterIDs {
ing := ringDesc.Ingesters[id]
heartbeatTimestamp := time.Unix(ing.Timestamp, 0)
state := ing.State.String()
if !ing.IsHealthy(Reporting, h.heartbeatPeriod, now) {
state = unhealthy
}

// Format the registered timestamp.
registeredTimestamp := ""
if ing.RegisteredTimestamp != 0 {
registeredTimestamp = ing.GetRegisteredAt().String()
state = "UNHEALTHY"
}

ingesters = append(ingesters, ingesterDesc{
ID: id,
State: state,
Address: ing.Addr,
HeartbeatTimestamp: heartbeatTimestamp.String(),
RegisteredTimestamp: registeredTimestamp,
HeartbeatTimestamp: time.Unix(ing.Timestamp, 0).UTC(),
RegisteredTimestamp: ing.GetRegisteredAt().UTC(),
Tokens: ing.Tokens,
Zone: ing.Zone,
NumTokens: len(ing.Tokens),
Expand Down Expand Up @@ -161,14 +164,7 @@ func (h *ringPageHandler) forget(ctx context.Context, id string) error {
func writeJSONResponse(w http.ResponseWriter, v httpResponse) {
w.Header().Set("Content-Type", "application/json")

data, err := json.Marshal(v)
if err != nil {
if err := json.NewEncoder(w).Encode(v); err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}

// We ignore errors here, because we cannot do anything about them.
// Write will trigger sending Status code, so we cannot send a different status code afterwards.
// Also this isn't internal error, but error communicating with client.
_, _ = w.Write(data)
}
6 changes: 3 additions & 3 deletions ring/status.gohtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@
<td>{{ .Zone }}</td>
<td>{{ .State }}</td>
<td>{{ .Address }}</td>
<td>{{ .RegisteredTimestamp }}</td>
<td>{{ .HeartbeatTimestamp }}</td>
<td>{{ .RegisteredTimestamp | timeOrEmptyString }}</td>
<td>{{ .HeartbeatTimestamp | durationSince }} ago ({{ .HeartbeatTimestamp.Format "15:04:05.999" }})</td>
<td>{{ .NumTokens }}</td>
<td>{{ .Ownership }}%</td>
<td>{{ .Ownership | humanFloat }}%</td>
<td>
<button name="forget" value="{{ .ID }}" type="submit">Forget</button>
</td>
Expand Down