Skip to content

Commit

Permalink
PR feedback.
Browse files Browse the repository at this point in the history 
Signed-off-by: Ryan Lamb <[email protected]>
  • Loading branch information
@kinyoklion
kinyoklion committed Oct 25, 2024
1 parent e7a7bf4 commit 29bc48c
Showing 1 changed file with 69 additions and 72 deletions.
141 changes: 69 additions & 72 deletions specification/sections/04-hooks.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,7 @@ Hooks can be configured to run globally (impacting all flag evaluations), per cl

### Definitions

**Hook**: Application author/integrator-supplied logic that is called by the OpenFeature framework at a specific stage.

**Stage**: An explicit portion of the flag evaluation lifecycle. e.g. `before` being "before the [resolution](../glossary.md#resolving-flag-values) is run.

**Invocation**: A single call to evaluate a flag. `client.getBooleanValue(..)` is an invocation.

**API**: The global API singleton.
**Hook**: Application author/integrator-supplied logic that is called by the OpenFeature framework at a specific stage. **Stage**: An explicit portion of the flag evaluation lifecycle. e.g. `before` being "before the [resolution](../glossary.md#resolving-flag-values) is run. **Invocation**: A single call to evaluate a flag. `client.getBooleanValue(..)` is an invocation. **API**: The global API singleton.

### 4.1. Hook context

Expand Down Expand Up @@ -87,42 +81,15 @@ see: [dynamic-context paradigm](../glossary.md#dynamic-context-paradigm)

> Condition: The provider `metadata` field in the `hook context` **MUST** be immutable.
### 4.3. Hook data

Hook data exists to allow hook stages to share data for a specific evaluation. For instance a span
for OpenTelemetry could be created in a `before` stage and closed in an `after` stage.

```Java
public Optional<EvaluationContext> before(HookContext context, HookHints hints, HookData data) {
SpanBuilder builder = tracer.spanBuilder('sample')
.setParent(Context.current().with(Span.current()));
Span span = builder.startSpan()
data.set("span", span);
}

public void after(HookContext context, FlagEvaluationDetails details, HookHints hints, HookData data) {
// Only accessible by this hook for this specific evaluation.
Object value = data.get("span");
if (value instanceof Span) {
Span span = (Span) value;
span.end();
}
}
```
### 4.3. Hook creation and parameters

#### Requirement 4.3.1

> `hook data` **MUST** be a structure supports definition of arbitrary properties, with keys of type `string`, and values of type `boolean | string | number | datetime | structure`.
### 4.4. Hook creation and parameters

#### Requirement 4.4.1

> Hooks **MUST** specify at least one stage.
#### Requirement 4.4.2
#### Requirement 4.3.2

> `Hook data` **MUST** must be created before the first `stage` invoked in a hook for a specific evaluation and propagated between each `stage` of the hook.
> `Hook data` **MUST** must be created before the first `stage` invoked in a hook for a specific evaluation and propagated between each `stage` of the hook. The hook data is not shared between different hooks.
Example showing data between `before` and `after` stage for two different hooks.
```mermaid
Expand All @@ -137,31 +104,31 @@ activate Client
Client-->>Client: create hook data for HookA
Client->>HookA: before(hookContext, data: {})
Client->>HookA: before(hookContext: {data: {}, ... })
activate HookA
HookA-->>HookA: data.set('key',' data for A')
HookA-->>HookA: hookContext.hookData.set('key',' data for A')
HookA-->>Client: (return)
deactivate HookA
Client-->>Client: create hook data for HookB
Client->>HookB: before(hookContext, hints, data: {})
Client->>HookB: before(hookContext: {data: {}, ... }, hints)
activate HookB
HookB-->>HookB: data.set('key', 'data for B')
HookB-->>HookB: hookContext.hookData.set('key', 'data for B')
deactivate HookB
Client-->>Client: Flag evaluation
Client->>HookB: after(hookContext, detail, hints, data: {key: 'data for B'})
Client->>HookB: after(hookContext: {data: {key: 'data for B'}, ... }, detail, hints)
activate HookB
HookB-->>Client: (return)
deactivate HookB
Client->>HookA: after(hookContext, data: {'key': 'data for A'})
Client->>HookA: after(hookContext: {data: {'key': 'data for A'}, ... })
activate HookA
HookA-->>Client: (return)
Expand All @@ -172,69 +139,69 @@ deactivate Client
```

#### Condition 4.4.2
#### Condition 4.3.2

> The implementation uses the dynamic-context paradigm.
see: [dynamic-context paradigm](../glossary.md#dynamic-context-paradigm)

##### Conditional Requirement 4.4.2.1
##### Conditional Requirement 4.3.2.1

> The `before` stage **MUST** run before flag resolution occurs. It accepts a `hook context` (required), `hook hints` (optional), and `hook data` (required) as parameters and returns either an `evaluation context` or nothing.
> The `before` stage **MUST** run before flag resolution occurs. It accepts a `hook context` (required) and `hook hints` (optional) as parameters and returns either an `evaluation context` or nothing.
```java
EvaluationContext | void before(HookContext hookContext, HookHints hints, HookData data);
EvaluationContext | void before(HookContext hookContext, HookHints hints);
```

#### Condition 4.4.3
#### Condition 4.3.3

[![experimental](https://img.shields.io/static/v1?label=Status&message=experimental&color=orange)](https://github.com/open-feature/spec/tree/main/specification#experimental)

> The implementation uses the static-context paradigm.
see: [static-context paradigm](../glossary.md#static-context-paradigm)

##### Conditional Requirement 4.4.3.1
##### Conditional Requirement 4.3.3.1

> The `before` stage **MUST** run before flag resolution occurs. It accepts a `hook context` (required), `hook hints` (optional), and `hook data` (required) as parameters. It has no return value.
> The `before` stage **MUST** run before flag resolution occurs. It accepts a `hook context` (required) and `hook hints` (optional) as parameters. It has no return value.
```java
void before(HookContext hookContext, HookHints hints, HookData data);
void before(HookContext hookContext, HookHints hints);
```

#### Requirement 4.4.4
#### Requirement 4.3.4

> Any `evaluation context` returned from a `before` hook **MUST** be passed to subsequent `before` hooks (via `HookContext`).
#### Requirement 4.4.5
#### Requirement 4.3.5

> When `before` hooks have finished executing, any resulting `evaluation context` **MUST** be merged with the existing `evaluation context`.
Evaluation context merge order is defined in [Context levels and merging](./03-evaluation-context.md#32-context-levels-and-merging).

#### Requirement 4.4.6
#### Requirement 4.3.6

> The `after` stage **MUST** run after flag resolution occurs. It accepts a `hook context` (required), `flag evaluation details` (required), `hook hints` (optional), and `hook data` (required). It has no return value.
> The `after` stage **MUST** run after flag resolution occurs. It accepts a `hook context` (required), `flag evaluation details` (required) and `hook hints` (optional). It has no return value.
#### Requirement 4.4.7
#### Requirement 4.3.7

> The `error` hook **MUST** run when errors are encountered in the `before` stage, the `after` stage or during flag resolution. It accepts `hook context` (required), `exception` representing what went wrong (required), `hook hints` (optional), and `hook data`. It has no return value.
> The `error` hook **MUST** run when errors are encountered in the `before` stage, the `after` stage or during flag resolution. It accepts `hook context` (required), `exception` representing what went wrong (required), and `hook hints` (optional). It has no return value.
#### Requirement 4.4.8
#### Requirement 4.3.8

> The `finally` hook **MUST** run after the `before`, `after`, and `error` stages. It accepts a `hook context` (required), `hook hints` (optional), and `hook data`. There is no return value.
> The `finally` hook **MUST** run after the `before`, `after`, and `error` stages. It accepts a `hook context` (required) and `hook hints` (optional). There is no return value.
#### Condition 4.4.9
#### Condition 4.3.9

> `finally` is a reserved word in the language.
##### Conditional Requirement 4.4.9.1
##### Conditional Requirement 4.3.9.1

> Instead of `finally`, `finallyAfter` **SHOULD** be used.
### 4.5. Hook registration & ordering
### 4.4. Hook registration & ordering

#### Requirement 4.5.1
#### Requirement 4.4.1

> The API, Client, Provider, and invocation **MUST** have a method for registering hooks.
Expand All @@ -251,7 +218,7 @@ client.addHooks(new Hook2());
client.getValue('my-flag', 'defaultValue', new Hook3());
```

#### Requirement 4.5.2
#### Requirement 4.4.2

> Hooks **MUST** be evaluated in the following order:
>
Expand All @@ -260,27 +227,27 @@ client.getValue('my-flag', 'defaultValue', new Hook3());
> - error (if applicable): Provider, Invocation, Client, API
> - finally: Provider, Invocation, Client, API

#### Requirement 4.5.3
#### Requirement 4.4.3

> If a `finally` hook abnormally terminates, evaluation **MUST** proceed, including the execution of any remaining `finally` hooks.

In languages with try/catch semantics, this means that exceptions thrown in `finally` hooks should be caught, and not propagated up the call stack.

#### Requirement 4.5.4
#### Requirement 4.4.4

> If an `error` hook abnormally terminates, evaluation **MUST** proceed, including the execution of any remaining `error` hooks.

In languages with try/catch semantics, this means that exceptions thrown in `error` hooks should be caught, and not propagated up the call stack.

#### Requirement 4.5.5
#### Requirement 4.4.5

> If an error occurs in the `before` or `after` hooks, the `error` hooks **MUST** be invoked.

#### Requirement 4.5.6
#### Requirement 4.4.6

> If an error occurs during the evaluation of `before` or `after` hooks, any remaining hooks in the `before` or `after` stages **MUST NOT** be invoked.

#### Requirement 4.5.7
#### Requirement 4.4.7

> If an error occurs in the `before` hooks, the default value **MUST** be returned.

Expand All @@ -299,14 +266,44 @@ val = client.get_boolean_value('my-key', False, evaluation_options={

see: [Flag evaluation options](./01-flag-evaluation.md#evaluation-options)

#### Requirement 4.6.1
#### Requirement 4.5.1

> `Flag evaluation options` **MAY** contain `hook hints`, a map of data to be provided to hook invocations.

#### Requirement 4.6.2
#### Requirement 4.5.2

> `hook hints` **MUST** be passed to each hook.

#### Requirement 4.6.3
#### Requirement 4.5.3

> The hook **MUST NOT** alter the `hook hints` structure.

### 4.6. Hook data

Hook data exists to allow hook stages to share data for a specific evaluation. For instance a span
for OpenTelemetry could be created in a `before` stage and closed in an `after` stage.

Hook data is scoped to a specific hook instance. The different stages of a hook share the same data,
but different hooks have different hook data instances.

```Java
public Optional<EvaluationContext> before(HookContext context, HookHints hints) {
SpanBuilder builder = tracer.spanBuilder('sample')
.setParent(Context.current().with(Span.current()));
Span span = builder.startSpan()
context.hookData.set("span", span);
}
public void after(HookContext context, FlagEvaluationDetails details, HookHints hints) {
// Only accessible by this hook for this specific evaluation.
Object value = context.hookData.get("span");
if (value instanceof Span) {
Span span = (Span) value;
span.end();
}
}
```

#### Requirement4.6.1

> `hook data` **MUST** be a structure supports definition of arbitrary properties, with keys of type `string`, and values of type `boolean | string | number | datetime | structure`.

0 comments on commit 29bc48c

Please sign in to comment.