docs: update flagd provider specs #1432

toddbaert · 2024-10-28T18:43:38Z

This PR contains significant enhancements to flagd provider specs. If merged, I will be opening a bunch of issues to implement what's described here based on recon I've been doing with the existing implementations

Specifically:

adds retryGraceAttempts param, which defines the amount of stream retry attempts before provider moves from STALE to ERROR state
adds contextEnricher param, which defines mapping function for sync-metadata to evaluation context for in process providers (exists already in Java provider)
improves consistency between in-process and RPC stream reconnect behavior
simplifies provider doc and spec to remove duplication and improve readability

netlify · 2024-10-28T18:43:55Z

✅ Deploy Preview for polite-licorice-3db33c ready!

Name	Link
🔨 Latest commit	`075efb7`
🔍 Latest deploy log	https://app.netlify.com/sites/polite-licorice-3db33c/deploys/672275579bdd330008eb1037
😎 Deploy Preview	https://deploy-preview-1432--polite-licorice-3db33c.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

toddbaert · 2024-10-28T18:55:55Z

mkdocs.yml

+        'reference/specifications/rpc-providers.md': 'reference/specifications/providers.md#rpc-providers'
+        'reference/specifications/in-process-providers.md': 'reference/specifications/providers.md#in-process-providers'


Redirects from old pages.

toddbaert · 2024-10-28T18:56:32Z

.markdownlint-cli2.yaml

+  MD007:
+    indent: 4


mkdocs doesn't render nested lists properly unless they are double indented (4 spaces) so I've added this rule.

docs/reference/specifications/providers.md

toddbaert · 2024-10-28T18:57:59Z

docs/reference/specifications/providers.md

+| streamDeadlineMs   | FLAGD_STREAM_DEADLINE_MS   | deadline for streaming calls, useful as an application-layer keepalive          | int                      | 600000                        | rpc & in-process    |
+| retryBackoffMs     | FLAGD_RETRY_BACKOFF_MS     | initial backoff for stream retry                                                | int                      | 1000                          | rpc & in-process    |
+| retryBackoffMaxMs  | FLAGD_RETRY_BACKOFF_MAX_MS | maximum backoff for stream retry                                                | int                      | 120000                        | rpc & in-process    |
+| retryGraceAttempts | FLAGD_RETRY_GRACE_ATTEMPTS | amount of stream retry attempts before provider moves from STALE to ERROR state | int                      | 5                             | rpc & in-process    |


retryGraceAttempts is a new param, not yet implemented in any provider but I think this is nicer than the "silent first retry" we have in Java, and solves the same problem in a more sensible way.

Should the default be 5? With exponential backoff this would mean 31s.

Ya, 15-30 seemed correct to me, personally, since I think most services and infra would be able to cycle within that time. 5 was more or less based on that idea.

Open to other arguments here though!

toddbaert · 2024-10-28T18:58:55Z

docs/reference/specifications/providers.md

+In-process flagd providers also inject any properties returned by the [sync-metadata RPC response](./protos.md#getmetadataresponse) into the context.
+This allows for static properties defined in flagd to be added to in-process evaluations.
+If only a subset of the sync-metadata response is desired to be injected into the evaluation context, you can use the define a mapping function with the `contextEnricher` option.


We don't yet actually support the addition of arbitrary props into the evaluation context in flagd itself. If this is merged, I will create an issue for that.

Signed-off-by: Todd Baert <[email protected]>

guidobrei

Thank you @toddbaert for consolidating all the different provider implementations into this spec. 🥇

guidobrei · 2024-10-29T08:38:55Z

docs/reference/specifications/providers.md

+| streamDeadlineMs   | FLAGD_STREAM_DEADLINE_MS   | deadline for streaming calls, useful as an application-layer keepalive          | int                      | 600000                        | rpc & in-process    |
+| retryBackoffMs     | FLAGD_RETRY_BACKOFF_MS     | initial backoff for stream retry                                                | int                      | 1000                          | rpc & in-process    |
+| retryBackoffMaxMs  | FLAGD_RETRY_BACKOFF_MAX_MS | maximum backoff for stream retry                                                | int                      | 120000                        | rpc & in-process    |
+| retryGraceAttempts | FLAGD_RETRY_GRACE_ATTEMPTS | amount of stream retry attempts before provider moves from STALE to ERROR state | int                      | 5                             | rpc & in-process    |


Should the default be 5? With exponential backoff this would mean 31s.

docs/reference/specifications/providers.md

guidobrei · 2024-10-29T08:43:39Z

docs/reference/specifications/providers.md

+            - RPC mode resolves `STALE` from cache where possible
+            - in-process mode resolves `STALE` from stored `flag set` rules
+- on stream reconnection:
+    - emit `PROVIDER_READY` and `PROVIDER_CONFIGURATION_CHANGED`


Should it emit PROVIDER_CONFIGURATION_CHANGED if we reconnect and the config did not change in the meantime?

Ya, we don't know if the config has changed, since we could have missed change events, so we fire a change regardless to make sure any change handlers run; change handlers are only a hook to cause additional evaluations, so if no changes have actually happened, it's not problem (flag values will just be the same).

This is how the Java provider currently works. We could just only run READY in this case, but IMO that's risky since it is possible that a missed change event would never be detected and handlers which re-evaluate flags never run, so I consumer stays out of sync.

docs/reference/specifications/providers.md

guidobrei · 2024-10-29T09:09:42Z

docs/reference/specifications/providers.md

+
+### Custom Name Resolution
+
+Some implementations support [gRPC custom name resolution](https://grpc.io/docs/guides/custom-name-resolution/), and abstractions to introduce additional resolvers.


Some implementations support...

In the provider spec we should clarify if this feature is optional. But if it's implemented it should be consistent across implementations.

Ya the main difficulty is not all gRPC implementations actually have this feature: https://grpc.io/docs/guides/custom-name-resolution/#language-support

docs/reference/specifications/providers.md

guidobrei · 2024-10-29T09:14:52Z

docs/reference/specifications/providers.md

+| retryBackoffMs     | FLAGD_RETRY_BACKOFF_MS     | initial backoff for stream retry                                                | int                      | 1000                          | rpc & in-process    |
+| retryBackoffMaxMs  | FLAGD_RETRY_BACKOFF_MAX_MS | maximum backoff for stream retry                                                | int                      | 120000                        | rpc & in-process    |
+| retryGraceAttempts | FLAGD_RETRY_GRACE_ATTEMPTS | amount of stream retry attempts before provider moves from STALE to ERROR state | int                      | 5                             | rpc & in-process    |
+| keepAliveTime      | FLAGD_KEEP_ALIVE_TIME_MS   | http 2 keepalive                                                                | long                     | 0                             | rpc & in-process    |


Do we still have HTTP 2 keepAlive support?

The gRPC keepalive is just an HTTP 2 keepalive.

Though considering how it didn't help us that much, we could consider not adding it at all. WDYT?

docs/reference/specifications/providers.md

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

Signed-off-by: Todd Baert <[email protected]>

toddbaert · 2024-10-29T20:17:55Z

Build failure is due to Trivvy rate limit.

Signed-off-by: Todd Baert <[email protected]>

toddbaert · 2024-10-30T16:14:49Z

docs/reference/specifications/providers.md

@@ -73,18 +35,19 @@ The lifecycle is summarized below:
    - if stream connection fails or exceeds the time specified by `deadline`, abort initialization (SDK will emit `PROVIDER_ERROR`), and attempt to [reconnect](#stream-reconnection)
 - while connected:
    - flags are resolved according to resolver mode; either by calling evaluation RPCs, or by evaluating the stored `flag set` rules
-    - for RPC providers, flags resolved with `reason=STATIC` are [cached](#flag-evaluation-caching)  
+    - for RPC providers, flags resolved with `reason=STATIC` are [cached](#flag-evaluation-caching)


This is under "while connected" so I think it's fine as is.

Signed-off-by: Todd Baert <[email protected]>

toddbaert requested a review from a team as a code owner October 28, 2024 18:43

dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Oct 28, 2024

toddbaert requested a review from beeme1mr October 28, 2024 18:43

toddbaert requested review from aepfli, craigpastro, DBlanchard88 and guidobrei October 28, 2024 18:44

toddbaert commented Oct 28, 2024

View reviewed changes

docs/reference/specifications/providers.md Outdated Show resolved Hide resolved

toddbaert commented Oct 28, 2024

View reviewed changes

toddbaert requested review from hairyhenderson and juanparadox October 28, 2024 19:02

toddbaert force-pushed the docs/flagd-spec-updates branch from bf6b566 to ce64a43 Compare October 28, 2024 19:40

toddbaert added 3 commits October 28, 2024 19:03

docs: update flagd provider specs

40b3f58

Signed-off-by: Todd Baert <[email protected]>

fixup: lifecycle summary

db9d89d

Signed-off-by: Todd Baert <[email protected]>

fixup: md lint

2e228f0

Signed-off-by: Todd Baert <[email protected]>

toddbaert force-pushed the docs/flagd-spec-updates branch from 5c55e9c to 2e228f0 Compare October 28, 2024 23:03

toddbaert requested review from bacherfl and Kavindu-Dodan October 28, 2024 23:17

guidobrei reviewed Oct 29, 2024

View reviewed changes

docs/reference/specifications/providers.md Outdated Show resolved Hide resolved

beeme1mr reviewed Oct 29, 2024

View reviewed changes

docs/reference/specifications/providers.md Outdated Show resolved Hide resolved

toddbaert and others added 5 commits October 29, 2024 09:30

Update docs/reference/specifications/providers.md

2840d38

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

Update docs/reference/specifications/providers.md

ef81f4a

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

Update docs/reference/specifications/providers.md

7095d0f

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

Update docs/reference/specifications/providers.md

a522c28

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

Update docs/reference/specifications/providers.md

d416de1

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

toddbaert and others added 2 commits October 29, 2024 09:49

Update docs/reference/specifications/providers.md

4683011

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

Update docs/reference/specifications/providers.md

460be57

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

toddbaert requested a review from pradeepbbl October 29, 2024 13:57

toddbaert and others added 2 commits October 29, 2024 09:59

Update docs/reference/specifications/providers.md

3978df7

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

Update docs/reference/specifications/providers.md

ba39b62

Co-authored-by: Guido Breitenhuber <[email protected]> Signed-off-by: Todd Baert <[email protected]>

toddbaert requested review from guidobrei and beeme1mr October 29, 2024 14:09

toddbaert added 3 commits October 29, 2024 10:10

fixup: pr feedback

8366519

Signed-off-by: Todd Baert <[email protected]>

fixup: FLAGD_TARGET_URI

17f79d5

Signed-off-by: Todd Baert <[email protected]>

fixup: links

fe9ad0b

Signed-off-by: Todd Baert <[email protected]>

pradeepbbl approved these changes Oct 30, 2024

View reviewed changes

fixup: feedback from mike

60f6231

Signed-off-by: Todd Baert <[email protected]>

toddbaert commented Oct 30, 2024

View reviewed changes

fixup: flags changed

3d5ae9d

Signed-off-by: Todd Baert <[email protected]>

beeme1mr approved these changes Oct 30, 2024

View reviewed changes

beeme1mr and others added 2 commits October 30, 2024 14:00

Merge branch 'main' into docs/flagd-spec-updates

bb1da79

fixup: flags changed++

075efb7

Signed-off-by: Todd Baert <[email protected]>

toddbaert merged commit a19cb42 into main Oct 30, 2024
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: update flagd provider specs #1432

docs: update flagd provider specs #1432

toddbaert commented Oct 28, 2024 •

edited

Loading

netlify bot commented Oct 28, 2024 •

edited

Loading

toddbaert Oct 28, 2024

toddbaert Oct 28, 2024

toddbaert Oct 28, 2024 •

edited

Loading

guidobrei Oct 29, 2024

toddbaert Oct 29, 2024

toddbaert Oct 28, 2024

guidobrei left a comment

guidobrei Oct 29, 2024

guidobrei Oct 29, 2024

toddbaert Oct 29, 2024 •

edited

Loading

guidobrei Oct 29, 2024

toddbaert Oct 29, 2024

guidobrei Oct 29, 2024

toddbaert Oct 29, 2024

toddbaert commented Oct 29, 2024 •

edited

Loading

toddbaert Oct 30, 2024

		'reference/specifications/rpc-providers.md': 'reference/specifications/providers.md#rpc-providers'
		'reference/specifications/in-process-providers.md': 'reference/specifications/providers.md#in-process-providers'


		### Custom Name Resolution

		Some implementations support [gRPC custom name resolution](https://grpc.io/docs/guides/custom-name-resolution/), and abstractions to introduce additional resolvers.

docs: update flagd provider specs #1432

docs: update flagd provider specs #1432

Conversation

toddbaert commented Oct 28, 2024 • edited Loading

netlify bot commented Oct 28, 2024 • edited Loading

✅ Deploy Preview for polite-licorice-3db33c ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toddbaert Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guidobrei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toddbaert Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toddbaert commented Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

toddbaert commented Oct 28, 2024 •

edited

Loading

netlify bot commented Oct 28, 2024 •

edited

Loading

toddbaert Oct 28, 2024 •

edited

Loading

toddbaert Oct 29, 2024 •

edited

Loading

toddbaert commented Oct 29, 2024 •

edited

Loading