Skip to content

Commit 3d64c21

Browse files
fix: invalid error message on health check failure (#26040) (cherry-pick #26039 for 3.3) (#26063)
Signed-off-by: Eugene Doudine <eugene.doudine@octopus.com> Co-authored-by: dudinea <eugene.doudine@octopus.com>
1 parent 0fa47b1 commit 3d64c21

File tree

2 files changed

+28
-3
lines changed

2 files changed

+28
-3
lines changed

util/healthz/healthz.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package healthz
33
import (
44
"fmt"
55
"net/http"
6+
"time"
67

78
log "github.com/sirupsen/logrus"
89
)
@@ -11,9 +12,13 @@ import (
1112
// ServeHealthCheck relies on the provided function to return an error if unhealthy and nil otherwise.
1213
func ServeHealthCheck(mux *http.ServeMux, f func(r *http.Request) error) {
1314
mux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
15+
startTs := time.Now()
1416
if err := f(r); err != nil {
1517
w.WriteHeader(http.StatusServiceUnavailable)
16-
log.Errorln(w, err)
18+
log.WithFields(log.Fields{
19+
"duration": time.Since(startTs),
20+
"component": "healthcheck",
21+
}).WithError(err).Error("Error serving health check request")
1722
} else {
1823
fmt.Fprintln(w, "ok")
1924
}

util/healthz/healthz_test.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,19 @@ import (
55
"net"
66
"net/http"
77
"testing"
8+
"time"
89

10+
log "github.com/sirupsen/logrus"
11+
"github.com/sirupsen/logrus/hooks/test"
12+
"github.com/stretchr/testify/assert"
913
"github.com/stretchr/testify/require"
1014
)
1115

1216
func TestHealthCheck(t *testing.T) {
1317
sentinel := false
1418
lc := &net.ListenConfig{}
1519
ctx := t.Context()
16-
20+
svcErrMsg := "This is a dummy error"
1721
serve := func(c chan<- string) {
1822
// listen on first available dynamic (unprivileged) port
1923
listener, err := lc.Listen(ctx, "tcp", ":0")
@@ -27,7 +31,7 @@ func TestHealthCheck(t *testing.T) {
2731
mux := http.NewServeMux()
2832
ServeHealthCheck(mux, func(_ *http.Request) error {
2933
if sentinel {
30-
return errors.New("This is a dummy error")
34+
return errors.New(svcErrMsg)
3135
}
3236
return nil
3337
})
@@ -52,10 +56,26 @@ func TestHealthCheck(t *testing.T) {
5256
require.Equalf(t, http.StatusOK, resp.StatusCode, "Was expecting status code 200 from health check, but got %d instead", resp.StatusCode)
5357

5458
sentinel = true
59+
hook := test.NewGlobal()
5560

5661
req, err = http.NewRequestWithContext(ctx, http.MethodGet, server+"/healthz", http.NoBody)
5762
require.NoError(t, err)
5863
resp, err = http.DefaultClient.Do(req)
5964
require.NoError(t, err)
6065
require.Equalf(t, http.StatusServiceUnavailable, resp.StatusCode, "Was expecting status code 503 from health check, but got %d instead", resp.StatusCode)
66+
assert.NotEmpty(t, hook.Entries, "Was expecting at least one log entry from health check, but got none")
67+
expectedMsg := "Error serving health check request"
68+
var foundEntry log.Entry
69+
for _, entry := range hook.Entries {
70+
if entry.Level == log.ErrorLevel &&
71+
entry.Message == expectedMsg {
72+
foundEntry = entry
73+
break
74+
}
75+
}
76+
require.NotEmpty(t, foundEntry, "Expected an error message '%s', but it was't found", expectedMsg)
77+
actualErr, ok := foundEntry.Data["error"].(error)
78+
require.True(t, ok, "Expected 'error' field to contain an error, but it doesn't")
79+
assert.Equal(t, svcErrMsg, actualErr.Error(), "expected original error message '"+svcErrMsg+"', but got '"+actualErr.Error()+"'")
80+
assert.Greater(t, foundEntry.Data["duration"].(time.Duration), time.Duration(0))
6181
}

0 commit comments

Comments
 (0)