flagsmith-js-client
flagsmith-js-client copied to clipboard
feat: send client info to Flagsmith
Ref: #2479
- This pull request includes changes to the
flagsmith-core.tsfile to add support forappNameandappVersionin theFlagsmithclass. - Updated the
getJSONmethod to includeX-Customer-Application-NameandX-Customer-Application-Versionheaders ifappNameandappVersionare provided.
Do we want to make the new properties a part of the shared evaluation context interface?
CC @matthewelwell @kyle-ssg
Do we want to make the new properties a part of the shared evaluation context interface?
CC @matthewelwell @kyle-ssg
@khvn26 I'm not sure if these properties should be part of the evaluation context? Would we expect them to change?
@matthewelwell We wouldn't during client runtime, but actually segmenting by app name and app version in evaluation could be desirable for the future?
@matthewelwell We wouldn't during client runtime, but actually segmenting by app name and app version in evaluation could be desirable for the future?
That makes sense, we could use it the future
Ok, sounds good.
@khvn26 @tiagoapolo do you guys have an idea of the changes that are needed then?
I've opened a PR to update the schema here: https://github.com/Flagsmith/flagsmith/pull/5203
After it's merged, we'll be able to regenerate evaluation-context.ts
The code itself looks fine, but I'm curious why we chose this approach instead of setting these attributes as traits. I can speculate as to why:
- We don't have reserved trait names, so we can't guarantee there will not be collisions with existing customer traits. This is still a solvable problem, with some effort
- Customers that track application attributes would always have to use the
/identitiesendpoint (probably with transient traits), since/flagsdoes not accept traitsMy impression is that setting these attributes as traits is the better option; it has its challenges, but I don't see what we would gain by tracking them separate from traits. By using traits, every SDK (including OpenFeature) automatically supports this and we don't have to adapt clients in any way.
Another question of asking this would be - if customers want to target flags based on application name/version/platform/etc, they can do that right now with traits. What would they gain by using special headers instead? I'm sure I'm missing some context to be able to answer this question.
The goal is to track usage and using headers for that case is more appropriate. Also, we don't want to store that information in traits for future evaluation for now, that can come up in a new task/PR.
cc: @matthewelwell
@rolodato I get your point. It would be critical when be used for traits to avoid any discontinuity in case of value change.
(I'm repeating also to confirm my understanding of the objective) In the context of gathering usage and tracking data, as long as they are informational it wouldn't bring any disruption of service to change the name of an application. Because we have the API key on which we could rely to track [project,environment], I see this as a way to group together requests/usage of a sub-application on a given environment, answering the question:
- For my production environment, with 4 different applications using the same key, how is my usage distributed.
So with a name change, it would cause a spike change (from bullet to flagsmith) but the actionability of the underlying data would stay the same imo ("we need to decrease our usage in this one").
I agree with you though, as soon as this is used for traits and evaluation it will need to have a persistent key over time even with name changing etc
In the context of this PR I personally think it's not blocking and it's a base for future finetuning