feat: implement ADR-0008 event stream support in OpenAPI spec#67
feat: implement ADR-0008 event stream support in OpenAPI spec#67jonathannorris merged 15 commits intomainfrom
Conversation
- Bump OpenAPI version from 3.1.0 to 3.2.0 for text/event-stream support - Bump OFREP version from 0.2.0 to 0.3.0 - Add flagConfigEtag and flagConfigLastModified query parameters to both eval endpoints - Add eventStreams field to bulkEvaluationSuccess and serverEvaluationSuccess responses - Add eventStream schema with mutually exclusive url/endpoint fields - Add eventStreamEndpoint schema for structured origin + requestUri - Add sseEvent and sseEventData schemas for event stream payloads - Add webhook documenting text/event-stream content type (OAS 3.2.0) - Disable oas3-schema spectral rule until 3.2.0 support lands (stoplightio/spectral#2910) Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the OpenFeature Remote Evaluation Protocol (OFREP) by integrating event stream support, aligning with ADR-0008. It updates the OpenAPI specification to version 3.2.0 and the OFREP version to 0.3.0, enabling real-time change notifications for flag configurations. This allows providers to subscribe to event streams for immediate updates, moving beyond traditional polling mechanisms for detecting changes. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
There was a problem hiding this comment.
Code Review
This pull request successfully implements ADR-0008 by adding event stream support to the OpenAPI specification. The changes are well-structured, including the version bump to OpenAPI 3.2.0, the addition of new schemas for event streams, and new query parameters. My review includes a few suggestions to enhance maintainability and schema precision by reducing parameter duplication and adding more specific constraints to the new schemas.
I am having trouble creating individual review comments. Click here to see my feedback.
service/openapi.yaml (57-90)
To improve maintainability and reduce duplication, consider defining the flagConfigEtag and flagConfigLastModified parameters as reusable components under components.parameters. You can then reference them in both the /ofrep/v1/evaluate/flags/{key} and /ofrep/v1/evaluate/flags paths using $ref. This ensures consistency and simplifies future updates.
service/openapi.yaml (453-459)
The description states that currently only sse is a defined type. To make the schema more precise and self-documenting for the current version, consider constraining this field to the known value using enum.
type:
type: string
enum: ["sse"]
description: |
The connection type identifying the push mechanism to use.
Currently only `sse` is defined. Providers must ignore entries
with unknown types for forward compatibility.
example: "sse"service/openapi.yaml (474-485)
The description mentions that providers should default to 120 seconds for inactivityDelaySec if the value is not present. It's a good practice to explicitly define this default in the schema using the default keyword. This makes the API contract clearer for consumers.
inactivityDelaySec:
type: integer
minimum: 1
default: 120
description: |
Number of seconds of client inactivity (e.g., browser tab hidden,
mobile app backgrounded) after which the connection should be closed
to conserve resources. The client must reconnect and perform a full
unconditional re-fetch when activity resumes. When determining the
effective inactivity timeout, providers should use a client-side
override if configured; otherwise use this value when present;
otherwise default to 120 seconds.
example: 120service/openapi.yaml (524-529)
The description states that the SSE event type is always message. To enforce this and make the schema more strict, you can add an enum with the single allowed value.
event:
type: string
enum: ["message"]
description: |
The SSE event type. Always `message` for OFREP events. Providers
must inspect `data.type` rather than this field for event routing.
example: "message"service/openapi.yaml (550-555)
The description mentions that currently only refetchEvaluation is defined for the event type. To make the schema more precise for this version of the API, you can constrain this field to the known value using enum.
type:
type: string
enum: ["refetchEvaluation"]
description: |
The OFREP event type. Currently only `refetchEvaluation` is defined.
Providers must ignore unknown values for forward compatibility.
example: "refetchEvaluation"Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the OFREP specification to introduce Server-Sent Events (SSE) for real-time change notifications, primarily for client-side providers using bulk evaluation. Key changes include updating ADR-0008 to reflect this scope, bumping the OpenAPI version to 3.2.0, and adding the necessary schemas, parameters, and webhook definitions for event streams. Review feedback suggests adding a comment to explain the use of the non-standard OpenAPI 3.2.0 version for better maintainability and simplifying ETag examples by removing unnecessary double quotes to improve clarity for implementers.
There was a problem hiding this comment.
Pull request overview
Implements ADR-0008’s event stream (SSE) change-notification support in the OFREP OpenAPI spec for the bulk evaluation endpoint, including new schemas and request metadata for change-triggered re-fetches.
Changes:
- Bump OpenAPI version to 3.2.0 and OFREP version to 0.3.0.
- Add
eventStreamsto the bulk evaluation success response plus neweventStream/ SSE event payload schemas and atext/event-streamwebhook description. - Add
flagConfigEtag/flagConfigLastModifiedquery parameters for change-triggered bulk re-fetches; adjust Spectral rules to accommodate OAS 3.2.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| service/openapi.yaml | Adds bulk-eval event stream support (schemas + response field + query params) and documents SSE stream format; bumps OpenAPI/OFREP versions. |
| service/adrs/0008-sse-for-bulk-evaluation-changes.md | Aligns ADR scope to client-side bulk evaluation only and updates the documented schema additions accordingly. |
| .spectral.yaml | Disables a Spectral rule due to lack of OpenAPI 3.2 support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
toddbaert
left a comment
There was a problem hiding this comment.
Minor nit; looks great overall.
toddbaert
left a comment
There was a problem hiding this comment.
Approved, minor nit: #67 (comment)
Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
f882e1f to
3da9c7c
Compare
|
@lukas-reining / @thomaspoignant / @erka any further comments on this one? |
lukas-reining
left a comment
There was a problem hiding this comment.
Looks good so far, left some feedback.
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
|
/gemini review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request updates the SSE ADR and OpenAPI specification to scope real-time change notifications to static-context providers. It bumps the OpenAPI version to 3.2.0, introduces SSE-related schemas and parameters, and temporarily disables a Spectral rule. Feedback was provided to ensure ETag examples remain quoted to comply with RFC 7232.
Revert .spectral.yaml and service/openapi.yaml to main. These ADR-0008 changes belong in PR #67. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
…pec to 3.1.0 Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
|
Split the spec into two files to keep the main
|
lukas-reining
left a comment
There was a problem hiding this comment.
Looks good to me, thanks @jonathannorris!
Summary
POST /ofrep/v1/evaluate/flags)eventStreamsis intentionally limited tobulkEvaluationSuccess— SSE support for dynamic-context providers using single-flag evaluations is deferred to a follow-up ADReventStreamstobulkEvaluationSuccess,flagConfigEtag/flagConfigLastModifiedquery parameters to the bulk eval endpoint, and new schemas for event stream connections and event payloadsevent-streams.yaml(OAS 3.2.0) documenting thetext/event-stream+itemSchemaevent format, while keeping the mainopenapi.yamlat OAS 3.1.0 with full spectral lintingNotes
eventStreamschema enforces mutual exclusivity betweenurlandendpointusingoneOf+notclausesendpointprovides a structuredrequestUri(required) and optionaloriginfor proxied deployments that need to override the origin while preserving the request target; iforiginis absent, providers use their configured OFREP base URLeventStreamsis designed to be extensible beyond SSEoas3-schemalinting re-enabled; the OAS 3.2.0text/event-stream+itemSchemarepresentation lives inservice/event-streams.yamlRelated Issues