-
Notifications
You must be signed in to change notification settings - Fork 247
Adding code to address docker healthcheck issue #1285
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
Changes from 20 commits
eff0b3b
9215a4a
3c57fe0
b41677e
f26b71c
321da87
50cc518
c89eff0
240e19c
2f36a68
6856c9c
e35fe1c
9dd4611
94e18a1
aa04ba5
37888a4
f1722b2
3157b7f
d591e62
4cb0364
07315c6
2bc7fbe
9cd838b
fb54f42
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 |
---|---|---|
@@ -0,0 +1,67 @@ | ||
package main | ||
|
||
import ( | ||
"flag" | ||
"fmt" | ||
"log" | ||
"net/http" | ||
"os" | ||
"strconv" | ||
) | ||
|
||
func main() { | ||
const host = "127.0.0.1" // default host | ||
usedPort := "13133" // default port | ||
const path = "/" //default path | ||
generateCmd := flag.NewFlagSet("generate", flag.ExitOnError) | ||
port := generateCmd.String("port", usedPort, "Specify collector health-check port") | ||
|
||
if len(os.Args) > 1 { | ||
err := generateCmd.Parse(os.Args[1:]) | ||
if err != nil { | ||
log.Fatalf("%s", err) | ||
} | ||
} | ||
|
||
validationErr := validatePort(*port) | ||
if validationErr != nil { | ||
log.Fatalf("%s", validationErr) | ||
} | ||
|
||
status, healthCheckError := executeHealthCheck(host, port, path) | ||
|
||
if healthCheckError != nil { | ||
log.Fatalf(healthCheckError.Error()) | ||
} | ||
|
||
log.Printf(status) | ||
|
||
} | ||
|
||
func executeHealthCheck(host string, port *string, path string) (string, error) { | ||
resp, err := http.Get(fmt.Sprint("http://", host, ":", *port, path)) | ||
|
||
if err != nil { | ||
return "", fmt.Errorf("unable to retrieve health status: %s", err.Error()) | ||
} | ||
|
||
if resp.StatusCode != 200 { | ||
return "", fmt.Errorf("STATUS: %d", resp.StatusCode) | ||
} | ||
|
||
return fmt.Sprintf("STATUS: %d", resp.StatusCode), nil | ||
} | ||
|
||
// validatePort checks if the port configuration is valid | ||
func validatePort(port string) error { | ||
|
||
portInt, err := strconv.Atoi(port) | ||
if err != nil { | ||
return fmt.Errorf("invalid port: %w", err) | ||
} | ||
if portInt < 1 || portInt > 65535 { | ||
return fmt.Errorf("port outside of range [1:65535]: %d", portInt) | ||
} | ||
return nil | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
package main | ||
|
||
import ( | ||
"fmt" | ||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
"net" | ||
"net/http" | ||
"net/http/httptest" | ||
"os" | ||
"testing" | ||
) | ||
|
||
func TestHealthStatusHealthy(t *testing.T) { | ||
server := setUpMockCollector(t, "127.0.0.1:13133", http.StatusOK) | ||
os.Args = []string{"-port=13133"} | ||
bryan-aguilar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
defer server.Close() | ||
port := "13133" | ||
got, err := executeHealthCheck("127.0.0.1", &port, "/") | ||
|
||
expectedErrorString := "STATUS: 200" | ||
|
||
assert.Contains(t, got, expectedErrorString, | ||
fmt.Sprintf("Unexpected log message. Got %s but should contain %s", got, expectedErrorString)) | ||
|
||
assert.NoError(t, err) | ||
|
||
} | ||
|
||
func TestHealthStatusUnhealthy(t *testing.T) { | ||
server := setUpMockCollector(t, "127.0.0.1:13133", http.StatusInternalServerError) | ||
os.Args = []string{"-port=13133"} | ||
defer server.Close() | ||
port := "13133" | ||
got, err := executeHealthCheck("127.0.0.1", &port, "/") | ||
|
||
expectedErrorString := "STATUS: 500" | ||
|
||
assert.Contains(t, err.Error(), expectedErrorString, | ||
fmt.Sprintf("Unexpected log message. Got %s but should contain %s", got, expectedErrorString)) | ||
|
||
} | ||
func TestHealthStatusServerDown(t *testing.T) { | ||
server := setUpMockCollector(t, "127.0.0.1:13132", http.StatusInternalServerError) | ||
os.Args = []string{"-port=13133"} | ||
defer server.Close() | ||
port := "13133" | ||
got, err := executeHealthCheck("127.0.0.1", &port, "/") | ||
|
||
expectedErrorString := "unable to retrieve health status" | ||
|
||
assert.Contains(t, err.Error(), expectedErrorString, | ||
fmt.Sprintf("Unexpected log message. Got %s but should contain %s", got, expectedErrorString)) | ||
|
||
} | ||
|
||
func setUpMockCollector(t *testing.T, healthCheckDefaultEndpoint string, statusCode int) *httptest.Server { | ||
l, err := net.Listen("tcp", healthCheckDefaultEndpoint) | ||
require.NoError(t, err) | ||
server := httptest.NewUnstartedServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { | ||
rw.WriteHeader(statusCode) | ||
})) | ||
server.Listener.Close() | ||
server.Listener = l | ||
server.Start() | ||
return server | ||
} | ||
|
||
func TestValidatePort(t *testing.T) { | ||
testCases := []struct { | ||
name string | ||
port string | ||
errorExpected bool | ||
bryan-aguilar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}{ | ||
{ | ||
name: "WrongString", | ||
port: "StringPort", | ||
errorExpected: true, | ||
bryan-aguilar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
{ | ||
name: "EmptyString", | ||
port: "", | ||
errorExpected: true, | ||
}, | ||
{ | ||
name: "WrongPort", | ||
port: "65536", | ||
errorExpected: true, | ||
}, | ||
{ | ||
name: "ValidPort", | ||
port: "13133", | ||
errorExpected: false, | ||
}, | ||
} | ||
for _, tc := range testCases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
err := validatePort(tc.port) | ||
if tc.errorExpected { | ||
assert.Error(t, err) | ||
} else { | ||
assert.NoError(t, err) | ||
} | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
extensions: | ||
health_check: | ||
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. Do we need to specify endpoint and path in this config like the others? 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 have removed specific mention of endpoint and path from all configs- as pointed out by in one of the comments by Anthony. The default healthcheck endpoints will be applicable in this case. |
||
|
||
receivers: | ||
otlp: | ||
protocols: | ||
|
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.
When does this get utilized?
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.
As per your initial comment I added build commands for other architectures..