App Configuration Spring Feature Management Usage (Azure#33854)

* Adding FeatureFlag Tracing * Update test-resources.json * Adding Container App info * Fixing refresh bug with multiple feature flag filters in a single store. Fixed bug were invalid feature flags would cause refresh loop. * Removed todo, has been fixed. * Update sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/config/implementation/http/policy/TracingInfo.java Co-authored-by: Xiaolu Dai <[email protected]> * Update sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/config/implementation/http/policy/TracingInfo.java Co-authored-by: Xiaolu Dai <[email protected]> * Update sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/config/implementation/http/policy/TracingInfo.java Co-authored-by: Xiaolu Dai <[email protected]> * Update sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/config/implementation/AppConfigurationReplicaClient.java Co-authored-by: Xiaolu Dai <[email protected]> * Code Review comments * Update from comments * Fixing watchKeySize * Updating Tracing usage * Adding back connectionStrings --------- Co-authored-by: Xiaolu Dai <[email protected]>
conniey · Mar 14, 2023 · 388b720 · 388b720
1 parent 5a5226f
commit 388b720
Show file tree

Hide file tree

Showing 18 changed files with 603 additions and 174 deletions.
diff --git a/...src/main/java/com/azure/spring/cloud/config/implementation/AppConfigurationConstants.java b/...src/main/java/com/azure/spring/cloud/config/implementation/AppConfigurationConstants.java
@@ -27,20 +27,15 @@ public class AppConfigurationConstants {
      */
     public static final String FEATURE_FLAG_PREFIX = ".appconfig.featureflag/";
 
-    /**
-     * Feature Store Prefix
-     */
-    public static final String FEATURE_STORE_SUFFIX = ".appconfig";
-
     /**
      * Separator for multiple labels.
      */
     public static final String LABEL_SEPARATOR = ",";
 
     /**
-     * Key for returning all feature flags
+     * The key filter for selecting all feature flags.
      */
-    public static final String FEATURE_STORE_WATCH_KEY = FEATURE_STORE_SUFFIX + "*";
+    public static final String SELECT_ALL_FEATURE_FLAGS = FEATURE_FLAG_PREFIX + "*";
 
     /**
      * Constant for tracing if the library is being used with a dev profile.

diff --git a/...e/spring/cloud/config/implementation/AppConfigurationFeatureManagementPropertySource.java b/...e/spring/cloud/config/implementation/AppConfigurationFeatureManagementPropertySource.java
@@ -8,6 +8,7 @@
 import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.FEATURE_FLAG_CONTENT_TYPE;
 import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.FEATURE_FLAG_PREFIX;
 import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.FEATURE_MANAGEMENT_KEY;
+import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.SELECT_ALL_FEATURE_FLAGS;
 import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.GROUPS;
 import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.GROUPS_CAPS;
 import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.TARGETING_FILTER;
@@ -30,6 +31,7 @@
 import com.azure.data.appconfiguration.models.FeatureFlagFilter;
 import com.azure.data.appconfiguration.models.SettingSelector;
 import com.azure.spring.cloud.config.implementation.feature.management.entity.Feature;
+import com.azure.spring.cloud.config.implementation.http.policy.TracingInfo;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.MapperFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -47,13 +49,6 @@ final class AppConfigurationFeatureManagementPropertySource extends AppConfigura
     private static final ObjectMapper CASE_INSENSITIVE_MAPPER = JsonMapper.builder()
         .configure(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES, true).build();
 
-    /**
-     * App Configuration Feature Filter prefix.
-     */
-    private static final String KEY_FILTER_PREFIX = ".appconfig.featureflag/";
-
-    private static final String KEY_FILTER_DEFAULT = KEY_FILTER_PREFIX + "*";
-
     private final List<ConfigurationSetting> featureConfigurationSettings;
 
     AppConfigurationFeatureManagementPropertySource(String originEndpoint, AppConfigurationReplicaClient replicaClient,
@@ -84,10 +79,10 @@ private static List<Object> convertToListOrEmptyList(Map<String, Object> paramet
     public void initProperties() {
         SettingSelector settingSelector = new SettingSelector();
 
-        String keyFilter = KEY_FILTER_DEFAULT;
+        String keyFilter = SELECT_ALL_FEATURE_FLAGS;
 
         if (StringUtils.hasText(this.keyFilter)) {
-            keyFilter = KEY_FILTER_PREFIX + this.keyFilter;
+            keyFilter = FEATURE_FLAG_PREFIX + this.keyFilter;
         }
 
         settingSelector.setKeyFilter(keyFilter);
@@ -99,18 +94,21 @@ public void initProperties() {
             settingSelector.setLabelFilter(label);
 
             List<ConfigurationSetting> features = replicaClient.listSettings(settingSelector);
+            TracingInfo tracing = replicaClient.getTracingInfo();
 
             // Reading In Features
             for (ConfigurationSetting setting : features) {
                 if (setting instanceof FeatureFlagConfigurationSetting
                     && FEATURE_FLAG_CONTENT_TYPE.equals(setting.getContentType())) {
                     featureConfigurationSettings.add(setting);
-                    Object feature = createFeature((FeatureFlagConfigurationSetting) setting);
+                    FeatureFlagConfigurationSetting featureFlag = (FeatureFlagConfigurationSetting) setting;
 
-                    String configName = FEATURE_MANAGEMENT_KEY      
+                    String configName = FEATURE_MANAGEMENT_KEY
                         + setting.getKey().trim().substring(FEATURE_FLAG_PREFIX.length());
 
-                    properties.put(configName, feature);
+                    updateTelemetry(featureFlag, tracing);
+
+                    properties.put(configName, createFeature(featureFlag));
                 }
             }
         }
@@ -172,6 +170,18 @@ private Object createFeature(FeatureFlagConfigurationSetting item) {
         return feature;
 
     }
+
+    /**
+     * Looks at each filter used in a Feature Flag to check what types it is using.
+     * 
+     * @param featureFlag FeatureFlagConfigurationSetting
+     * @param tracing The TracingInfo for this store.
+     */
+    private void updateTelemetry(FeatureFlagConfigurationSetting featureFlag, TracingInfo tracing) {
+        for (FeatureFlagFilter filter : featureFlag.getClientFilters()) {
+            tracing.getFeatureFlagTracing().updateFeatureFilterTelemetry(filter.getName());
+        }
+    }
 
     private String getFeatureSimpleName(ConfigurationSetting setting) {
         return setting.getKey().trim().substring(FEATURE_FLAG_PREFIX.length());

diff --git a/...a/com/azure/spring/cloud/config/implementation/AppConfigurationPropertySourceLocator.java b/...a/com/azure/spring/cloud/config/implementation/AppConfigurationPropertySourceLocator.java
@@ -61,7 +61,7 @@ public final class AppConfigurationPropertySourceLocator implements PropertySour
      */
     public AppConfigurationPropertySourceLocator(AppConfigurationProviderProperties appProperties,
         AppConfigurationReplicaClientFactory clientFactory, AppConfigurationKeyVaultClientFactory keyVaultClientFactory,
-        Duration refreshInterval,  List<ConfigStore> configStores) {
+        Duration refreshInterval, List<ConfigStore> configStores) {
         this.refreshInterval = refreshInterval;
         this.appProperties = appProperties;
         this.configStores = configStores;
@@ -210,12 +210,10 @@ private List<ConfigurationSetting> getWatchKeys(AppConfigurationReplicaClient cl
     private List<ConfigurationSetting> getFeatureFlagWatchKeys(ConfigStore configStore,
         List<AppConfigurationPropertySource> sources) {
         List<ConfigurationSetting> watchKeysFeatures = new ArrayList<>();
-
         if (configStore.getFeatureFlags().getEnabled()) {
             for (AppConfigurationPropertySource propertySource : sources) {
                 if (propertySource instanceof AppConfigurationFeatureManagementPropertySource) {
-                    watchKeysFeatures = ((AppConfigurationFeatureManagementPropertySource) propertySource)
-                        .getFeatureFlagSettings();
+                    watchKeysFeatures.addAll(((AppConfigurationFeatureManagementPropertySource) propertySource).getFeatureFlagSettings());
                 }
             }
         }
@@ -261,14 +259,6 @@ private List<AppConfigurationPropertySource> create(AppConfigurationReplicaClien
         List<AppConfigurationPropertySource> sourceList = new ArrayList<>();
         List<AppConfigurationKeyValueSelector> selects = store.getSelects();
 
-        for (AppConfigurationKeyValueSelector selectedKeys : selects) {
-            AppConfigurationApplicationSettingPropertySource propertySource = new AppConfigurationApplicationSettingPropertySource(
-                store.getEndpoint(), client, keyVaultClientFactory, selectedKeys.getKeyFilter(),
-                selectedKeys.getLabelFilter(profiles), appProperties.getMaxRetryTime());
-            propertySource.initProperties();
-            sourceList.add(propertySource);
-        }
-
         if (store.getFeatureFlags().getEnabled()) {
             for (FeatureFlagKeyValueSelector selectedKeys : store.getFeatureFlags().getSelects()) {
                 AppConfigurationFeatureManagementPropertySource propertySource = new AppConfigurationFeatureManagementPropertySource(
@@ -280,6 +270,14 @@ private List<AppConfigurationPropertySource> create(AppConfigurationReplicaClien
             }
         }
 
+        for (AppConfigurationKeyValueSelector selectedKeys : selects) {
+            AppConfigurationApplicationSettingPropertySource propertySource = new AppConfigurationApplicationSettingPropertySource(
+                store.getEndpoint(), client, keyVaultClientFactory, selectedKeys.getKeyFilter(),
+                selectedKeys.getLabelFilter(profiles), appProperties.getMaxRetryTime());
+            propertySource.initProperties();
+            sourceList.add(propertySource);
+        }
+
         return sourceList;
     }
 

diff --git a/...c/main/java/com/azure/spring/cloud/config/implementation/AppConfigurationRefreshUtil.java b/...c/main/java/com/azure/spring/cloud/config/implementation/AppConfigurationRefreshUtil.java
@@ -12,14 +12,16 @@
 import org.springframework.util.StringUtils;
 
 import com.azure.data.appconfiguration.models.ConfigurationSetting;
+import com.azure.data.appconfiguration.models.FeatureFlagConfigurationSetting;
 import com.azure.data.appconfiguration.models.SettingSelector;
 import com.azure.spring.cloud.config.implementation.http.policy.BaseAppConfigurationPolicy;
 import com.azure.spring.cloud.config.implementation.properties.AppConfigurationStoreMonitoring;
 import com.azure.spring.cloud.config.implementation.properties.FeatureFlagKeyValueSelector;
 import com.azure.spring.cloud.config.implementation.properties.FeatureFlagStore;
 
+import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.FEATURE_FLAG_CONTENT_TYPE;
 import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.FEATURE_FLAG_PREFIX;
-import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.FEATURE_STORE_WATCH_KEY;
+import static com.azure.spring.cloud.config.implementation.AppConfigurationConstants.SELECT_ALL_FEATURE_FLAGS;
 
 class AppConfigurationRefreshUtil {
 
@@ -171,8 +173,7 @@ private static void refreshWithTime(AppConfigurationReplicaClient client, State
 
             if (eventData.getDoRefresh()) {
                 // Just need to reset refreshInterval, if a refresh was triggered it will be updated after loading the
-                // new
-                // configurations.
+                // new configurations.
                 StateHolder.getCurrentState().updateStateRefresh(state, refreshInterval);
             }
         }
@@ -207,8 +208,10 @@ private static void refreshWithTimeFeatureFlags(AppConfigurationReplicaClient cl
         Instant date = Instant.now();
         if (date.isAfter(state.getNextRefreshCheck())) {
 
+            int watchedKeySize = 0;
+
             for (FeatureFlagKeyValueSelector watchKey : featureStore.getSelects()) {
-                String keyFilter = FEATURE_STORE_WATCH_KEY;
+                String keyFilter = SELECT_ALL_FEATURE_FLAGS;
 
                 if (StringUtils.hasText(watchKey.getKeyFilter())) {
                     keyFilter = FEATURE_FLAG_PREFIX + watchKey.getKeyFilter();
@@ -218,42 +221,46 @@ private static void refreshWithTimeFeatureFlags(AppConfigurationReplicaClient cl
                     .setLabelFilter(watchKey.getLabelFilterText(profiles));
                 List<ConfigurationSetting> currentKeys = client.listSettings(selector);
 
-                int watchedKeySize = 0;
-
-                keyCheck: for (ConfigurationSetting currentKey : currentKeys) {
+                watchedKeySize = checkFeatureFlags(currentKeys, state, client, eventData);
+            }
 
-                    watchedKeySize += 1;
-                    for (ConfigurationSetting watchFlag : state.getWatchKeys()) {
+            if (!eventData.getDoRefresh() && watchedKeySize != state.getWatchKeys().size()) {
+                String eventDataInfo = ".appconfig.featureflag/*";
 
-                        // If there is no result, etag will be considered empty.
-                        // A refresh will trigger once the selector returns a value.
-                        if (watchFlag != null && watchFlag.getKey().equals(currentKey.getKey())
-                            && watchFlag.getLabel().equals(currentKey.getLabel())) {
-                            checkETag(watchFlag, currentKey, client.getEndpoint(), eventData);
-                            if (eventData.getDoRefresh()) {
-                                break keyCheck;
+                // Only one refresh Event needs to be call to update all of the
+                // stores, not one for each.
+                LOGGER.info("Configuration Refresh Event triggered by " + eventDataInfo);
 
-                            }
-                        }
+                eventData.setMessage(eventDataInfo);
+            }
 
-                    }
-                }
+            // Just need to reset refreshInterval, if a refresh was triggered it will be updated after loading the new
+            // configurations.
+            StateHolder.getCurrentState().updateStateRefresh(state, refreshInterval);
+        }
+    }
 
-                if (watchedKeySize != state.getWatchKeys().size()) {
-                    String eventDataInfo = ".appconfig.featureflag/*";
+    private static int checkFeatureFlags(List<ConfigurationSetting> currentKeys, State state,
+        AppConfigurationReplicaClient client, RefreshEventData eventData) {
+        int watchedKeySize = 0;
+        for (ConfigurationSetting currentKey : currentKeys) {
+            if (currentKey instanceof FeatureFlagConfigurationSetting
+                && FEATURE_FLAG_CONTENT_TYPE.equals(currentKey.getContentType())) {
 
-                    // Only one refresh Event needs to be call to update all of the
-                    // stores, not one for each.
-                    LOGGER.info("Configuration Refresh Event triggered by " + eventDataInfo);
+                watchedKeySize += 1;
+                for (ConfigurationSetting watchFlag : state.getWatchKeys()) {
 
-                    eventData.setMessage(eventDataInfo);
+                    // If there is no result, etag will be considered empty.
+                    // A refresh will trigger once the selector returns a value.
+                    if (compairKeys(watchFlag, currentKey, client.getEndpoint(), eventData)) {
+                        if (eventData.getDoRefresh()) {
+                            return watchedKeySize;
+                        }
+                    }
                 }
             }
-
-            // Just need to reset refreshInterval, if a refresh was triggered it will be updated after loading the new
-            // configurations.
-            StateHolder.getCurrentState().updateStateRefresh(state, refreshInterval);
         }
+        return watchedKeySize;
     }
 
     private static void refreshWithoutTimeFeatureFlags(AppConfigurationReplicaClient client,
@@ -272,9 +279,7 @@ private static void refreshWithoutTimeFeatureFlags(AppConfigurationReplicaClient
 
                     // If there is no result, etag will be considered empty.
                     // A refresh will trigger once the selector returns a value.
-                    if (watchFlag != null && watchFlag.getKey().equals(currentTriggerConfiguration.getKey())
-                        && watchFlag.getLabel().equals(currentTriggerConfiguration.getLabel())) {
-                        checkETag(watchFlag, currentTriggerConfiguration, client.getEndpoint(), eventData);
+                    if (compairKeys(watchFlag, currentTriggerConfiguration, client.getEndpoint(), eventData)) {
                         if (eventData.getDoRefresh()) {
                             return;
                         }
@@ -297,6 +302,16 @@ private static void refreshWithoutTimeFeatureFlags(AppConfigurationReplicaClient
         }
     }
 
+    private static Boolean compairKeys(ConfigurationSetting key1, ConfigurationSetting key2,
+        String endpoint, RefreshEventData eventData) {
+        if (key1 != null && key1.getKey().equals(key2.getKey()) && key1.getLabel().equals(key2.getLabel())) {
+            checkETag(key1, key2, endpoint, eventData);
+            return true;
+        }
+        return false;
+
+    }
+
     private static void checkETag(ConfigurationSetting watchSetting, ConfigurationSetting currentTriggerConfiguration,
         String endpoint, RefreshEventData eventData) {
         if (currentTriggerConfiguration == null) {

diff --git a/...main/java/com/azure/spring/cloud/config/implementation/AppConfigurationReplicaClient.java b/...main/java/com/azure/spring/cloud/config/implementation/AppConfigurationReplicaClient.java
@@ -13,6 +13,7 @@
 import com.azure.data.appconfiguration.ConfigurationClient;
 import com.azure.data.appconfiguration.models.ConfigurationSetting;
 import com.azure.data.appconfiguration.models.SettingSelector;
+import com.azure.spring.cloud.config.implementation.http.policy.TracingInfo;
 
 /**
  * Client for connecting to App Configuration when multiple replicas are in use.
@@ -27,16 +28,19 @@ class AppConfigurationReplicaClient {
 
     private int failedAttempts;
 
+    private final TracingInfo tracingInfo;
+
     /**
      * Holds Configuration Client and info needed to manage backoff.
      * @param endpoint client endpoint
      * @param client Configuration Client to App Configuration store
      */
-    AppConfigurationReplicaClient(String endpoint, ConfigurationClient client) {
+    AppConfigurationReplicaClient(String endpoint, ConfigurationClient client, TracingInfo tracingInfo) {
         this.endpoint = endpoint;
         this.client = client;
         this.backoffEndTime = Instant.now().minusMillis(1);
         this.failedAttempts = 0;
+        this.tracingInfo = tracingInfo;
     }
 
     /**
@@ -148,4 +152,8 @@ void updateSyncToken(String syncToken) {
         }
     }
 
+    TracingInfo getTracingInfo() {
+        return tracingInfo;
+    }
+
 }
diff --git a/...a/com/azure/spring/cloud/config/implementation/AppConfigurationReplicaClientsBuilder.java b/...a/com/azure/spring/cloud/config/implementation/AppConfigurationReplicaClientsBuilder.java
@@ -9,6 +9,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.boot.convert.DurationStyle;
@@ -20,12 +21,14 @@
 import com.azure.core.http.policy.ExponentialBackoff;
 import com.azure.core.http.policy.RetryPolicy;
 import com.azure.core.http.policy.RetryStrategy;
+import com.azure.core.util.Configuration;
 import com.azure.data.appconfiguration.ConfigurationClientBuilder;
 import com.azure.identity.ManagedIdentityCredentialBuilder;
 import com.azure.spring.cloud.autoconfigure.context.AzureGlobalProperties;
 import com.azure.spring.cloud.autoconfigure.implementation.appconfiguration.AzureAppConfigurationProperties;
 import com.azure.spring.cloud.config.ConfigurationClientCustomizer;
 import com.azure.spring.cloud.config.implementation.http.policy.BaseAppConfigurationPolicy;
+import com.azure.spring.cloud.config.implementation.http.policy.TracingInfo;
 import com.azure.spring.cloud.config.implementation.properties.ConfigStore;
 import com.azure.spring.cloud.service.implementation.appconfiguration.ConfigurationClientBuilderFactory;
 
@@ -72,7 +75,7 @@ public class AppConfigurationReplicaClientsBuilder implements EnvironmentAware {
     private boolean isDev = false;
 
     private boolean isKeyVaultConfigured = false;
-    
+
     private final boolean credentialConfigured;
 
     private final int defaultMaxRetries;
@@ -134,7 +137,7 @@ List<AppConfigurationReplicaClient> buildClients(ConfigStore configStore) {
             throw new IllegalArgumentException(
                 "More than 1 Connection method was set for connecting to App Configuration.");
         }
-        
+
         boolean connectionStringIsPresent = configStore.getConnectionString() != null;
 
         if (credentialConfigured && connectionStringIsPresent) {
@@ -182,12 +185,13 @@ List<AppConfigurationReplicaClient> buildClients(ConfigStore configStore) {
 
     private AppConfigurationReplicaClient modifyAndBuildClient(ConfigurationClientBuilder builder, String endpoint,
         Integer replicaCount) {
-        builder.addPolicy(new BaseAppConfigurationPolicy(isDev, isKeyVaultConfigured, replicaCount));
+        TracingInfo tracingInfo = new TracingInfo(isDev, isKeyVaultConfigured, replicaCount, Configuration.getGlobalConfiguration());
+        builder.addPolicy(new BaseAppConfigurationPolicy(tracingInfo));
 
         if (clientProvider != null) {
             clientProvider.customize(builder, endpoint);
         }
-        return new AppConfigurationReplicaClient(endpoint, builder.buildClient());
+        return new AppConfigurationReplicaClient(endpoint, builder.buildClient(), tracingInfo);
     }
 
     @Override