Server side evaluation #24
Conversation
There was a problem hiding this comment.
instead of using this middleware, we call a new generalized fetcher services.features.fetchFeatures() and let it run in the background
src/services/features/index.ts
Outdated
| if (feature.rules) { | ||
| // track any removely evaluated experiments | ||
| for (const rule of feature.rules) { | ||
| if (rule.tracks) { |
There was a problem hiding this comment.
- Do we need this? Included for compatibility.
- Do we care that track.experiment and track.result will probably not be scrubbed?
src/services/features/index.ts
Outdated
| const result = gb.evalFeature(key); | ||
| if (result.on) { | ||
| // reduced feature definition | ||
| evaluatedFeatures[key] = { defaultValue: true }; |
There was a problem hiding this comment.
Is this the correct minimal format for an evaluated feature flag?
There was a problem hiding this comment.
defaultValue: result.value
There was a problem hiding this comment.
If result.source === "experiment":
{
defaultValue: result.value,
rules: [
{
force: result.value,
tracks: [
{
experiment: result.experiment,
result: result
}
]
}
]
}Otherwise:
{ defaultValue: result.value }There was a problem hiding this comment.
make sure the SDK doesn't actually push the track payload for this case (deferred tracking)
| const result = gb.run(experiment); | ||
| if (result.inExperiment) { | ||
| // reduced experiment definition | ||
| const evaluatedExperiment = { |
There was a problem hiding this comment.
What do we think of this scrubbed format for an evaluated experiment?
src/services/features/index.ts
Outdated
| evaluatedExperiments.push(evaluatedExperiment); | ||
|
|
||
| // track experiment | ||
| trackExperiments.push({ experiment: evaluatedExperiment, result }); |
There was a problem hiding this comment.
- Any issues tracking the evaluated (scrubbed) experiment instead of the original (unscrubbed)? Trying to avoid leaking extra data in the SDK payload.
- Similar question for result: is this too much info?
src/services/features/index.ts
Outdated
| } | ||
| } | ||
|
|
||
| trackExperiments = gb.getTrackedExperiments(); |
There was a problem hiding this comment.
relies on new SDK functionality, exportable _trackedExperimentQueue
There was a problem hiding this comment.
we must deploy the new SDK changes before this works for general users
| if (result.on) { | ||
| // reduced feature definition | ||
| evaluatedFeatures[key] = { defaultValue: true }; | ||
| } |
There was a problem hiding this comment.
if it was an exp, don't want to track, but rather store in track object for later FE eval + tracking
src/services/features/index.ts
Outdated
| ), | ||
| weights: Array.from( | ||
| { length: experiment.variations.length }, | ||
| (_, i) => (i === result.variationId ? 1 : 0) |
There was a problem hiding this comment.
likely don't need this
src/services/features/index.ts
Outdated
| } | ||
| } | ||
|
|
||
| trackExperiments = gb.getTrackedExperiments(); |
There was a problem hiding this comment.
dont' need this one, let the tracking happen on the FE instead
src/services/features/index.ts
Outdated
| ...payload, | ||
| features: evaluatedFeatures, | ||
| experiments: evaluatedExperiments, | ||
| trackExperiments: [ |
There was a problem hiding this comment.
- don't need this field during ssEval (proxy eval)
- but we will need this for BE->FE deferred tracking
|
See: #33 |
open new eval endpoint for server side evaluation on the proxy
trackExperimentsin the sdk payloadalters the caching behavior slightly within the proxy
alters the SSE (streaming) behavior
{update: true}instead of revealing the entire payload.