Skip to content

[FEATURE] Make offline file usage an own provider type #1504

Open
Open
[FEATURE] Make offline file usage an own provider type#1504
Labels
enhancementNew feature or request

Description

@aepfli
aepfli
opened on Jan 9, 2025

Requirements

Currently, we have two modes: RPC and in-process, and in-process has two different sources: GRPC and file. Initializing in-process GRPC uses many settings, such as host, port, certPath, socket path, connection settings, etc. In contrast, the in-process file provider only needs a file and a polling interval. This confuses users, and the difference for those settings is not 100% clear. We could simplify the API surface and introduce a third provider type, "FILE," representing the offline path.

Proposal

A file-based resolution must be implicitly in-process (It doesn't connect to anything). It needs a minimum of settings, and we can improve our testing significantly. We also want to run all kinds of evaluations, events, and mismatch tests for the file version, making those implementations and implications more transparent.

Add another ProviderType called "FILE".
This provider only needs 4 properties:

  • deadlineMs
  • retryGracePeriod
  • offlineFlagSourcePath (maybe worth renaming to filePath)
  • offlinePollIntervalMs (maybe worth renaming to filePollIntervalMs)

new Config table

Option name Environment variable name Explanation Type & Values Default Compatible resolver
resolver FLAGD_RESOLVER mode of operation String - rpc, in-process rpc rpc & in-process
host FLAGD_HOST remote host String localhost rpc & in-process
port FLAGD_PORT remote port int 8013 (rpc), 8015 (in-process) rpc & in-process
targetUri FLAGD_TARGET_URI alternative to host/port, supporting custom name resolution string null rpc & in-process
tls FLAGD_TLS connection encryption boolean false rpc & in-process
socketPath FLAGD_SOCKET_PATH alternative to host port, unix socket String null rpc & in-process
certPath FLAGD_SERVER_CERT_PATH tls cert path String null rpc & in-process
deadlineMs FLAGD_DEADLINE_MS deadline for unary calls, and timeout for initialization int 500 rpc & in-process & file
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
retryGracePeriod FLAGD_RETRY_GRACE_PERIOD period in seconds before provider moves from STALE to ERROR state int 5 rpc & in-process & file
keepAliveTime FLAGD_KEEP_ALIVE_TIME_MS http 2 keepalive long 0 rpc & in-process
cache FLAGD_CACHE enable cache of static flags String - lru, disabled lru rpc
maxCacheSize FLAGD_MAX_CACHE_SIZE max size of static flag cache int 1000 rpc
selector FLAGD_SOURCE_SELECTOR selects a single sync source to retrieve flags from only that source string null in-process
offlineFlagSourcePath FLAGD_OFFLINE_FLAG_SOURCE_PATH offline, file-based flag definitions, overrides host/port/targetUri string null in-process file
offlinePollIntervalMs FLAGD_OFFLINE_POLL_MS poll interval for reading offlineFlagSourcePath int 5000 in-process file
contextEnricher - sync-metadata to evaluation context mapping function function identity function in-process

ensure backward compatibility

As some providers already implement offline file paths, we could implement logic that sets the provider type to FILE when the property for offline_source_file is set. This should be fairly easy to achieve, as we have good abstraction in most places.

if offlineFlagSourcePath is set:
    providerType = "FILE"

wdyt?

Activity

  1. aepfli
    added
    enhancementNew feature or request
    Needs TriageThis issue needs to be investigated by a maintainer
    on Jan 9, 2025
  2. aepfli
    changed the title [-][FEATURE] Make offline file usage an own provider[/-] [+][FEATURE] Make offline file usage an own provider type[/+] on Jan 9, 2025

  3. beeme1mr commented on Jan 13, 2025

    @beeme1mr
    beeme1mr
    on Jan 13, 2025
    Member

    How much effort do you think this will be? It sounds relatively useful but I'm not sure it's worth the effort.

  4. aepfli commented on Jan 17, 2025

    @aepfli
    aepfli
    on Jan 17, 2025
    MemberAuthor

    open-feature-forking/java-sdk-contrib#1 this pr shows the efforts for java, with backwards compatibility

  5. aepfli commented on Jan 19, 2025

    @aepfli
    aepfli
    on Jan 19, 2025
    MemberAuthor
    
  @file
  Scenario Outline: File Backwards compatibility
    Given an option "resolver" of type "ResolverType" with value "<resolverSet>"
    And an option "offlineFlagSourcePath" of type "String" with value "some-path"
    When a config was initialized
    Then the option "resolver" of type "ResolverType" should have the value "<resolverExpected>"
    Scenarios:
        | resolverSet | resolverExpected |
        | in-process  | file             |
        | rpc         | rpc              |
        | file        | file             |

  @file
  Scenario: Default Config File
    Given an option "resolver" of type "ResolverType" with value "file"
    When a config was initialized
    Then we should have an error

    possible gherkin tests to ensure this behaviour, throughout all our providers.

  6. aepfli commented on Jan 19, 2025

    @aepfli
    aepfli
    on Jan 19, 2025
    MemberAuthor

    i created a fully fledged demo for java - with gherkin testbed tests, to show how this looks like for java. i do think, that this is a easy to do change, which allows us to treat those providers and logics seperately:

  7. toddbaert commented on Jan 20, 2025

    @toddbaert
    toddbaert
    on Jan 20, 2025
    Member

    How much effort do you think this will be? It sounds relatively useful but I'm not sure it's worth the effort.

    open-feature-forking/java-sdk-contrib#1 this pr shows the efforts for java, with backwards compatibility

    This seems like a pretty small change:

    Image

  8. toddbaert commented on Jan 20, 2025

    @toddbaert
    toddbaert
    on Jan 20, 2025
    Member

    I'm definitely in favor of this. Here is my simple reasoning:

    • @aepfli has proved the implementation is pretty low-effort
    • we've taking the time to make the file source work well (initially it was just used for testing) but due to complicated naming, I think it's extremely hard to discover
    • the new configuration clarifies the intent quite a bit
  9. toddbaert
    removed
    Needs TriageThis issue needs to be investigated by a maintainer
    on Jan 20, 2025

  10. aepfli commented on Jan 21, 2025

    @aepfli
    aepfli
    on Jan 21, 2025
    MemberAuthor

    so i can move forward with the migration?

  11. beeme1mr commented on Jan 21, 2025

    @beeme1mr
    beeme1mr
    on Jan 21, 2025
    Member

    so i can move forward with the migration?

    Yes, I'm okay with this change. Thanks for the proposal and Java example.

  12. aepfli
    added a commit that references this issue on Jan 28, 2025

    feat: migrate file evaluation to own provider type (#1525)

    f298866

  13. aepfli commented on Feb 8, 2025

    @aepfli
    aepfli
    on Feb 8, 2025
    MemberAuthor

    TODO: create tasks for all flagd provider implementations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions