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

Jump to bottom

feat: add grpc health, deprecate old health #861

Closed
wants to merge 6 commits into from
Closed

feat: add grpc health, deprecate old health #861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
93a9069
feat: add grpc health, deprecate old health
toddbaert Aug 24, 2023
4648c26
fixup: lint
toddbaert Aug 24, 2023
a00ae5a
fixup: minor refactor
toddbaert Aug 24, 2023
9c2fa3e
fixup: more compiler options
toddbaert Aug 25, 2023
c26e569
fixup: conflictPolicy=ignore in prod
toddbaert Aug 25, 2023
465add4
fixup: more doc
toddbaert Aug 25, 2023
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
2 changes: 1 addition & 1 deletion .github/workflows/release-please.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ jobs:
VERSION_NO_PREFIX: ${{ needs.release-please.outputs[format('{0}_version', matrix.path)] }}
COMMIT: ${{ github.sha }}
DATE: ${{ needs.release-please.outputs.date }}
BUILD_ARGS: '-a -ldflags "-X main.version=${VERSION} -X main.commit=${COMMIT} -X main.date=${DATE}"'
BUILD_ARGS: '-a -ldflags "-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=ignore -X main.version=${VERSION} -X main.commit=${COMMIT} -X main.date=${DATE}"'
steps:
- name: Checkout
uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3
Expand Down
4 changes: 3 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ ZD_TEST_NAMESPACE ?= flagd-zd-test
ZD_CLIENT_IMG ?= zd-client:latest
FLAGD_PROXY_IMG ?= flagd-proxy:latest
FLAGD_PROXY_IMG_ZD ?= flagd-proxy:zd
# the same "status.proto" is supplied by 2 modules, which causes an error we can safely ignore
export GOLANG_PROTOBUF_REGISTRATION_CONFLICT=warn

workspace-init: workspace-clean
go work init
Expand Down Expand Up @@ -35,7 +37,7 @@ docker-push-flagd:
build: workspace-init # default to flagd
make build-flagd
build-flagd:
go build -ldflags "-X main.version=dev -X main.commit=$$(git rev-parse --short HEAD) -X main.date=$$(date +%FT%TZ)" -o ./bin/flagd ./flagd
go build -ldflags "-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=warn -X main.version=dev -X main.commit=$$(git rev-parse --short HEAD) -X main.date=$$(date +%FT%TZ)" -o ./bin/flagd ./flagd
.PHONY: test
test: # default to core
make test-core
Expand Down
4 changes: 2 additions & 2 deletions config/deployments/flagd/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ spec:
readinessProbe:
httpGet:
path: /readyz
port: 8014
Copy link
Member

Choose a reason for hiding this comment

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

please, adapt also the ZD tests manifest to use the new port :)

Copy link
Member Author

@toddbaert toddbaert Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@thisthat flagd-proxy uses the sync package from core, NOT the flag-evaluation package. For this reason, the health-checks of flagd-proxy are not impacted by this change (which may or may not be good, since things will be a bit inconsistent.

Problem is, I'm not 100% sure I can add HTTP 1.1 health-check support to the sync package since it doesn't use connect.

port: 8013
initialDelaySeconds: 5
periodSeconds: 5
livenessProbe:
httpGet:
path: /healthz
port: 8014
port: 8013
initialDelaySeconds: 5
periodSeconds: 60
ports:
Expand Down
25 changes: 21 additions & 4 deletions core/pkg/service/flag-evaluation/connect_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

schemaConnectV1 "buf.build/gen/go/open-feature/flagd/bufbuild/connect-go/schema/v1/schemav1connect"
"connectrpc.com/grpchealth"
Copy link
Member Author

Choose a reason for hiding this comment

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

this package is a connect-compatible implementation of a gRPC healthcheck.

"github.com/open-feature/flagd/core/pkg/eval"
"github.com/open-feature/flagd/core/pkg/logger"
"github.com/open-feature/flagd/core/pkg/service"
Expand Down Expand Up @@ -101,6 +102,7 @@ func (s *ConnectService) Notify(n service.Notification) {
s.eventingConfiguration.emitToAll(n)
}

// nolint: funlen
func (s *ConnectService) setupServer(svcConf service.Configuration) (net.Listener, error) {
var lis net.Listener
var err error
Expand All @@ -127,18 +129,29 @@ func (s *ConnectService) setupServer(svcConf service.Configuration) (net.Listene
protojson.UnmarshalOptions{DiscardUnknown: true},
)

path, handler := schemaConnectV1.NewServiceHandler(fes, append(svcConf.Options, marshalOpts)...)
mux.Handle(path, handler)
mux.Handle(schemaConnectV1.NewServiceHandler(fes, append(svcConf.Options, marshalOpts)...))
mux.Handle(grpchealth.NewHandler(grpchealth.NewStaticChecker(
Copy link
Member Author

@toddbaert toddbaert Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Standard implementation, recommended by buf/conntect.

You can test this locally with grpcurl (below command assumes you've downloaded the health proto to ~/Downloads):

grpcurl -import-path '/home/{your-user}/Downloads/'  -proto health.proto -plaintext localhost:8013 grpc.health.v1.Health/Check

schemaConnectV1.ServiceName,
)))
mux.Handle("/healthz", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
mux.Handle("/readyz", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if s.readinessEnabled && svcConf.ReadinessProbe() {
w.WriteHeader(http.StatusOK)
} else {
w.WriteHeader(http.StatusPreconditionFailed)
}
}))

s.serverMtx.Lock()
s.server = &http.Server{
ReadHeaderTimeout: time.Second,
Handler: handler,
Handler: mux,
Copy link
Member Author

@toddbaert toddbaert Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The mux created above was actually unused - instead we just used the one handler originally.

}
s.serverMtx.Unlock()

// Add middlewares

metricsMiddleware := metricsmw.NewHTTPMetric(metricsmw.Config{
Service: svcConf.ServiceName,
MetricRecorder: s.metrics,
Expand Down Expand Up @@ -206,8 +219,12 @@ func (s *ConnectService) startMetricsServer(svcConf service.Configuration) error
s.metricsServer.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
Copy link
Member Author

Choose a reason for hiding this comment

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

In my new implementation, I've used mux instead of this kind of switch statement.

case "/healthz":
s.logger.Warn(
fmt.Sprintf("/healthz endpoint on port %d is deprecated, use port %d instead", svcConf.MetricsPort, svcConf.Port))
w.WriteHeader(http.StatusOK)
case "/readyz":
s.logger.Warn(
fmt.Sprintf("/readyz endpoint on port %d is deprecated, use port %d instead", svcConf.MetricsPort, svcConf.Port))
if s.readinessEnabled && svcConf.ReadinessProbe() {
w.WriteHeader(http.StatusOK)
} else {
Expand Down
13 changes: 9 additions & 4 deletions docs/other_resources/high_level_architecture.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,18 @@ process gets pushed to event subscribers.

## Readiness & Liveness probes

### HTTP

Flagd exposes HTTP liveness and readiness probes.
These probes can be used for K8s deployments.
With default
start-up configurations, these probes are exposed at the following URLs,
With default start-up configurations, these probes are exposed on the service port (default: 8013) at the following URLs,

- Liveness: <http://localhost:8013/healthz>
- Readiness: <http://localhost:8013/readyz>

### gRPC

- Liveness: <http://localhost:8014/healthz>
- Readiness: <http://localhost:8014/readyz>
Flagd exposes a [standard gRPC liveness check](https://github.com/grpc/grpc/blob/master/doc/health-checking.md) on the service port (default: 8013).

### Definition of Liveness

Expand Down
2 changes: 1 addition & 1 deletion flagd-proxy/build.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ RUN --mount=type=cache,target=/go/pkg/mod/ \
--mount=type=bind,source=./core,target=./core \
--mount=type=bind,source=./flagd,target=./flagd \
--mount=type=bind,source=./flagd-proxy,target=./flagd-proxy \
CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -a -ldflags "-X main.version=${VERSION} -X main.commit=${COMMIT} -X main.date=${DATE}" -o /bin/flagd-proxy flagd-proxy/main.go
CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -a -ldflags "-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=ignore -X main.version=${VERSION} -X main.commit=${COMMIT} -X main.date=${DATE}" -o /bin/flagd-proxy flagd-proxy/main.go

# # Use distroless as minimal base image to package the manager binary
# # Refer to https://github.com/GoogleContainerTools/distroless for more details
Expand Down
2 changes: 1 addition & 1 deletion flagd/build.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ RUN --mount=type=cache,target=/go/pkg/mod/ \
--mount=type=cache,target=/root/.cache/go-build \
--mount=type=bind,source=./core,target=./core \
--mount=type=bind,source=./flagd,target=./flagd \
CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -a -ldflags "-X main.version=${VERSION} -X main.commit=${COMMIT} -X main.date=${DATE}" -o /bin/flagd-build flagd/main.go
CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -a -ldflags "-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=ignore -X main.version=${VERSION} -X main.commit=${COMMIT} -X main.date=${DATE}" -o /bin/flagd-build flagd/main.go

# # Use distroless as minimal base image to package the manager binary
# # Refer to https://github.com/GoogleContainerTools/distroless for more details
Expand Down
2 changes: 2 additions & 0 deletions flagd/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ require (
buf.build/gen/go/open-feature/flagd/bufbuild/connect-go v1.9.0-20230720212818-3675556880a1.1 // indirect
buf.build/gen/go/open-feature/flagd/grpc/go v1.3.0-20230710190440-2333a9579c1a.1 // indirect
buf.build/gen/go/open-feature/flagd/protocolbuffers/go v1.31.0-20230720212818-3675556880a1.1 // indirect
connectrpc.com/connect v1.11.0 // indirect
connectrpc.com/grpchealth v1.2.0 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bufbuild/connect-go v1.10.0 // indirect
github.com/bufbuild/connect-opentelemetry-go v0.4.0 // indirect
Expand Down
4 changes: 4 additions & 0 deletions flagd/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,10 @@ cloud.google.com/go/workflows v1.6.0/go.mod h1:6t9F5h/unJz41YqfBmqSASJSXccBLtD1V
cloud.google.com/go/workflows v1.7.0/go.mod h1:JhSrZuVZWuiDfKEFxU0/F1PQjmpnpcoISEXH2bcHC3M=
cloud.google.com/go/workflows v1.8.0/go.mod h1:ysGhmEajwZxGn1OhGOGKsTXc5PyxOc0vfKf5Af+to4M=
cloud.google.com/go/workflows v1.9.0/go.mod h1:ZGkj1aFIOd9c8Gerkjjq7OW7I5+l6cSvT3ujaO/WwSA=
connectrpc.com/connect v1.11.0 h1:Av2KQXxSaX4vjqhf5Cl01SX4dqYADQ38eBtr84JSUBk=
connectrpc.com/connect v1.11.0/go.mod h1:3AGaO6RRGMx5IKFfqbe3hvK1NqLosFNP2BxDYTPmNPo=
connectrpc.com/grpchealth v1.2.0 h1:aHP33Bki+F2jPNI1mFVSFG7v0qJrgmfbg7X7nOdSj0M=
connectrpc.com/grpchealth v1.2.0/go.mod h1:fZos12C4p/ZaZC6OwBGZUM+i/fhnRhv75ax/6V/zIeM=
dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
Expand Down
2 changes: 1 addition & 1 deletion flagd/profile.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ RUN --mount=type=cache,target=/go/pkg/mod/ \
--mount=type=cache,target=/root/.cache/go-build \
--mount=type=bind,source=./core,target=./core \
--mount=type=bind,source=./flagd,target=./flagd \
CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -a -ldflags "-X main.version=${VERSION} -X main.commit=${COMMIT} -X main.date=${DATE}" -o /bin/flagd-build ./flagd/main.go ./flagd/profiler.go
CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -a -ldflags "-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=ignore -X main.version=${VERSION} -X main.commit=${COMMIT} -X main.date=${DATE}" -o /bin/flagd-build ./flagd/main.go ./flagd/profiler.go

# Use distroless as minimal base image to package the manager binary
# Refer to https://github.com/GoogleContainerTools/distroless for more details
Expand Down
13 changes: 9 additions & 4 deletions web-docs/concepts/architecture.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,18 @@ process gets pushed to event subscribers.

## Readiness & Liveness probes

### HTTP

Flagd exposes HTTP liveness and readiness probes.
These probes can be used for K8s deployments.
With default
start-up configurations, these probes are exposed at the following URLs,
With default start-up configurations, these probes are exposed on the service port (default: 8013) at the following URLs,

- Liveness: <http://localhost:8013/healthz>
- Readiness: <http://localhost:8013/readyz>

### gRPC

- Liveness: <http://localhost:8014/healthz>
- Readiness: <http://localhost:8014/readyz>
Flagd exposes a [standard gRPC liveness check](https://github.com/grpc/grpc/blob/master/doc/health-checking.md) on the service port (default: 8013).

### Definition of Liveness

Expand Down
Loading