Skip to content

fix: provider state ownership via events#385

Open
toddbaert wants to merge 1 commit into
mainfrom
fix/provider-event-derived-state
Open

fix: provider state ownership via events#385
toddbaert wants to merge 1 commit into
mainfrom
fix/provider-event-derived-state

Conversation

@toddbaert
Copy link
Copy Markdown
Member

@toddbaert toddbaert commented May 18, 2026

Providers emit events to signal all status transitions; the SDK derives
provider status from these events rather than inferring from lifecycle
method return values.

  • Add section 2.8 (Provider status) with requirements 2.8.1-2.8.4
  • Update 1.7.3/4/5 to reference provider events instead of init outcomes
  • Change 5.1.1 from MAY to MUST; reframe 5.3.1/2 as event-driven
  • Update 5.3.5 table (PROVIDER_CONTEXT_CHANGED -> READY, PROVIDER_RECONCILING -> RECONCILING)
  • Add non-normative threading note to 5.3.3
  • Add Appendix E (Migrations) for legacy provider compatibility
  • Fix lint check for non-requirement entries

Resolves #365


This is an alternative to #380 which took a different approach (adding getState() to providers). That one works but is ~10x larger and introduces concurrent-safety requirements on the provider that don't exist if state lives only in the SDK. Same problem, simpler solution IMO. Happy to discuss tradeoffs.

@toddbaert toddbaert requested a review from a team as a code owner May 18, 2026 15:54
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the OpenFeature specification to shift the responsibility of emitting lifecycle events from the SDK to the providers. By requiring providers to emit events for status transitions such as initialization and context reconciliation, the SDK can derive provider status directly from the event stream, which helps prevent race conditions in multi-threaded environments. The changes include updated requirements, a new migration guide in Appendix E, and documentation adjustments. Reviewers identified an inconsistency where 'shutdown' was included in lists of methods requiring event emission despite the lack of a defined shutdown event; they provided suggestions to remove 'shutdown' from these requirements and ensure consistent terminology for context changes.

Comment thread specification.json
{
"id": "Requirement 2.8.1",
"machine_id": "requirement_2_8_1",
"content": "The provider MUST emit an event to signal each status transition, including transitions resulting from lifecycle methods (`initialize`, `shutdown`, `on context change`) and spontaneous transitions.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The inclusion of shutdown in the list of lifecycle methods that MUST emit an event is inconsistent with the rest of the specification. There is no defined event for shutdown (e.g., PROVIDER_NOT_READY), and Requirement 1.7.9 states that the SDK updates the status to NOT_READY when the shutdown function terminates. Additionally, the PR description notes that shutdown remains unchanged and the SDK infers the state. Also, the method name should be on context changed to match Requirement 2.6.1.

Suggested change
"content": "The provider MUST emit an event to signal each status transition, including transitions resulting from lifecycle methods (`initialize`, `shutdown`, `on context change`) and spontaneous transitions.",
"content": "The provider MUST emit an event to signal each status transition, including transitions resulting from lifecycle methods (initialize, on context changed) and spontaneous transitions.",


#### Requirement 2.8.1

> The provider **MUST** emit an event to signal each status transition, including transitions resulting from lifecycle methods (`initialize`, `shutdown`, `on context change`) and spontaneous transitions.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The inclusion of shutdown here is inconsistent with the fact that no shutdown event is defined in the specification. Requirement 1.7.9 confirms that the SDK manages the transition to NOT_READY upon termination of the shutdown function. Also, the method name should be on context changed for consistency with Requirement 2.6.1.

Suggested change
> The provider **MUST** emit an event to signal each status transition, including transitions resulting from lifecycle methods (`initialize`, `shutdown`, `on context change`) and spontaneous transitions.
The provider MUST emit an event to signal each status transition, including transitions resulting from lifecycle methods (initialize, on context changed) and spontaneous transitions.


Providers cannot emit `PROVIDER_CONTEXT_CHANGED` or `PROVIDER_RECONCILING` events.
These are emitted only by the SDK during context reconciliation.
Providers must emit events to signal all state transitions, including those resulting from lifecycle methods (initialize, shutdown, reconciliation).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

As noted in other sections, shutdown does not have a corresponding event and its state transition is inferred by the SDK when the method terminates (per Requirement 1.7.9). It should be removed from this list of examples.

Suggested change
Providers must emit events to signal all state transitions, including those resulting from lifecycle methods (initialize, shutdown, reconciliation).
Providers must emit events to signal all state transitions, including those resulting from lifecycle methods (initialize, reconciliation).

Providers emit events to signal all status transitions; the SDK derives
provider status from these events rather than inferring from lifecycle
method return values.

- Add section 2.8 (Provider status) with requirements 2.8.1-2.8.4
- Add Condition 2.8.5 for simple providers (no lifecycle, no events)
- Update 1.7.3/4/5 to reference provider events instead of init outcomes
- Change 5.1.1 from MAY to MUST; reframe 5.3.1/2 as event-driven
- Update 5.3.5 table (PROVIDER_CONTEXT_CHANGED -> READY, PROVIDER_RECONCILING -> RECONCILING)
- Add non-normative threading note to 5.3.3
- Add Appendix E (Migrations) for legacy provider compatibility
- Fix lint check for non-requirement entries

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert force-pushed the fix/provider-event-derived-state branch from 3177a63 to 4734a58 Compare May 18, 2026 16:28
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.

Provider lifecycle race condition in multi-threaded SDKs

1 participant