open-feature / spec

OpenFeature specification
https://openfeature.dev
Apache License 2.0
588 stars 35 forks source link

feat: add domain as an openfeature concept #229

Closed beeme1mr closed 5 months ago

beeme1mr commented 5 months ago

This PR

Related Issues

Fixes #228

Follow-up Tasks

dabeeeenster commented 5 months ago

Would this allow for multiple clients across multiple (or the same) provider?

beeme1mr commented 5 months ago

Would this allow for multiple clients across multiple (or the same) provider?

Yes, exactly. The functionality already exists in many SDKs.

toddbaert commented 5 months ago

Would this allow for multiple clients across multiple (or the same) provider?

Yes, exactly. The functionality already exists in many SDKs.

Ya, it exists but is (IMHO) confusingly named, so I like this change.

federicobond commented 5 months ago

I agree with the motivation, the current wording around named clients is a bit awkward. I don't think the word namespace makes things clear though. A namespace usually denotes a logical grouping of identifiers or symbols.

I propose we survey some examples of APIs solving similar problems to get a better idea of the alternatives.

As a first data point, the Python logging library uses "name" for the parameter to logging.getLogger which returns a logger with a specific configuration defined somewhere else.

beeme1mr commented 5 months ago

@federicobond I thought namespace would be a good fit because it's a logical grouping of clients to a provider. However, I would happily change the proposal if we can find a more appropriate term.

toddbaert commented 5 months ago

I agree with the motivation, the current wording around named clients is a bit awkward. I don't think the word namespace makes things clear though. A namespace usually denotes a logical grouping of identifiers or symbols.

I propose we survey some examples of APIs solving similar problems to get a better idea of the alternatives.

As a first data point, the Python logging library uses "name" for the parameter to logging.getLogger which returns a logger with a specific configuration defined somewhere else.

I don't think we need to go with "namespace" but I don't think "name" is a good candidate. We've had feedback that name makes it sound like a unique identifier for a client/provider, which this is not.

A namespace usually denotes a logical grouping of identifiers or symbols.

@federicobond I would argue that's pretty much what we're going for. A "namespace" in this case could group clients/providers/contexts/events in a single scope ("scope" would also possibly be a decent name) associating them all. "ClientName" already does this, but it's a poor descriptor.

federicobond commented 5 months ago

I think the awkward part in named clients is not the "named" but the "clients" bit, which as you say makes it sound like a unique identifier for a client/provider.

I think the confusion can be resolved by keeping name but rewording the spec to clarify it refers to a scope.

Looking at how OpenTelemetry handles this, it says:

name (required): This name SHOULD uniquely identify the instrumentation scope, such as the instrumentation library (e.g. io.opentelemetry.contrib.mongodb), package, module or class name.

https://opentelemetry.io/docs/specs/otel/trace/api/#get-a-tracer

sheepduke commented 5 months ago

Now in the Rust SDK "named" is used. We have functions like create_client and create_named_client. I agree that it is a bit confusing because it sounds like "create a client that has a name", instead of "create a client that binds to a specific provider".

Personally I think "namespace" maybe too "strong" towards programming side. Maybe something similar to "scope" or "category" feels more business/domain specific. create_scoped_client or create_categorized_client etc maybe?

justinabrahms commented 5 months ago

Instead of thinking about them as "named clients", is it perhaps useful to think of them as "scoped providers"?

sheepduke commented 5 months ago

Instead of thinking about them as "named clients", is it perhaps useful to think of them as "scoped providers"?

I agree. Just wondering if there is a better name than scope. The scope (or namespace etc) are actually defined for providers. The clients are just bound to the corresponding provider.

lukas-reining commented 5 months ago

I think the awkward part in named clients is not the "named" but the "clients" bit, which as you say makes it sound like a unique identifier for a client/provider.

I think the confusion can be resolved by keeping name but rewording the spec to clarify it refers to a scope.

Looking at how OpenTelemetry handles this, it says:

name (required): This name SHOULD uniquely identify the instrumentation scope, such as the instrumentation library (e.g. io.opentelemetry.contrib.mongodb), package, module or class name.

https://opentelemetry.io/docs/specs/otel/trace/api/#get-a-tracer

It feels that a naming for the concept of such a "named provider registration" would be helpful. Semantically we do not want to give a name to a client, so giving a name to a getClient method feels misleading to me.

Personally I think "namespace" maybe too "strong" towards programming side.

This is also what I feel. To me, the semantics of namespace are not exactly what we want to say.

The best thing I can think of is scope, but it does not fully capture this "named provider registration".

federicobond commented 5 months ago

The key concept here is that named clients allow for late binding providers to clients, so it's never the providers you want to name but the scope of the clients. You want the clients to not know anything about the provider that they will end up using.

In particular, this would be a bad practice:

OpenFeature.setProvider("flagd", new FlagDProvider())
...
OpenFeature.getClient("flagd") // do not do this

You rather want:

OpenFeature.setProvider("billingsystem", new FlagDProvider())
...
OpenFeature.getClient("billingsystem")

A word that may better capture the meaning of what we want to do here and lead users away from naming the individual providers is domain. A client then would be initialized with a domain and a provider would be bound to a domain.

federicobond commented 5 months ago

Another important discussion that is out of the scope of this thread but might come up in the future is the possibility of these scopes/domains having hierarchical semantics.

In that were the case, a client with billingsystem.subsystem domain could use the provider bound to billingsystem if no provider were bound to billingsystem.subsystem.

toddbaert commented 5 months ago

The key concept here is that named clients allow for late binding providers to clients, so it's never the providers you want to name but the scope of the clients. You want the clients to not know anything about the provider that they will end up using.

@federicobond This is very well-put!. "Late binding" is exactly what we want to support and communicate. :clinking_glasses:

I don't so much care we do it, but I think we need to communicate this better, both to SDK implementors and API users. Maybe we just need to improve our non-normative text and some inline doc... I would also be open to changing the term to "domain", because I agree with the idea here; this is fundamentally what we wanted to support, particularly for large monolithic applications which might encompass multiple teams and problem "domains".

beeme1mr commented 5 months ago

Thanks for the feedback. I'll bring this up as a topic tomorrow in the community meeting. I think we're close to finding a good term.

maxveldink commented 5 months ago

No new thoughts here than what's been said, but I think the term domain fits well for the use case our company has for this. We have names such as pay_flags, tax_calculation, and legacy_flags, which bind to different provider implementations. Clarifying the spec to call these domains would have helped clarify which names to use.

federicobond commented 5 months ago

As a recap, if I understand correctly, the confusion with named clients arises because the name does not affect the client in any way other than to enable it to late bind to a provider that is different from the one configured globally.

This leads some to think that it's the provider that is given a name, but using provider names defeats the original motivation for introducing this functionality: being able to swap the provider from a central location without having to touch all the client code across different modules and files, code that may even belong to third-party dependencies.

Introducing the word domain as a concept helps clarify the purpose of this value: A domain is a string identifying a scope of flags that are covered by a single provider.

The word domain is used also in the gettext translation system, where it serves a similar purpose: one can bind a message catalog to a specific domain and have multiple domains within a single software system served by different catalogs.

beeme1mr commented 5 months ago

Thanks for the feedback everyone. I've updated the PR to use the term domain. Please take a look when you have a moment.

beeme1mr commented 5 months ago

Thanks for the great reviews and feedback. I'll merge this on Wednesday the 24th unless there's an objection.