-
Notifications
You must be signed in to change notification settings - Fork 10
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 hooks implementation #95
Open
MaximFischuk
wants to merge
7
commits into
open-feature:main
Choose a base branch
from
MaximFischuk:feature/hooks
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
b87c512
feat: Add implementation of Hooks
MaximFischuk 435f0c0
refactor: Remove unused submodules
MaximFischuk dfff03a
fix: Evaluation context passing on before hook
MaximFischuk 60ca477
test: Fix tests to do not use global OpenFeature singletone
MaximFischuk a2b6497
refactor: Update `finally` hook signature to be compliant to `Require…
MaximFischuk af0d637
refactor: Rename hooks field to client_hooks for clarity and consiste…
MaximFischuk cd0df01
chore: Correct typo in comment regarding resolution result in client.rs
MaximFischuk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
use open_feature::{ | ||
provider::{FeatureProvider, ProviderMetadata, ProviderStatus, ResolutionDetails}, | ||
EvaluationContext, EvaluationDetails, EvaluationError, EvaluationOptions, EvaluationResult, | ||
Hook, HookContext, HookHints, OpenFeature, StructValue, Value, | ||
}; | ||
|
||
struct DummyProvider(ProviderMetadata); | ||
|
||
impl Default for DummyProvider { | ||
fn default() -> Self { | ||
Self(ProviderMetadata::new("Dummy Provider")) | ||
} | ||
} | ||
|
||
#[async_trait::async_trait] | ||
impl FeatureProvider for DummyProvider { | ||
fn metadata(&self) -> &ProviderMetadata { | ||
&self.0 | ||
} | ||
|
||
fn status(&self) -> ProviderStatus { | ||
ProviderStatus::Ready | ||
} | ||
|
||
async fn resolve_bool_value( | ||
&self, | ||
_flag_key: &str, | ||
_evaluation_context: &EvaluationContext, | ||
) -> EvaluationResult<ResolutionDetails<bool>> { | ||
Ok(ResolutionDetails::new(true)) | ||
} | ||
|
||
async fn resolve_int_value( | ||
&self, | ||
_flag_key: &str, | ||
_evaluation_context: &EvaluationContext, | ||
) -> EvaluationResult<ResolutionDetails<i64>> { | ||
unimplemented!() | ||
} | ||
|
||
async fn resolve_float_value( | ||
&self, | ||
_flag_key: &str, | ||
_evaluation_context: &EvaluationContext, | ||
) -> EvaluationResult<ResolutionDetails<f64>> { | ||
unimplemented!() | ||
} | ||
|
||
async fn resolve_string_value( | ||
&self, | ||
_flag_key: &str, | ||
_evaluation_context: &EvaluationContext, | ||
) -> EvaluationResult<ResolutionDetails<String>> { | ||
unimplemented!() | ||
} | ||
|
||
async fn resolve_struct_value( | ||
&self, | ||
_flag_key: &str, | ||
_evaluation_context: &EvaluationContext, | ||
) -> Result<ResolutionDetails<StructValue>, EvaluationError> { | ||
unimplemented!() | ||
} | ||
} | ||
|
||
struct DummyLoggingHook(String); | ||
|
||
#[async_trait::async_trait] | ||
impl Hook for DummyLoggingHook { | ||
async fn before<'a>( | ||
&self, | ||
context: &HookContext<'a>, | ||
_hints: Option<&'a HookHints>, | ||
) -> Result<Option<EvaluationContext>, EvaluationError> { | ||
log::info!( | ||
"Evaluating({}) flag {} of type {}", | ||
self.0, | ||
context.flag_key, | ||
context.flag_type | ||
); | ||
|
||
Ok(None) | ||
} | ||
|
||
async fn after<'a>( | ||
&self, | ||
context: &HookContext<'a>, | ||
details: &EvaluationDetails<Value>, | ||
_hints: Option<&'a HookHints>, | ||
) -> Result<(), EvaluationError> { | ||
log::info!( | ||
"Flag({}) {} of type {} evaluated to {:?}", | ||
self.0, | ||
context.flag_key, | ||
context.flag_type, | ||
details.value | ||
); | ||
|
||
Ok(()) | ||
} | ||
|
||
async fn error<'a>( | ||
&self, | ||
context: &HookContext<'a>, | ||
error: &EvaluationError, | ||
_hints: Option<&'a HookHints>, | ||
) { | ||
log::error!( | ||
"Error({}) evaluating flag {} of type {}: {:?}", | ||
self.0, | ||
context.flag_key, | ||
context.flag_type, | ||
error | ||
); | ||
} | ||
|
||
async fn finally<'a>( | ||
&self, | ||
context: &HookContext<'a>, | ||
_: &EvaluationDetails<Value>, | ||
_hints: Option<&'a HookHints>, | ||
) { | ||
log::info!( | ||
"Finally({}) evaluating flag {} of type {}", | ||
self.0, | ||
context.flag_key, | ||
context.flag_type | ||
); | ||
} | ||
} | ||
|
||
#[tokio::main] | ||
async fn main() { | ||
env_logger::builder() | ||
.filter_level(log::LevelFilter::Info) | ||
.init(); | ||
|
||
let mut api = OpenFeature::singleton_mut().await; | ||
api.set_provider(DummyProvider::default()).await; | ||
api.add_hook(DummyLoggingHook("global".to_string())).await; | ||
drop(api); | ||
|
||
let client = OpenFeature::singleton() | ||
.await | ||
.create_client() | ||
.with_hook(DummyLoggingHook("client".to_string())); // Add a client-level hook | ||
|
||
let eval = EvaluationOptions::default().with_hook(DummyLoggingHook("eval".to_string())); | ||
let feature = client | ||
.get_bool_details("my_feature", None, Some(&eval)) | ||
.await | ||
.unwrap(); | ||
|
||
println!("Feature value: {}", feature.value); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,12 @@ use tokio::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}; | |
|
||
use crate::{ | ||
provider::{FeatureProvider, ProviderMetadata}, | ||
Client, EvaluationContext, | ||
Client, EvaluationContext, Hook, HookWrapper, | ||
}; | ||
|
||
use super::{ | ||
global_evaluation_context::GlobalEvaluationContext, provider_registry::ProviderRegistry, | ||
global_evaluation_context::GlobalEvaluationContext, global_hooks::GlobalHooks, | ||
provider_registry::ProviderRegistry, | ||
}; | ||
|
||
lazy_static! { | ||
|
@@ -21,6 +22,7 @@ lazy_static! { | |
#[derive(Default)] | ||
pub struct OpenFeature { | ||
evaluation_context: GlobalEvaluationContext, | ||
hooks: GlobalHooks, | ||
|
||
provider_registry: ProviderRegistry, | ||
} | ||
|
@@ -54,6 +56,12 @@ impl OpenFeature { | |
self.provider_registry.set_named(name, provider).await; | ||
} | ||
|
||
/// Add a new hook to the global list of hooks. | ||
pub async fn add_hook<T: Hook>(&mut self, hook: T) { | ||
let mut lock = self.hooks.get_mut().await; // TODO: Remove async context to avoid deadlock and enforce initialization before usage. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure about the TODO here. didn't you There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of my notes were accidentally published to PR, sorry, I'll remove them |
||
lock.push(HookWrapper::new(hook)); | ||
} | ||
|
||
/// Return the metadata of default (unnamed) provider. | ||
pub async fn provider_metadata(&self) -> ProviderMetadata { | ||
self.provider_registry | ||
|
@@ -77,6 +85,7 @@ impl OpenFeature { | |
Client::new( | ||
String::default(), | ||
self.evaluation_context.clone(), | ||
self.hooks.clone(), | ||
self.provider_registry.clone(), | ||
) | ||
} | ||
|
@@ -87,6 +96,7 @@ impl OpenFeature { | |
Client::new( | ||
name.to_string(), | ||
self.evaluation_context.clone(), | ||
self.hooks.clone(), | ||
self.provider_registry.clone(), | ||
) | ||
} | ||
|
@@ -156,6 +166,7 @@ mod tests { | |
// Set the new provider and ensure the value comes from it. | ||
let mut provider = MockFeatureProvider::new(); | ||
provider.expect_initialize().returning(|_| {}); | ||
provider.expect_hooks().return_const(vec![]); | ||
provider | ||
.expect_resolve_int_value() | ||
.return_const(Ok(ResolutionDetails::new(200))); | ||
|
@@ -203,6 +214,7 @@ mod tests { | |
// Bind provider to the same name. | ||
let mut provider = MockFeatureProvider::new(); | ||
provider.expect_initialize().returning(|_| {}); | ||
provider.expect_hooks().return_const(vec![]); | ||
provider | ||
.expect_resolve_int_value() | ||
.return_const(Ok(ResolutionDetails::new(30))); | ||
|
@@ -246,12 +258,14 @@ mod tests { | |
|
||
let mut default_provider = MockFeatureProvider::new(); | ||
default_provider.expect_initialize().returning(|_| {}); | ||
default_provider.expect_hooks().return_const(vec![]); | ||
default_provider | ||
.expect_resolve_int_value() | ||
.return_const(Ok(ResolutionDetails::new(100))); | ||
|
||
let mut named_provider = MockFeatureProvider::new(); | ||
named_provider.expect_initialize().returning(|_| {}); | ||
named_provider.expect_hooks().return_const(vec![]); | ||
named_provider | ||
.expect_resolve_int_value() | ||
.return_const(Ok(ResolutionDetails::new(200))); | ||
|
@@ -314,6 +328,7 @@ mod tests { | |
// Setup expectations for different evaluation contexts. | ||
let mut provider = MockFeatureProvider::new(); | ||
provider.expect_initialize().returning(|_| {}); | ||
provider.expect_hooks().return_const(vec![]); | ||
|
||
provider | ||
.expect_resolve_int_value() | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think something went wrong here 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, TOML formatter did weird thing :)