Skip to content

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

Merged
merged 24 commits into from
Oct 26, 2022
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
eff0b3b
adding draft code to address docker healthcheck issue
PaurushGarg Jun 7, 2022
9215a4a
Resolving Comments
PaurushGarg Jun 22, 2022
3c57fe0
Resolving comments on github
PaurushGarg Jun 22, 2022
b41677e
Adding arguments validation and error handling
PaurushGarg Jun 23, 2022
f26b71c
Removing host and path from command line arguments and updating make …
PaurushGarg Jun 24, 2022
321da87
Deleting binary file and adding test cases
PaurushGarg Jun 29, 2022
50cc518
Removing lint errors
PaurushGarg Jun 30, 2022
c89eff0
Roll back changes to makefile to build healthcheck for multiple archi…
PaurushGarg Jul 15, 2022
240e19c
Removing changes from dockerfile as they will only part of execution …
PaurushGarg Jul 21, 2022
2f36a68
Adding changes to build multi-platform
PaurushGarg Jul 29, 2022
6856c9c
Removing healthcheck from dockerfile
PaurushGarg Aug 23, 2022
e35fe1c
Adding code changes to config and template files for healthcheck setu…
PaurushGarg Sep 7, 2022
9dd4611
Resolving comments
PaurushGarg Oct 19, 2022
94e18a1
Updating default path to match current default path of opentelemetry …
PaurushGarg Oct 19, 2022
aa04ba5
Resolving comments
PaurushGarg Oct 24, 2022
37888a4
Reverting the changes to ec2-sidecar-deployment-cfn
PaurushGarg Oct 24, 2022
f1722b2
Reverting unintended change to config.yaml
PaurushGarg Oct 25, 2022
3157b7f
Resolving Comments
PaurushGarg Oct 25, 2022
d591e62
Resolving comments
PaurushGarg Oct 25, 2022
4cb0364
Reverting change to cfn templates
PaurushGarg Oct 25, 2022
07315c6
Resolving comments
PaurushGarg Oct 25, 2022
2bc7fbe
Resoling lint check issue
PaurushGarg Oct 25, 2022
9cd838b
resolving comments
PaurushGarg Oct 25, 2022
fb54f42
Merge branch 'aws-observability:main' into docker_health_check
PaurushGarg Oct 25, 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
23 changes: 21 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ build: install-tools lint multimod-verify
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/amd64/aoc ./cmd/awscollector
GOOS=linux GOARCH=arm64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/arm64/aoc ./cmd/awscollector
GOOS=windows GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/windows/amd64/aoc ./cmd/awscollector
GOOS=darwin GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/darwin/amd64/healthcheck ./cmd/healthcheck
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/amd64/healthcheck ./cmd/healthcheck
GOOS=linux GOARCH=arm64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/arm64/healthcheck ./cmd/healthcheck
GOOS=windows GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/windows/amd64/healthcheck ./cmd/healthcheck


.PHONY: amd64-build
amd64-build: install-tools lint multimod-verify
Expand All @@ -119,6 +124,8 @@ amd64-build-only:
awscollector:
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./bin/awscollector_$(GOOS)_$(GOARCH) ./cmd/awscollector
GOOS=windows GOARCH=amd64 EXTENSION=.exe $(GOBUILD) $(LDFLAGS) -o ./bin/windows/aoc_windows_amd64.exe ./cmd/awscollector
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./bin/healthcheck_$(GOOS)_$(GOARCH) ./cmd/healthcheck
GOOS=windows GOARCH=amd64 EXTENSION=.exe $(GOBUILD) $(LDFLAGS) -o ./bin/windows/healthcheck_windows_amd64.exe ./cmd/healthcheck

.PHONY: package-rpm
package-rpm: build
Expand All @@ -130,13 +137,25 @@ package-deb: build
ARCH=arm64 DEST=build/packages/debian/arm64 tools/packaging/debian/create_deb.sh

.PHONY: docker-build
docker-build: amd64-build
docker-build: amd64-build amd64-build-healthcheck
docker buildx build --platform linux/amd64 --build-arg BUILDMODE=copy --load -t $(DOCKER_NAMESPACE)/$(COMPONENT):$(VERSION) -f ./cmd/$(COMPONENT)/Dockerfile .

.PHONY: amd64-build-healthcheck
amd64-build-healthcheck: install-tools lint multimod-verify
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/amd64/healthcheck ./cmd/healthcheck

.PHONY: docker-build-arm
docker-build-arm: arm64-build
docker-build-arm: arm64-build arm64-build-healthcheck
docker buildx build --platform linux/arm64 --build-arg BUILDMODE=copy --load -t $(DOCKER_NAMESPACE)/$(COMPONENT):$(VERSION) -f ./cmd/$(COMPONENT)/Dockerfile .

.PHONY: arm64-build-healthcheck
arm64-build-healthcheck: install-tools lint multimod-verify
GOOS=linux GOARCH=arm64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/arm64/healthcheck ./cmd/healthcheck

.PHONY: windows-build-healthcheck
Copy link
Contributor

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?

Copy link
Contributor Author

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..

windows-build-healthcheck: install-tools lint multimod-verify
GOOS=windows GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/windows/amd64/healthcheck ./cmd/healthcheck

.PHONY: docker-push
docker-push:
docker push $(DOCKER_NAMESPACE)/$(COMPONENT):$(VERSION)
Expand Down
2 changes: 2 additions & 0 deletions cmd/awscollector/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ ARG TARGETARCH
# copy artifacts
# always assume binary is created
COPY build/linux/$TARGETARCH/aoc /workspace/awscollector
COPY build/linux/$TARGETARCH/healthcheck /workspace/healthcheck

################################
# Packing Stage #
Expand All @@ -70,6 +71,7 @@ FROM scratch
COPY --from=certs /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
COPY --from=package /workspace/awscollector /awscollector
COPY --from=package /workspace/config/ /etc/
COPY --from=package /workspace/healthcheck /healthcheck

ENV RUN_IN_CONTAINER="True"
# aws-sdk-go needs $HOME to look up shared credentials
Expand Down
67 changes: 67 additions & 0 deletions cmd/healthcheck/main.go
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

}
106 changes: 106 additions & 0 deletions cmd/healthcheck/main_test.go
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"}
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
}{
{
name: "WrongString",
port: "StringPort",
errorExpected: true,
},
{
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)
}
})
}
}
1 change: 0 additions & 1 deletion config/ecs/ecs-default-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
extensions:
health_check:
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:
Expand Down
4 changes: 4 additions & 0 deletions config/ecs/otel-instance-metrics-config.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
extensions:
health_check:

receivers:
awscontainerinsightreceiver:
collection_interval: 10s
Expand Down Expand Up @@ -47,3 +50,4 @@ service:
receivers: [awscontainerinsightreceiver]
processors: [batch/metrics]
exporters: [awsemf,logging]
extensions: [health_check]
6 changes: 6 additions & 0 deletions docs/developers/docker-demo.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ If you haven't setup your AWS Credential profile yet, please follow the [instruc
cd aws-otel-collector
```

* [Optional] To configure `healthcheck` for `aws-otel-collector`, [run](https://github.com/aws-observability/aws-otel-collector/blob/main/docs/developers/docker-demo.md) the `aws-otel-collector` instance in Docker with these additional arguments:
```bash
--health-cmd='/healthcheck' \
--health-interval=5s
```

* Start the `aws-otel-collector` instance in Docker using the `default` AWS Credential profile.

```bash
Expand Down
7 changes: 7 additions & 0 deletions examples/ecs/aws-cloudwatch/ecs-ec2-sidecar.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@
"awslogs-create-group": "True"
}
},
"healthCheck": {
"command": [ "/healthcheck" ],
"interval": 5,
"timeout": 6,
"retries": 5,
"startPeriod": 1
},
"portMappings": [
{
"hostPort": 2000,
Expand Down
7 changes: 7 additions & 0 deletions examples/ecs/aws-cloudwatch/ecs-fargate-sidecar.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
"awslogs-stream-prefix": "ecs",
"awslogs-create-group": "True"
}
},
"healthCheck": {
"command": [ "/healthcheck" ],
"interval": 5,
"timeout": 6,
"retries": 5,
"startPeriod": 1
}
},
{
Expand Down