-
Notifications
You must be signed in to change notification settings - Fork 671
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
MF-1308 - Use IETF Health Check standard #1541
Changes from 6 commits
b91a6ce
a12d4fc
c7f534b
794e871
247d5b4
f439983
24e8cf5
775dd7c
848982a
4c06dee
ecdefa1
f92c147
784d43f
85d8625
a285f83
0772600
438eef9
c9d3111
57cca96
7d6f4dc
2ba2345
4b87bfc
19be83b
3517aeb
a4b7c0f
36727bb
2cc816a
d47bdc6
d3c3638
42de5ef
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 |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
// +build !test | ||
|
||
package api | ||
|
||
import ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
// +build !test | ||
|
||
package api | ||
|
||
import ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
// +build !test | ||
|
||
package api | ||
|
||
import ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
// +build !test | ||
|
||
package api | ||
|
||
import ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
// +build !test | ||
|
||
package api | ||
|
||
import ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package mainflux | ||
|
||
import ( | ||
"encoding/json" | ||
"net/http" | ||
) | ||
|
||
const ( | ||
version string = "0.12.1" | ||
contentType = "Content-Type" | ||
contentTypeJSON string = "application/json" | ||
svcStatus string = "pass" | ||
) | ||
|
||
// HealthInfo contains version endpoint response. | ||
type HealthInfo struct { | ||
// Status contains service status. | ||
Status string `json:"status"` | ||
|
||
// Version contains current service version. | ||
Version string `json:"version"` | ||
|
||
// Description contains service description. | ||
Description string `json:"description"` | ||
} | ||
|
||
// Health exposes an HTTP handler for retrieving service health. | ||
func Health(service string) http.HandlerFunc { | ||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
w.Header().Add(contentType, contentTypeJSON) | ||
if r.Method != http.MethodGet && r.Method != http.MethodHead { | ||
w.WriteHeader(http.StatusMethodNotAllowed) | ||
return | ||
} | ||
|
||
res := HealthInfo{ | ||
Status: svcStatus, | ||
Description: service, | ||
Version: version, | ||
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. it would be good to have timestamp of build so that when debugging deployment by querying the health you can confirm that right version is deployed 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. It should be covered by @blokovi proposition. There is an issue about: https://github.com/mainflux/mainflux/issues/1543 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. @mteodor there is a way to put timestamp and Git hash during the compilation phase. We had this before. I think it would make sense to add it back. |
||
} | ||
|
||
if err := json.NewEncoder(w).Encode(res); err != nil { | ||
w.WriteHeader(http.StatusInternalServerError) | ||
} | ||
|
||
w.WriteHeader(http.StatusOK) | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
// +build !test | ||
|
||
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. Please don't remove test tags. 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. Ok but can you explain me why this tags are not everywhere? For what this is needed? 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. Replaced by the new one 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.
It is used to ignore logging and metrics for testing (coverage included) - it is used here: https://github.com/mainflux/mainflux/blob/6483969927adb71736b8d4ed7553f8c2988973ce/scripts/ci.sh#L98 |
||
package api | ||
|
||
import ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
// +build !test | ||
|
||
package api | ||
|
||
import ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// Copyright (c) Mainflux | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package sdk | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"io/ioutil" | ||
"net/http" | ||
|
||
"github.com/mainflux/mainflux" | ||
"github.com/mainflux/mainflux/pkg/errors" | ||
) | ||
|
||
func (sdk mfSDK) Health() (mainflux.HealthInfo, error) { | ||
url := fmt.Sprintf("%s/health", sdk.thingsURL) | ||
|
||
resp, err := sdk.client.Get(url) | ||
if err != nil { | ||
return mainflux.HealthInfo{}, err | ||
} | ||
defer resp.Body.Close() | ||
|
||
body, err := ioutil.ReadAll(resp.Body) | ||
if err != nil { | ||
return mainflux.HealthInfo{}, err | ||
} | ||
|
||
if resp.StatusCode != http.StatusOK { | ||
return mainflux.HealthInfo{}, errors.Wrap(ErrFetchHealth, errors.New(resp.Status)) | ||
} | ||
|
||
var h mainflux.HealthInfo | ||
if err := json.Unmarshal(body, &h); err != nil { | ||
return mainflux.HealthInfo{}, err | ||
} | ||
|
||
return h, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ import ( | |
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestVersion(t *testing.T) { | ||
func TestHealth(t *testing.T) { | ||
svc := newThingsService(map[string]string{token: email}) | ||
ts := newThingsServer(svc) | ||
defer ts.Close() | ||
|
@@ -24,14 +24,14 @@ func TestVersion(t *testing.T) { | |
empty bool | ||
err error | ||
}{ | ||
"get version": { | ||
"get things service version": { | ||
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. Please check all the response fields, not only Version. |
||
empty: false, | ||
err: nil, | ||
}, | ||
} | ||
for desc, tc := range cases { | ||
ver, err := mainfluxSDK.Version() | ||
h, err := mainfluxSDK.Health() | ||
assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", desc, tc.err, err)) | ||
assert.Equal(t, tc.empty, ver == "", fmt.Sprintf("%s: expected non-empty result version, got %s", desc, ver)) | ||
assert.Equal(t, tc.empty, h.Version == "", fmt.Sprintf("%s: expected non-empty result version, got %s", desc, h.Version)) | ||
} | ||
} |
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.
is there a way that version is automatically set by release process
something like explained here
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.
You forgot the link :)
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.
@mteodor explained where?
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.
https://polyverse.com/blog/how-to-embed-versioning-information-in-go-applications-f76e2579b572/