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: Update in-process resolver to support flag metadata #1102 #1122

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 11 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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import dev.openfeature.sdk.Value;
import dev.openfeature.sdk.exceptions.ParseError;
import dev.openfeature.sdk.exceptions.TypeMismatchError;
import java.util.Map;
import java.util.function.Consumer;
import java.util.function.Supplier;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -39,8 +40,9 @@ public class InProcessResolver implements Resolver {
private final Consumer<ConnectionEvent> onConnectionEvent;
private final Operator operator;
private final long deadline;
private final ImmutableMetadata metadata;
private final ImmutableMetadata fallBackMetadata;
private final Supplier<Boolean> connectedSupplier;
private final String scope;

/**
* Resolves flag values using
Expand All @@ -62,11 +64,14 @@ public InProcessResolver(
this.onConnectionEvent = onConnectionEvent;
this.operator = new Operator();
this.connectedSupplier = connectedSupplier;
this.metadata = options.getSelector() == null
? null
: ImmutableMetadata.builder()
.addString("scope", options.getSelector())
.build();
if (options.getSelector() == null) {
this.scope = null;
this.fallBackMetadata = null;
} else {
this.scope = options.getSelector();
this.fallBackMetadata =
ImmutableMetadata.builder().addString("scope", this.scope).build();
}
}

/**
Expand Down Expand Up @@ -178,6 +183,7 @@ private <T> ProviderEvaluation<T> resolve(Class<T> type, String key, EvaluationC
return ProviderEvaluation.<T>builder()
.errorMessage("flag: " + key + " not found")
.errorCode(ErrorCode.FLAG_NOT_FOUND)
.flagMetadata(fallBackMetadata)
Copy link
Member

Choose a reason for hiding this comment

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

As a corollary to the inheritance of the flag set metadata, for error cases we should make a "best effort" to return whatever metadata we can - so here we should still return the flag set metadata. We've added this to the spec: https://github.com/open-feature/flagd/pull/1505/files#diff-28ba4b4041b6c39c4b9deb542276312bebdd08f45b39b29d860849d9e8b81a6eR297

The simplest way might be to store the flag set metadata on the flag store and expose it there.

This will greatly help in telemetry and debugging, since we'll have some idea of the flag set, it's version, etc in error reporting and metrics.

.build();
}

Expand All @@ -186,6 +192,7 @@ private <T> ProviderEvaluation<T> resolve(Class<T> type, String key, EvaluationC
return ProviderEvaluation.<T>builder()
.errorMessage("flag: " + key + " is disabled")
.errorCode(ErrorCode.FLAG_NOT_FOUND)
.flagMetadata(getFlagMetadata(flag))
.build();
}

Expand Down Expand Up @@ -232,13 +239,51 @@ private <T> ProviderEvaluation<T> resolve(Class<T> type, String key, EvaluationC
throw new TypeMismatchError(message);
}

final ProviderEvaluation.ProviderEvaluationBuilder<T> evaluationBuilder = ProviderEvaluation.<T>builder()
return ProviderEvaluation.<T>builder()
.value((T) value)
.variant(resolvedVariant)
.reason(reason);
.reason(reason)
.flagMetadata(getFlagMetadata(flag))
.build();
}

private ImmutableMetadata getFlagMetadata(FeatureFlag flag) {
if (flag == null) {
return fallBackMetadata;
}

ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder = ImmutableMetadata.builder();
Copy link
Author

Choose a reason for hiding this comment

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

I would prefer to have a mergeMetdata or addMetadata method in the ImmutableMetadataBuilder

if (scope != null) {
metadataBuilder.addString("scope", scope);
}

for (Map.Entry<String, Object> entry : flag.getMetadata().entrySet()) {
Object value = entry.getValue();
if (value instanceof Number) {
if (value instanceof Long) {
metadataBuilder.addLong(entry.getKey(), (Long) value);
continue;
} else if (value instanceof Double) {
metadataBuilder.addDouble(entry.getKey(), (Double) value);
continue;
} else if (value instanceof Integer) {
metadataBuilder.addInteger(entry.getKey(), (Integer) value);
continue;
} else if (value instanceof Float) {
metadataBuilder.addFloat(entry.getKey(), (Float) value);
continue;
}
} else if (value instanceof Boolean) {
metadataBuilder.addBoolean(entry.getKey(), (Boolean) value);
continue;
} else if (value instanceof String) {
metadataBuilder.addString(entry.getKey(), (String) value);
continue;
}
throw new IllegalArgumentException("The type of the Metadata entry with key " + entry.getKey()
+ " and value " + entry.getValue() + " is not supported");
}

return this.metadata == null
? evaluationBuilder.build()
: evaluationBuilder.flagMetadata(this.metadata).build();
return metadataBuilder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.HashMap;
import java.util.Map;
import lombok.EqualsAndHashCode;
import lombok.Getter;
Expand All @@ -23,18 +24,46 @@ public class FeatureFlag {
private final String defaultVariant;
private final Map<String, Object> variants;
private final String targeting;
private final Map<String, Object> metadata;

/** Construct a flagd feature flag. */
@JsonCreator
public FeatureFlag(
@JsonProperty("state") String state,
@JsonProperty("defaultVariant") String defaultVariant,
@JsonProperty("variants") Map<String, Object> variants,
@JsonProperty("targeting") @JsonDeserialize(using = StringSerializer.class) String targeting) {
@JsonProperty("targeting") @JsonDeserialize(using = StringSerializer.class) String targeting,
@JsonProperty("metadata") Map<String, Object> metadata) {
this.state = state;
this.defaultVariant = defaultVariant;
this.variants = variants;
this.targeting = targeting;
if (metadata == null) {
this.metadata = new HashMap<>();
} else {
this.metadata = metadata;
}
}

/** Construct a flagd feature flag. */
public FeatureFlag(String state, String defaultVariant, Map<String, Object> variants, String targeting) {
this.state = state;
this.defaultVariant = defaultVariant;
this.variants = variants;
this.targeting = targeting;
this.metadata = new HashMap<>();
}

/**
* Add global metadata to this FeatureFlag. Keys that already exist in the metadata of this flag are not
* overwritten.
*
* @param metadata The metadata to add to this flag
*/
public void addMetadata(Map<String, Object> metadata) {
for (Map.Entry<String, Object> entry : metadata.entrySet()) {
this.metadata.putIfAbsent(entry.getKey(), entry.getValue());
}
}

/** Get targeting rule of the flag. */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package dev.openfeature.contrib.providers.flagd.resolver.process.model;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.TreeNode;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.networknt.schema.JsonSchema;
import com.networknt.schema.JsonSchemaFactory;
Expand All @@ -24,6 +26,7 @@
justification = "Feature flag comes as a Json configuration, hence they must be exposed")
public class FlagParser {
private static final String FLAG_KEY = "flags";
private static final String METADATA_KEY = "metadata";
private static final String EVALUATOR_KEY = "$evaluators";
private static final String REPLACER_FORMAT = "\"\\$ref\":(\\s)*\"%s\"";
private static final ObjectMapper MAPPER = new ObjectMapper();
Expand Down Expand Up @@ -73,6 +76,8 @@ public static Map<String, FeatureFlag> parseString(final String configuration, b
try (JsonParser parser = MAPPER.createParser(transposedConfiguration)) {
final TreeNode treeNode = parser.readValueAsTree();
final TreeNode flagNode = treeNode.get(FLAG_KEY);
final TreeNode metadataNode = treeNode.get(METADATA_KEY);
final Map<String, Object> metadata = parseMetadata(metadataNode);

if (flagNode == null) {
throw new IllegalArgumentException("No flag configurations found in the payload");
Expand All @@ -81,13 +86,24 @@ public static Map<String, FeatureFlag> parseString(final String configuration, b
final Iterator<String> it = flagNode.fieldNames();
while (it.hasNext()) {
final String key = it.next();
flagMap.put(key, MAPPER.treeToValue(flagNode.get(key), FeatureFlag.class));
FeatureFlag flag = MAPPER.treeToValue(flagNode.get(key), FeatureFlag.class);
flag.addMetadata(metadata);
flagMap.put(key, flag);
}
}

return flagMap;
}

private static Map<String, Object> parseMetadata(TreeNode metadataNode) throws JsonProcessingException {
if (metadataNode == null) {
return new HashMap<>();
}

TypeReference<Map<String, Object>> typeRef = new TypeReference<Map<String, Object>>() {};
return MAPPER.treeToValue(metadataNode, typeRef);
}

private static String transposeEvaluators(final String configuration) throws IOException {
try (JsonParser parser = MAPPER.createParser(configuration)) {
final Map<String, Pattern> patternMap = new HashMap<>();
Expand Down
Loading
Loading