Skip to content

feat: expose js sdk identity on client metadata#1376

Draft
jonathannorris wants to merge 5 commits intomainfrom
feat/client-metadata-sdk-identity
Draft

feat: expose js sdk identity on client metadata#1376
jonathannorris wants to merge 5 commits intomainfrom
feat/client-metadata-sdk-identity

Conversation

@jonathannorris
Copy link
Copy Markdown
Member

@jonathannorris jonathannorris commented Apr 8, 2026

Summary

  • split ClientMetadata into separate JS package identity and runtime identity fields
  • set sdk to js-web / js-server, and add paradigm as client / server
  • keep framework metadata as-is for react, angular, and nest
  • avoid redundant Angular directive client initialization when the domain input already initialized the client

Motivation

Relates to #1375.

The original version of this PR overloaded sdk to represent runtime identity. I think it probably makes sense to keep those concerns separate:

  • sdk identifies the JS package family: js-web or js-server
  • paradigm identifies the runtime model: client or server
  • framework continues to identify wrapper SDKs like react, angular, and nest

This keeps the metadata more explicit and avoids conflating package family with runtime paradigm.

Usage

const client = OpenFeature.getClient('my-domain');

client.metadata;
// web-sdk:
// {
//   domain: 'my-domain',
//   sdk: 'js-web',
//   paradigm: 'client',
//   providerMetadata: { name: '...' }
// }
hook.before = (hookContext) => {
  hookContext.clientMetadata;
  // react-sdk:
  // {
  //   domain: 'my-domain',
  //   sdk: 'js-web',
  //   paradigm: 'client',
  //   framework: 'react',
  //   providerMetadata: { name: '...' }
  // }
};

Implementation

  • ClientMetadata now exposes both sdk and paradigm
  • the base web/server clients populate those fields directly
  • the framework SDKs still use the small Proxy wrapper so framework metadata is visible both via client.metadata and from evaluations that read this.metadata
  • Angular keeps its existing service/client behavior; I did not cache wrapped clients there because OpenFeature.getClient() in this SDK returns a new client instance rather than a singleton

Notes

  • the Angular directive still initializes from the domain input setter, but now avoids the redundant second initClient() call from ngOnInit()
  • this keeps the existing initialization behavior while removing duplicate setup/teardown work

Related Issues

Expose stable sdk and framework identity through client metadata so providers and hooks can distinguish web/server usage and framework wrappers without custom vendor shims.

Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
@jonathannorris jonathannorris requested a review from toddbaert April 8, 2026 20:03
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 introduces framework-specific metadata (Angular, Nest, and React) and SDK family identification ('web' or 'server') into OpenFeature clients. This is achieved by wrapping clients in a Proxy that intercepts metadata access. The changes include new utility functions for each framework, updates to the respective SDK providers/services to apply these wrappers, and expanded unit tests to verify metadata propagation. Review feedback highlights potential performance improvements in the Angular SDK, specifically regarding redundant client initialization in the directive and the need for caching proxied client instances in the service to avoid overhead during frequent flag evaluations.

Move the framework metadata proxy into shared client utilities so the React, Angular, and Nest SDKs reuse one implementation.

Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
@jonathannorris
Copy link
Copy Markdown
Member Author

Hum... should sdk actually reuse Paradigm instead of introducing a new union on ClientMetadata?

Right now this PR uses sdk?: 'web' | 'server', but Paradigm is already 'server' | 'client'.

I think it probably makes sense to decide whether this field is meant to represent JS package family (web | server) or runtime paradigm (client | server) before we merge this.

Define a minimal core client interface for metadata so shared framework wrappers can depend on an explicit contract instead of a broad object type.

Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
@MattIPv4
Copy link
Copy Markdown
Member

MattIPv4 commented Apr 8, 2026

Hum... should sdk actually reuse Paradigm instead of introducing a new union on ClientMetadata?

Right now this PR uses sdk?: 'web' | 'server', but Paradigm is already 'server' | 'client'.

I think it probably makes sense to decide whether this field is meant to represent JS package family (web | server) or runtime paradigm (client | server) before we merge this.

Reusing makes sense to me. I think they are essentially representing the same thing? The server package sets its runsOn to 'server', and the web package sets its runsOn to 'client'. Seems silly to introduce a new union to also represent the same thing with different values?

Keep framework metadata intact while distinguishing JS package family from runtime paradigm, and avoid reinitializing Angular directive clients when the domain input already created one.

Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
@jonathannorris
Copy link
Copy Markdown
Member Author

jonathannorris commented Apr 10, 2026

sdk is now JS package identity (js-web / js-server), and paradigm is the runtime identity (client / server).

I think this setup probably makes more sense if we ever expand this metadata shape into something more standard across other OpenFeature SDKs, since package identity and runtime paradigm won't necessarily be the same thing in every implementation.

Align the new directive regression test with the repository's Prettier formatting so the format-lint CI job passes.

Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
@jonathannorris
Copy link
Copy Markdown
Member Author

@MattIPv4 @beeme1mr @toddbaert @lukas-reining

I put together an alternate implementation in #1377 if you want to compare the two approaches directly.

The main difference is that this PR (#1376) uses a Proxy wrapper to inject framework metadata, while #1377 avoids Proxy entirely and instead marks the concrete SDK-owned client instances directly. Both end up exposing the same sdk / paradigm / framework metadata to providers and hooks, but the implementation tradeoff is different:

#1376: cleaner encapsulation, but framework identity comes from a wrapper layer
#1377: no Proxy, no public getClient(...) API change, and metadata comes from the real client instance, but the framework SDKs do need a small internal cast-and-mark helper

Would be good to get feedback on which direction feels cleaner to merge.

@MattIPv4
Copy link
Copy Markdown
Member

👀 Took a look at the alternative, and I think I prefer this approach with a Proxy, as it isolates the metadata change to just where we know for sure the client is being used within that framework -- your other PR mutates the actual client itself, which in the case of at least the React PR, could result in the framework metadata leaking beyond the framework?

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.

2 participants