open-feature / spec

OpenFeature specification
https://openfeature.dev
Apache License 2.0
779 stars 36 forks source link

feat: Add hook-data concept for hooks. #273

Open kinyoklion opened 2 months ago

kinyoklion commented 2 months ago

This PR

Add support for the hook-data concept for hooks. Hook-data allows for per-evaluation data to be propagated between hooks.

This is especially useful for analytics purposes where you may want to measure things that happen between stages, or you want to do something like create a span in one stage and close it in another.

This concept is similar to the series data concept for LaunchDarkly hooks. https://github.com/launchdarkly/open-sdk-specs/tree/main/specs/HOOK-hooks#evaluationseriesdata

Unlike series data the data in this approach is mutable. This is because the before stage already has a return value. We could workaround this by specifying a return structure, but it maybe seems more complex. The data is only passed to a specific hook instance, so mutability is not of great concern.

Some functional languages may still need to use an immutable with return values approach.

I can create an OFEP if we think this merits discussion prior to proposal.

Related Issues

Related discussion in a PR comment. https://github.com/open-feature/java-sdk/pull/1049#discussion_r1718895742

kinyoklion commented 1 month ago

I like this proposal, but I would like to see how it could be implemented in an SDK. Properly scoping hook data may be complicated.

I should be able to find time to update a OF SDK to demonstrate, but it may take me a bit.

It will be a little different now that we added it to hook context, but you can see what we did in the LD SDKs as its own argument: https://github.com/launchdarkly/js-core/blob/ae2b5bbed9ec32d9640f1c5a168badaad28174ec/packages/shared/sdk-server/src/hooks/HookRunner.ts#L109

In our case we create the data when we call the hook, and the return is the new data. Which means we can just map it to an array.

But, if that was not the case, as it will be in the OF example you instead do this.

  1. Create an array the same size as the number of hooks being invoked.
  2. As you iterate the hooks pass the data at the same index in the array as the hook is in its array.
  3. Maintain the array for the execution of that series of stages.

Some pseudocode:

const hookData = new Array(hooksForInvocation.length);
hookData.fill({})

...
hooksForInvocation.forEach((hook, index) => executeBeforeEvaluation(hookContext.withData(hookData[index])))

The hooks are stable for a specific invocation of stages, so you just need an equal sized list of hook data.