Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: report prerequisite relations in AllFlagState #19

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkgs/sdk/server/contract-tests/TestService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public class Webapp
"tags",
"inline-context",
"anonymous-redaction",
"evaluation-hooks"
"evaluation-hooks",
"client-prereq-events"
};

public readonly Handler Handler;
Expand Down
85 changes: 66 additions & 19 deletions pkgs/sdk/server/src/FeatureFlagsState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ public FeatureFlagsState Build()
/// </summary>
/// <param name="valid">true if valid, false if invalid (default is valid)</param>
/// <returns>the same builder</returns>
[Obsolete("Unused, construct a FeatureFlagState with valid/invalid state directly")]
public FeatureFlagsStateBuilder Valid(bool valid)
{
_valid = valid;
Expand All @@ -169,6 +170,19 @@ public FeatureFlagsStateBuilder Valid(bool valid)
/// <param name="result">the evaluation result</param>
/// <returns></returns>
public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail<LdValue> result)
{
return AddFlag(flagKey, result, new List<string>());
}


/// <summary>
/// Adds the result of a flag evaluation, including direct prerequisites.
/// </summary>
/// <param name="flagKey">the flag key</param>
/// <param name="result">the evaluation result</param>
/// <param name="prerequisites">the direct prerequisites evaluated for this flag</param>
/// <returns></returns>
public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail<LdValue> result, List<string> prerequisites)
{
return AddFlag(flagKey,
result.Value,
Expand All @@ -177,13 +191,14 @@ public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail<LdValue
0,
false,
false,
null);
null,
prerequisites);
}

// This method is defined with internal scope because metadata fields like trackEvents aren't
// relevant to the main external use case for the builder (testing server-side code)
internal FeatureFlagsStateBuilder AddFlag(string flagKey, LdValue value, int? variationIndex, EvaluationReason reason,
int flagVersion, bool flagTrackEvents, bool trackReason, UnixMillisecondTime? flagDebugEventsUntilDate)
int flagVersion, bool flagTrackEvents, bool trackReason, UnixMillisecondTime? flagDebugEventsUntilDate, List<string> prerequisites)
{
bool flagIsTracked = flagTrackEvents || flagDebugEventsUntilDate != null;
var flag = new FlagState
Expand All @@ -194,14 +209,15 @@ internal FeatureFlagsStateBuilder AddFlag(string flagKey, LdValue value, int? va
Reason = trackReason || (_withReasons && (!_detailsOnlyIfTracked || flagIsTracked)) ? reason : (EvaluationReason?)null,
DebugEventsUntilDate = flagDebugEventsUntilDate,
TrackEvents = flagTrackEvents,
TrackReason = trackReason
TrackReason = trackReason,
Prerequisites = prerequisites
};
_flags[flagKey] = flag;
return this;
}
}

internal struct FlagState
internal struct FlagState : IEquatable<FlagState>
{
internal LdValue Value { get; set; }
internal int? Variation { get; set; }
Expand All @@ -211,24 +227,37 @@ internal struct FlagState
internal UnixMillisecondTime? DebugEventsUntilDate { get; set; }
internal EvaluationReason? Reason { get; set; }

public override bool Equals(object other)
internal List<string> Prerequisites { get; set; }

public bool Equals(FlagState o)
{
if (other is FlagState o)
{
return Variation == o.Variation &&
Version == o.Version &&
TrackEvents == o.TrackEvents &&
TrackReason == o.TrackReason &&
DebugEventsUntilDate.Equals(o.DebugEventsUntilDate) &&
Object.Equals(Reason, o.Reason);
}
return false;
return Variation == o.Variation &&
Version == o.Version &&
TrackEvents == o.TrackEvents &&
TrackReason == o.TrackReason &&
DebugEventsUntilDate.Equals(o.DebugEventsUntilDate) &&
Object.Equals(Reason, o.Reason) &&
Prerequisites.SequenceEqual(o.Prerequisites);
}
public override bool Equals(object obj)
{
return obj is FlagState other && Equals(other);
}

public static bool operator ==(FlagState lhs, FlagState rhs)
{
return lhs.Equals(rhs);
}

public static bool operator !=(FlagState lhs, FlagState rhs)
{
return !(lhs == rhs);
}

public override int GetHashCode()
{
return new HashCodeBuilder().With(Variation).With(Version).With(TrackEvents).With(TrackReason).
With(DebugEventsUntilDate).With(Reason).Value;
return new HashCodeBuilder().With(Variation).With(Version).With(TrackEvents).With(TrackReason)
.With(DebugEventsUntilDate).With(Reason).With(Prerequisites).Value;
}
}

Expand Down Expand Up @@ -271,6 +300,14 @@ public override void Write(Utf8JsonWriter w, FeatureFlagsState state, JsonSerial
w.WritePropertyName("reason");
EvaluationReasonConverter.WriteJsonValue(meta.Reason.Value, w);
}
if (meta.Prerequisites.Count > 0) {
w.WriteStartArray("prerequisites");
foreach (var p in meta.Prerequisites)
{
w.WriteStringValue(p);
}
w.WriteEndArray();
}
w.WriteEndObject();
}
w.WriteEndObject();
Expand All @@ -295,7 +332,10 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon
for (var flagsObj = RequireObject(ref reader); flagsObj.Next(ref reader);)
{
var subKey = flagsObj.Name;
var flag = flags.ContainsKey(subKey) ? flags[subKey] : new FlagState();
Copy link
Contributor Author

@cwaldren-ld cwaldren-ld Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly awkward: have to new the prerequisite list here in the deserializer, and also on line 375. I'd rather just have this in the constructor so it's never a question, but looks like structs in our language version can't define constructors.

Another option is to make a null list be a possibility. Logically that'd be fine, but.. I'd rather the consumers never need to question if it's null or not. I could be convinced otherwise.

var flag = flags.ContainsKey(subKey)
? flags[subKey]
: new FlagState() { Prerequisites = new List<string>() };

for (var metaObj = RequireObject(ref reader); metaObj.Next(ref reader);)
{
switch (metaObj.Name)
Expand All @@ -318,14 +358,21 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon
flag.Reason = reader.TokenType == JsonTokenType.Null ? (EvaluationReason?)null :
EvaluationReasonConverter.ReadJsonValue(ref reader);
break;
case "prerequisites":
flag.Prerequisites = new List<string>();
for (var prereqs = RequireArray(ref reader); prereqs.Next(ref reader);)
{
flag.Prerequisites.Add(reader.GetString());
}
break;
}
}
flags[subKey] = flag;
}
break;

default:
var flagForValue = flags.ContainsKey(key) ? flags[key] : new FlagState();
var flagForValue = flags.ContainsKey(key) ? flags[key] : new FlagState(){Prerequisites = new List<string>()};
flagForValue.Value = LdValueConverter.ReadJsonValue(ref reader);
flags[key] = flagForValue;
break;
Expand Down
6 changes: 3 additions & 3 deletions pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ internal EvalResult(EvaluationDetail<LdValue> result, IList<PrerequisiteEvalReco
internal struct PrerequisiteEvalRecord
{
internal readonly FeatureFlag PrerequisiteFlag;
internal readonly string PrerequisiteOfFlagKey;
internal readonly string FlagKey;
internal readonly EvaluationDetail<LdValue> Result;

internal PrerequisiteEvalRecord(FeatureFlag prerequisiteFlag, string prerequisiteOfFlagKey,
internal PrerequisiteEvalRecord(FeatureFlag prerequisiteFlag, string flagKey,
EvaluationDetail<LdValue> result)
{
PrerequisiteFlag = prerequisiteFlag;
PrerequisiteOfFlagKey = prerequisiteOfFlagKey;
FlagKey = flagKey;
Result = result;
}
}
Expand Down
17 changes: 11 additions & 6 deletions pkgs/sdk/server/src/LdClient.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Security.Cryptography;
using LaunchDarkly.Logging;
Expand Down Expand Up @@ -371,8 +372,7 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[

var builder = new FeatureFlagsStateBuilder(options);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These weren't used at all in this function. They are used internal to the FeatureFlagsStateBuilder, presumably they didn't get cleaned up when that was introduced.

var clientSideOnly = FlagsStateOption.HasOption(options, FlagsStateOption.ClientSideOnly);
var withReasons = FlagsStateOption.HasOption(options, FlagsStateOption.WithReasons);
var detailsOnlyIfTracked = FlagsStateOption.HasOption(options, FlagsStateOption.DetailsOnlyForTrackedFlags);

KeyedItems<ItemDescriptor> flags;
try
{
Expand All @@ -397,6 +397,11 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[
{
EvaluatorTypes.EvalResult result = _evaluator.Evaluate(flag, context);
bool inExperiment = EventFactory.IsExperiment(flag, result.Result.Reason);

var directPrerequisites = result.PrerequisiteEvals.Where(
e => e.FlagKey == flag.Key)
.Select(p => p.PrerequisiteFlag.Key).ToList();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting to a list here, rather than leaving as an IEnumerable, because the intent is that order is preserved.


builder.AddFlag(
flag.Key,
result.Result.Value,
Expand All @@ -405,16 +410,16 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[
flag.Version,
flag.TrackEvents || inExperiment,
inExperiment,
flag.DebugEventsUntilDate
);
flag.DebugEventsUntilDate,
directPrerequisites);
}
catch (Exception e)
{
LogHelpers.LogException(_evalLog,
string.Format("Exception caught for feature flag \"{0}\" when evaluating all flags", flag.Key),
e);
EvaluationReason reason = EvaluationReason.ErrorReason(EvaluationErrorKind.Exception);
builder.AddFlag(flag.Key, new EvaluationDetail<LdValue>(LdValue.Null, null, reason));
builder.AddFlag(flag.Key, new EvaluationDetail<LdValue>(LdValue.Null, null, reason), new List<string>());
}
}
return builder.Build();
Expand Down Expand Up @@ -477,7 +482,7 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[
foreach (var prereqEvent in evalResult.PrerequisiteEvals)
{
_eventProcessor.RecordEvaluationEvent(eventFactory.NewPrerequisiteEvaluationEvent(
prereqEvent.PrerequisiteFlag, context, prereqEvent.Result, prereqEvent.PrerequisiteOfFlagKey));
prereqEvent.PrerequisiteFlag, context, prereqEvent.Result, prereqEvent.FlagKey));
}
}
var evalDetail = evalResult.Result;
Expand Down
61 changes: 55 additions & 6 deletions pkgs/sdk/server/test/FeatureFlagsStateTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,13 @@ public void CanConvertToValuesMap()
public void CanSerializeToJson()
{
var state = FeatureFlagsState.Builder(FlagsStateOption.WithReasons)
.AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null)
.AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, UnixMillisecondTime.OfMillis(1000))
.AddFlag("key3", LdValue.Null, null, EvaluationReason.ErrorReason(EvaluationErrorKind.MalformedFlag), 300, false, false, null)
.AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null,
new List<string>())
.AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false,
UnixMillisecondTime.OfMillis(1000), new List<string>())
.AddFlag("key3", LdValue.Null, null, EvaluationReason.ErrorReason(EvaluationErrorKind.MalformedFlag),
300, false, false, null, new List<string>()
)
.Build();

var expectedString = @"{""key1"":""value1"",""key2"":""value2"",""key3"":null,
Expand All @@ -92,18 +96,63 @@ public void CanSerializeToJson()
JsonAssertions.AssertJsonEqual(expectedString, actualString);
}

[Fact]
public void CanSerializeFlagPrerequisites()
{
var state = FeatureFlagsState.Builder(FlagsStateOption.WithReasons)
.AddFlag("prereq1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null,
new List<string>())
.AddFlag("prereq2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false,
UnixMillisecondTime.OfMillis(1000), new List<string>())
.AddFlag("toplevel", LdValue.Null, null,
EvaluationReason.ErrorReason(EvaluationErrorKind.MalformedFlag), 300, false, false, null,
new List<string>
{
"prereq1", "prereq2"
})
.Build();


var expectedString = @"{""prereq1"":""value1"",""prereq2"":""value2"",""toplevel"":null,
""$flagsState"":{
""prereq1"":{
""variation"":0,""version"":100,""reason"":{""kind"":""OFF""}
},""prereq2"":{
""variation"":1,""version"":200,""reason"":{""kind"":""FALLTHROUGH""},""trackEvents"":true,""debugEventsUntilDate"":1000
},""toplevel"":{
""version"":300,""reason"":{""kind"":""ERROR"",""errorKind"":""MALFORMED_FLAG""},""prerequisites"":[""prereq1"",""prereq2""]
}
},
""$valid"":true
}";
var actualString = LdJsonSerialization.SerializeObject(state);
JsonAssertions.AssertJsonEqual(expectedString, actualString);
}


[Fact]
public void CanDeserializeFromJson()
{
var state = FeatureFlagsState.Builder(FlagsStateOption.WithReasons)
.AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null)
.AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, UnixMillisecondTime.OfMillis(1000))
.AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null,
new List<string>())
.AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false,
UnixMillisecondTime.OfMillis(1000), new List<string> { "key1" })
.Build();

var jsonString = LdJsonSerialization.SerializeObject(state);
var state1 = LdJsonSerialization.DeserializeObject<FeatureFlagsState>(jsonString);

var jsonString2 = LdJsonSerialization.SerializeObject(state1);

// Ensure a roundtrip state -> json -> json is equal.
Assert.Equal(jsonString, jsonString2);

// Ensure a roundtrip state -> json -> state is equal.
Assert.Equal(state, state1);
}
}
}



}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void FlagReturnsOffVariationAndEventIfPrerequisiteIsOff()
Assert.Equal(f1.Key, e.PrerequisiteFlag.Key);
Assert.Equal(LdValue.Of("go"), e.Result.Value);
Assert.Equal(f1.Version, e.PrerequisiteFlag.Version);
Assert.Equal(f0.Key, e.PrerequisiteOfFlagKey);
Assert.Equal(f0.Key, e.FlagKey);
});
}

Expand Down Expand Up @@ -99,7 +99,7 @@ public void FlagReturnsOffVariationAndEventIfPrerequisiteIsNotMet()
Assert.Equal(f1.Key, e.PrerequisiteFlag.Key);
Assert.Equal(LdValue.Of("nogo"), e.Result.Value);
Assert.Equal(f1.Version, e.PrerequisiteFlag.Version);
Assert.Equal(f0.Key, e.PrerequisiteOfFlagKey);
Assert.Equal(f0.Key, e.FlagKey);
});
}

Expand Down Expand Up @@ -133,7 +133,7 @@ public void FlagReturnsFallthroughVariationAndEventIfPrerequisiteIsMetAndThereAr
Assert.Equal(f1.Key, e.PrerequisiteFlag.Key);
Assert.Equal(LdValue.Of("go"), e.Result.Value);
Assert.Equal(f1.Version, e.PrerequisiteFlag.Version);
Assert.Equal(f0.Key, e.PrerequisiteOfFlagKey);
Assert.Equal(f0.Key, e.FlagKey);
});
}

Expand Down Expand Up @@ -176,14 +176,14 @@ public void MultipleLevelsOfPrerequisitesProduceMultipleEvents()
Assert.Equal(f2.Key, e.PrerequisiteFlag.Key);
Assert.Equal(LdValue.Of("go"), e.Result.Value);
Assert.Equal(f2.Version, e.PrerequisiteFlag.Version);
Assert.Equal(f1.Key, e.PrerequisiteOfFlagKey);
Assert.Equal(f1.Key, e.FlagKey);
},
e =>
{
Assert.Equal(f1.Key, e.PrerequisiteFlag.Key);
Assert.Equal(LdValue.Of("go"), e.Result.Value);
Assert.Equal(f1.Version, e.PrerequisiteFlag.Version);
Assert.Equal(f0.Key, e.PrerequisiteOfFlagKey);
Assert.Equal(f0.Key, e.FlagKey);
});
}

Expand Down
5 changes: 5 additions & 0 deletions pkgs/sdk/server/test/Internal/Model/FeatureFlagBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ internal FeatureFlagBuilder OffWithValue(LdValue value)
return On(false).OffVariation(0).Variations(value);
}

internal FeatureFlagBuilder OnWithValue(LdValue value)
{
return On(true).OffVariation(0).FallthroughVariation(0).Variations(value);
}

internal FeatureFlagBuilder BooleanWithClauses(params Clause[] clauses)
{
return On(true).OffVariation(0)
Expand Down
Loading
Loading