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.

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

feat: add gRPC healthchecks #863

Merged
merged 5 commits into from
Aug 29, 2023
Merged

feat: add gRPC healthchecks #863

merged 5 commits into from
Aug 29, 2023

Conversation

toddbaert
Copy link
Member

@toddbaert toddbaert commented Aug 25, 2023

  • adds a gRPC healthcheck to the "metrics" port
  • supports both gRPC and HTTP on "metrics" port
  • adds relevant doc

@thisthat @Kavindu-Dodan @beeme1mr

I've closed the previous PR and gone about this a different way... Primarily because I realized it's a bad idea to put the healthcheck on the "main" socket, since that can be bound not just to a TCP socket, but to a unix socket, which would be very problematic for a healthcheck 😅 .

I've instead added the new gRPC health checks to the "metrics" ports of both flagd and flagd-proxy with some basic home-grown multiplexing code. I strongly recommend we re-name this port from "metrics" to "management" in a subsequent PR, since we are not just using it for metrics (we can probably use viper to make this non-breaking, ie use "metricsPort" if "managementPort" is missing).

@thisthat another bonus is this approach doesn't require and new deps, and therefore no proto conflicts!

fixes: #831

@netlify
Copy link

netlify bot commented Aug 25, 2023

Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit bdf57ce
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/64ee093eb45f7f0008b3e38c

@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Merging #863 (bdf57ce) into main (50df791) will decrease coverage by 0.02%.
The diff coverage is 39.28%.

@@            Coverage Diff             @@
##             main     #863      +/-   ##
==========================================
- Coverage   72.81%   72.80%   -0.02%     
==========================================
  Files          27       27              
  Lines        2755     2765      +10     
==========================================
+ Hits         2006     2013       +7     
- Misses        664      665       +1     
- Partials       85       87       +2     
Files Changed Coverage Δ
...ore/pkg/service/flag-evaluation/connect_service.go 68.29% <39.28%> (+0.11%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@toddbaert toddbaert changed the title feat: add gRPC healthcheck feat: add gRPC healthchecks Aug 25, 2023
@Kavindu-Dodan
Copy link
Contributor

Overal looks good 🙌

Copy link
Member

@thisthat thisthat left a comment

Choose a reason for hiding this comment

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

Beside the comment of @Kavindu-Dodan, LGTM 👍

@toddbaert
Copy link
Member Author

@Kavindu-Dodan @thisthat I've fixed that issue. Please re-review.

* adds a gRPC healthcheck to the "metrics" port
* supports both gRPC and HTTP on "metrics" port
* adds relevant doc

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
core/pkg/service/sync/server.go Outdated Show resolved Hide resolved
web-docs/concepts/architecture.md Show resolved Hide resolved
toddbaert and others added 3 commits August 29, 2023 09:05
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert merged commit da30b7b into main Aug 29, 2023
15 of 17 checks passed
@toddbaert toddbaert deleted the grpc-healthcheck branch August 29, 2023 17:41
@github-actions github-actions bot mentioned this pull request Aug 29, 2023
beeme1mr pushed a commit that referenced this pull request Aug 31, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.6.4</summary>

##
[0.6.4](flagd/v0.6.3...flagd/v0.6.4)
(2023-08-30)


### 🐛 Bug Fixes

* **deps:** update module github.com/cucumber/godog to v0.13.0
([#855](#855))
([5b42486](5b42486))
* **deps:** update module github.com/open-feature/flagd/core to v0.6.3
([#794](#794))
([9671964](9671964))


### 🧹 Chore

* **deps:** update golang docker tag to v1.21
([#822](#822))
([effe29d](effe29d))
</details>

<details><summary>flagd-proxy: 0.2.9</summary>

##
[0.2.9](flagd-proxy/v0.2.8...flagd-proxy/v0.2.9)
(2023-08-30)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.6.3
([#794](#794))
([9671964](9671964))


### 🧹 Chore

* **deps:** update golang docker tag to v1.21
([#822](#822))
([effe29d](effe29d))
</details>

<details><summary>core: 0.6.4</summary>

##
[0.6.4](core/v0.6.3...core/v0.6.4)
(2023-08-30)


### 🐛 Bug Fixes

* **deps:** update kubernetes packages to v0.28.0
([#841](#841))
([cc195e1](cc195e1))
* **deps:** update kubernetes packages to v0.28.1
([#860](#860))
([f3237c2](f3237c2))
* **deps:** update module github.com/open-feature/open-feature-operator
to v0.2.36 ([#799](#799))
([fa4da4b](fa4da4b))
* **deps:** update module golang.org/x/crypto to v0.12.0
([#797](#797))
([edae3fd](edae3fd))
* **deps:** update module golang.org/x/net to v0.14.0
([#798](#798))
([92c2f26](92c2f26))
* **deps:** update module sigs.k8s.io/controller-runtime to v0.15.1
([#795](#795))
([13d62fd](13d62fd))
* **deps:** update module sigs.k8s.io/controller-runtime to v0.16.0
([#856](#856))
([88d832a](88d832a))


### ✨ New Features

* add flag key to hash in fractional evaluation
([#847](#847))
([ca6a35f](ca6a35f))
* add gRPC healthchecks
([#863](#863))
([da30b7b](da30b7b))
* support nested props in fractional evaluator
([#869](#869))
([50ff739](50ff739))


### 🧹 Chore

* deprecate fractionalEvaluation for fractional
([#873](#873))
([243fef9](243fef9))
* replace xxh3 with murmur3 in bucket algorithm
([#846](#846))
([c3c9e4e](c3c9e4e))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] gRPC Health Check must be implemented in case flagd is accessed via AWS ALB
4 participants