js-sdk-contrib icon indicating copy to clipboard operation
js-sdk-contrib copied to clipboard

feat(flagd): Add flagd-selector gRPC metadata header support for in-process sync

Open Copilot opened this issue 1 month ago • 3 comments

Plan: Update selector handling for consistency with sync service (in-process mode only)

Context

Following open-feature/flagd#1814, normalizing selector usage for the sync service (in-process resolver mode) to use flagd-selector gRPC metadata header instead of request field.

Important: This change only affects in-process resolver mode, not RPC mode.

Changes Implemented

  • [x] Audit current selector usage - confirmed only affects in-process/sync mode
  • [x] Update GrpcFetch to send selector via flagd-selector gRPC metadata header
  • [x] Maintain backward compatibility by sending both field and header during deprecation period
  • [x] Add tests for new header-based selector handling (2 test cases)
  • [x] Document the change in README with deprecation notice
  • [x] Fix code formatting with prettier
  • [x] Refactor tests to use async/await pattern

Technical Implementation

Modified files:

  1. libs/providers/flagd/src/lib/service/in-process/grpc/grpc-fetch.ts:

    • Added Metadata import from @grpc/grpc-js
    • Added private _metadata field to store gRPC metadata
    • Populate metadata with flagd-selector header in constructor when selector is configured
    • Pass metadata to syncFlags() gRPC call
    • Keep selector in request field for backward compatibility
  2. libs/providers/flagd/src/lib/service/in-process/grpc/grpc-fetch.spec.ts:

    • Added test: "should send selector via flagd-selector metadata header"
    • Added test: "should handle empty selector gracefully"
    • Both tests verify request field and metadata header are populated correctly
    • Refactored to use async/await instead of done callback for better readability
  3. libs/providers/flagd/README.md:

    • Added "Selector (in-process mode only)" section with usage example
    • Documented dual implementation approach (header + field)
    • Added deprecation notice referencing flagd#1814
    • Clarified future removal of request field in next major version

Backward Compatibility

The implementation sends the selector in both places during the deprecation period:

  • New: flagd-selector gRPC metadata header (will be the only method in future)
  • Legacy: selector field in SyncFlagsRequest (deprecated, maintained for compatibility)

This ensures compatibility with:

  • Newer flagd versions that prefer the metadata header
  • Older flagd versions that only support the request field

Next Steps (future work, not in this PR)

  • Monitor flagd release timeline for selector field deprecation
  • Remove request field in next major version after deprecation period ends
Original prompt

This section details on the original issue you should resolve

<issue_title>[flagd] Update selector handling for consistency with sync, evaluation, and OFREP services</issue_title> <issue_description>### Context This ticket follows open-feature/flagd#1814, which aims to normalize selector usage across all flagd services. Currently, selectors are handled differently in sync, evaluation, and OFREP services.

Requirements

  • Reference: https://github.com/open-feature/flagd/issues/1814
  • Ensure selector handling is consistent between js-sdk-contrib and core flagd, especially for clients and plugins interfacing with flagd.
  • Ensure backwards compatibility, supporting the deprecated selector until the deprecation period ends.

Tasks

  1. Audit all usages of selector in js-sdk-contrib (including SDK components interacting with flagd)
  2. Update to prefer flagd-selector header when communicating with flagd services
  3. Ensure backward compatibility where needed
  4. Document selector handling patterns for maintainers and users
  5. Prepare for future removal of deprecated behaviors

Benefits

  • Predictable and consistent selector support when integrating with flagd
  • Lower maintenance burden for migration in future major releases

Additional notes

  • Work in alignment with the migration guidance and deprecation plan discussed in the core flagd ticket. </issue_description>

Comments on the Issue (you are @copilot in this section)

  • Fixes open-feature/js-sdk-contrib#1398

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot avatar Nov 05 '25 15:11 Copilot

/Gemini review

aepfli avatar Nov 05 '25 17:11 aepfli

medium

The test uses the done callback, which can make asynchronous tests more complex and harder to read due to the try/catch blocks and manual done() calls. It's generally recommended to use async/await for handling asynchronous operations in Jest tests. This simplifies the code, improves readability, and makes error handling more straightforward.

  it('should send selector via flagd-selector metadata header', async () => {
    const selector = 'app=weather';
    const cfgWithSelector: Config = { ...cfg, selector };
    const flagConfiguration = '{"flags":{}}';

    const fetch = new GrpcFetch(cfgWithSelector, serviceMock);
    const connectPromise = fetch.connect(dataCallback, reconnectCallback, jest.fn(), disconnectCallback);
    onDataCallback({ flagConfiguration });
    await connectPromise;

    // Verify syncFlags was called
    expect(serviceMock.syncFlags).toHaveBeenCalled();

    // Check that both request and metadata were passed
    const callArgs = (serviceMock.syncFlags as jest.Mock).mock.calls[0];
    expect(callArgs).toHaveLength(2);

    // Verify the request contains selector (for backward compatibility)
    expect(callArgs[0].selector).toBe(selector);

    // Verify the metadata contains flagd-selector header
    const metadata = callArgs[1] as Metadata;
    expect(metadata).toBeDefined();
    expect(metadata.get('flagd-selector')).toEqual([selector]);
  });

@copilot can you improve the tests please based on the example above, using async await

aepfli avatar Nov 06 '25 12:11 aepfli

![medium](https://camo.githubusercontent.com/a33c7ba47779a2aabda978d90fc2665dd949d88da87f5ff554e65308de059f02/68747470733a2f2f7777772e677374617469632e636f6d2f636f64657265766965776167656e742f6d656469...

Refactored both tests to use async/await pattern in commit 323bb33. The tests are now cleaner and more readable without the try/catch blocks and manual done() calls.

Copilot avatar Nov 06 '25 12:11 Copilot