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 Health and Readiness probes #418

Merged
merged 15 commits into from
Feb 27, 2023

Conversation

thisthat
Copy link
Member

@thisthat thisthat commented Feb 18, 2023

This PR

Add health and readiness probes to flagd to monitor the state of the services.

Related Issues

#408

Follow-up Tasks

Modify the OFO to use the newly exposed health and readiness probes.

Notes

The PR is in draft because I don't know if I should open an enhancement proposal before being able to merge this.
Please, let me know what's the usual process for this type of changes :)

@thisthat thisthat changed the title Add basic impl structure feat: add Health and Readiness probes Feb 18, 2023
@thisthat thisthat force-pushed the feat/408/add-probes branch 3 times, most recently from c201d53 to d8efc6e Compare February 18, 2023 22:44
@codecov
Copy link

codecov bot commented Feb 18, 2023

Codecov Report

Merging #418 (35451fa) into main (5a6a5d5) will increase coverage by 1.01%.
The diff coverage is 61.53%.

@@            Coverage Diff             @@
##             main     #418      +/-   ##
==========================================
+ Coverage   63.92%   64.93%   +1.01%     
==========================================
  Files          11       11              
  Lines        1325     1369      +44     
==========================================
+ Hits          847      889      +42     
+ Misses        428      427       -1     
- Partials       50       53       +3     
Impacted Files Coverage Δ
pkg/service/connect_service.go 59.63% <26.66%> (-1.79%) ⬇️
pkg/sync/http/http_sync.go 42.55% <55.55%> (+2.07%) ⬆️
pkg/sync/grpc/grpc_sync.go 60.31% <65.00%> (+15.27%) ⬆️
pkg/sync/file/filepath_sync.go 53.27% <85.71%> (+4.31%) ⬆️

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

pkg/runtime/runtime.go Outdated Show resolved Hide resolved
pkg/service/connect_service.go Outdated Show resolved Hide resolved
pkg/sync/file/filepath_sync.go Show resolved Hide resolved
pkg/sync/kubernetes/kubernetes_sync.go Outdated Show resolved Hide resolved
@toddbaert
Copy link
Member

toddbaert commented Feb 21, 2023

@thisthat I like your API changes and this makes sense to me. I have a few thoughts in some of the sync implementations which I would like your feedback on, but none of them seems like a blocker to me.

@toddbaert
Copy link
Member

toddbaert commented Feb 23, 2023

@thisthat I pushed a few small changes to the sync tests which improves those tests (they were previously testing private funcs in some cases) and hopefully fixes the codecov faliure. I hope you dont mind.

EDIT: codecov is nearly passing. I will add a tiny bit more.

EDIT 2: there we go! Passing now

Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

Approved, pending @thisthat confirms he's OK with my test changes: #418 (comment)

@thisthat
Copy link
Member Author

@thisthat I pushed a few small changes to the sync tests which improves those tests (they were previously testing private funcs in some cases) and hopefully fixes the codecov faliure. I hope you dont mind.

EDIT: codecov is nearly passing. I will add a tiny bit more.

EDIT 2: there we go! Passing now

Sure thing I am happy with your changes @toddbaert - that's why "allow edits by maintainers" is checked 😉 thanks for bumping the coverage 🙌

Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan left a comment

Choose a reason for hiding this comment

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

Looks good and approving with minor chage suggestion

pkg/runtime/runtime.go Outdated Show resolved Hide resolved
Signed-off-by: Giovanni Liva <[email protected]>
thisthat and others added 14 commits February 27, 2023 12:24
Signed-off-by: Giovanni Liva <[email protected]>
Signed-off-by: Giovanni Liva <[email protected]>
Signed-off-by: Giovanni Liva <[email protected]>
Signed-off-by: Giovanni Liva <[email protected]>
Signed-off-by: Giovanni Liva <[email protected]>
Signed-off-by: Giovanni Liva <[email protected]>
Signed-off-by: Giovanni Liva <[email protected]>
Signed-off-by: Giovanni Liva <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
@toddbaert toddbaert merged commit 7f2358c into open-feature:main Feb 27, 2023
beeme1mr pushed a commit that referenced this pull request Mar 2, 2023
🤖 I have created a release *beep* *boop*
---


##
[0.4.0](v0.3.7...v0.4.0)
(2023-03-02)


### ⚠ BREAKING CHANGES

* Use OTel to export metrics (metric name changes)
([#419](#419))

### 🧹 Chore

* add additional sections to the release notes
([#449](#449))
([798f71a](798f71a))
* attach image sbom to release artefacts
([#407](#407))
([fb4ee50](fb4ee50))
* **deps:** update actions/configure-pages digest to fc89b04
([#417](#417))
([04014e7](04014e7))
* **deps:** update amannn/action-semantic-pull-request digest to b6bca70
([#441](#441))
([ce0ebe1](ce0ebe1))
* **deps:** update docker/login-action digest to ec9cdf0
([#437](#437))
([2650670](2650670))
* **deps:** update docker/metadata-action digest to 3343011
([#438](#438))
([e7ebf32](e7ebf32))
* **deps:** update github/codeql-action digest to 32dc499
([#439](#439))
([f91d91b](f91d91b))
* **deps:** update google-github-actions/release-please-action digest to
d3c71f9 ([#406](#406))
([6e1ffb2](6e1ffb2))
* disable caching tests in CI
([#442](#442))
([28a35f6](28a35f6))
* fix race condition on init read
([#409](#409))
([0c9eb23](0c9eb23))
* integration test stability
([#432](#432))
([5a6a5d5](5a6a5d5))
* integration tests
([#312](#312))
([6192ac8](6192ac8))
* reorder release note sections
([df7bfce](df7bfce))
* use -short flag in benchmark tests
([#431](#431))
([e68a6aa](e68a6aa))


### 🐛 Bug Fixes

* **deps:** update kubernetes packages to v0.26.2
([#450](#450))
([2885227](2885227))
* **deps:** update module github.com/bufbuild/connect-go to v1.5.2
([#416](#416))
([feb7f04](feb7f04))
* **deps:** update module
github.com/open-feature/go-sdk-contrib/providers/flagd to v0.1.9
([#427](#427))
([42d2705](42d2705))
* **deps:** update module github.com/open-feature/open-feature-operator
to v0.2.29 ([#429](#429))
([b7fae81](b7fae81))
* **deps:** update module github.com/stretchr/testify to v1.8.2
([#440](#440))
([ab3e674](ab3e674))
* **deps:** update module golang.org/x/net to v0.7.0
([#410](#410))
([c6133b6](c6133b6))
* **deps:** update module sigs.k8s.io/controller-runtime to v0.14.5
([#454](#454))
([f907f11](f907f11))
* remove non-error error log from parseFractionalEvaluationData
([#446](#446))
([34aca79](34aca79))


### ✨ New Features

* add debug logging for merge behaviour
([#456](#456))
([dc71e84](dc71e84))
* add Health and Readiness probes
([#418](#418))
([7f2358c](7f2358c))
* Add version to startup message
([#430](#430))
([8daf613](8daf613))
* introduce flag merge behaviour
([#414](#414))
([524f65e](524f65e))
* introduce grpc sync for flagd
([#297](#297))
([33413f2](33413f2))
* refactor and improve K8s sync provider
([#443](#443))
([4c03bfc](4c03bfc))
* Use OTel to export metrics (metric name changes)
([#419](#419))
([eb3982a](eb3982a))


### 📚 Documentation

* add .net flagd provider
([73d7840](73d7840))
* configuration merge docs
([#455](#455))
([6cb66b1](6cb66b1))
* documentation for creating a provider
([#413](#413))
([d0c099d](d0c099d))
* updated filepaths for schema store regex
([#344](#344))
([2d0e9d9](2d0e9d9))

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

5 participants