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: eventing implementation #188

Merged
merged 13 commits into from
Jul 14, 2023

Conversation

Kavindu-Dodan
Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan commented Jun 20, 2023

This PR

Covers the initial implementation of #186

⚠️ Note that, this implementation does not cover the 1 to N relationship requirement of provider to named client requirement (i.e - single instance of a provider can be bound to multiple names, thus allowing that single provider to be bound by multiple clients). A follow-up PR is ready for this - #190

This PR introduces OpenFeature eventing spec [1] to Go SDK. The implementation is non-breaking and providers which plan to support eventing can opt-in by implementing EventHandler interface.

Implementation

EventHandler interface contains a single method EventChannel() <- chan Event. OpenFeature core implementation performs a type assertion on the provider for EventHandler and listen to Event channel. It is provider implementation responsibility to emit spec-compliant events (ex:- ProviderReady when initialization is complete OR ProviderError when initialization failed)

The core event handling logic is concentrated on event_executor.go. OpenFeature core implementation maintains a single EventExecutor instance that handles both global (api) level events and client level (scoped) events.

Client and API both add and remove event handlers (callbacks) through their spec client accessors and these get stored at the global EventExecutor

flowchart TD
    A[OpenFeature] -->|Register Global Handler|D
    D[API Singleton] -->  B(EventExecutor)
    C[Client Instance] -->|Register Client Handler|D

Loading

When handling events from providers, all global-level handlers are always invoked. But, the client (scoped) handlers only run if they are associated with the specific provider. This is done by comparing event payload's provider name & registered client name of the handler

sequenceDiagram
    Provider->>+EventExecutor: Emit Event: ProviderReady
    EventExecutor-->>EventExecutor: Run Global Handlers
    EventExecutor-->>EventExecutor: Run Client Handlers - filter with provider name
Loading

Testing

Consider checking event_executor_test.go [2] to validate spec compliance as well as the correctness of the implementation

[1] - https://github.com/open-feature/spec/blob/main/specification/sections/05-events.md#5-events
[2] - https://github.com/open-feature/go-sdk/blob/fe93b46b4afc11533b0530314cc71a823abfe705/pkg/openfeature/event_executor_test.go

@Kavindu-Dodan Kavindu-Dodan requested a review from a team as a code owner June 20, 2023 21:29
@Kavindu-Dodan Kavindu-Dodan changed the title eventing implementation feat: eventing implementation Jun 20, 2023
@codecov
Copy link

codecov bot commented Jun 20, 2023

Codecov Report

Merging #188 (f98c9bb) into main (501c34b) will increase coverage by 4.05%.
The diff coverage is 90.55%.

@@            Coverage Diff             @@
##             main     #188      +/-   ##
==========================================
+ Coverage   72.97%   77.03%   +4.05%     
==========================================
  Files           8        9       +1     
  Lines         766      997     +231     
==========================================
+ Hits          559      768     +209     
- Misses        187      207      +20     
- Partials       20       22       +2     
Impacted Files Coverage Δ
pkg/openfeature/provider.go 68.00% <0.00%> (-32.00%) ⬇️
pkg/openfeature/api.go 90.09% <60.00%> (-4.81%) ⬇️
pkg/openfeature/openfeature.go 97.67% <93.33%> (-2.33%) ⬇️
pkg/openfeature/event_executor.go 96.33% <96.33%> (ø)
pkg/openfeature/client.go 70.44% <100.00%> (+0.24%) ⬆️

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

pkg/openfeature/api.go Outdated Show resolved Hide resolved
pkg/openfeature/api.go Outdated Show resolved Hide resolved
pkg/openfeature/event_handler.go Outdated Show resolved Hide resolved
pkg/openfeature/event_handler.go Outdated Show resolved Hide resolved
pkg/openfeature/event_handler.go Outdated Show resolved Hide resolved
pkg/openfeature/event_handler.go Outdated Show resolved Hide resolved
pkg/openfeature/event_handler.go Outdated Show resolved Hide resolved
pkg/openfeature/event_handler.go Outdated Show resolved Hide resolved
pkg/openfeature/event_handler.go Outdated Show resolved Hide resolved
pkg/openfeature/event_handler.go Outdated Show resolved Hide resolved
@toddbaert toddbaert self-requested a review June 27, 2023 16:37
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.

Looks good to me overall, with a few nits (assuming the 1:N issue is fixed elsewhere).

@Kavindu-Dodan
Copy link
Contributor Author

Looks good to me overall, with a few nits (assuming the 1:N issue is fixed elsewhere).

correct, 1:N support was missing in this implementation and I am actively working on enabling this.

Kavindu-Dodan and others added 13 commits July 14, 2023 14:20
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@beeme1mr beeme1mr merged commit 220dc33 into open-feature:main Jul 14, 2023
6 checks passed
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.

6 participants