From 0ee1210f495ea256b6c37e60b6bcce88ced798b5 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 3 Jun 2025 10:29:22 -0700 Subject: [PATCH 1/5] Simple Fixes --- ...ationApplicationSettingPropertySource.java | 4 +- ...rationFeatureManagementPropertySource.java | 3 +- ...AppConfigurationKeyVaultClientFactory.java | 2 +- .../AppConfigurationPropertySource.java | 22 ++++++--- .../AppConfigurationReplicaClient.java | 16 +++++-- .../AppConfigurationReplicaClientFactory.java | 15 +++--- .../implementation/BackoffTimeCalculator.java | 47 +++++++++++++------ .../FeatureManagementConfiguration.java | 7 ++- 8 files changed, 80 insertions(+), 36 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java index 2f1f5e0f28a7..4ac64874a871 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationApplicationSettingPropertySource.java @@ -116,11 +116,11 @@ private void handleKeyVaultReference(String key, SecretReferenceConfigurationSet KeyVaultSecret secret = keyVaultClientFactory.getClient("https://" + uri.getHost()).getSecret(uri); properties.put(key, secret.getValue()); } catch (URISyntaxException e) { - LOGGER.error(String.format("Error Retrieving Key Vault Entry for key %s.", key)); + LOGGER.error("Error Retrieving Key Vault Entry for key {}.", key); throw new InvalidConfigurationPropertyValueException(key, "", "Invalid URI found in JSON property field 'uri' unable to parse."); } catch (RuntimeException e) { - LOGGER.error(String.format("Error Retrieving Key Vault Entry for key %s.", key)); + LOGGER.error("Error Retrieving Key Vault Entry for key {}.", key); throw e; } } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java index 63efff2e78bf..97efc7c20260 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationFeatureManagementPropertySource.java @@ -32,7 +32,8 @@ class AppConfigurationFeatureManagementPropertySource extends EnumerableProperty @Override public String[] getPropertyNames() { - if (featureFlagLoader != null && !featureFlagLoader.getFeatureFlags().isEmpty()) { + if (featureFlagLoader != null && featureFlagLoader.getFeatureFlags() != null + && !featureFlagLoader.getFeatureFlags().isEmpty()) { return new String[] { FEATURE_FLAG_KEY }; } return new String[0]; diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationKeyVaultClientFactory.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationKeyVaultClientFactory.java index 14882dd17047..e9ded98208b9 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationKeyVaultClientFactory.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationKeyVaultClientFactory.java @@ -15,9 +15,9 @@ * Vault host. */ class AppConfigurationKeyVaultClientFactory { - /** * Cache of secret client managers by Key Vault host. + */ private final Map keyVaultClients; /** diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySource.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySource.java index c2ce5df5e3f0..120adc189672 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySource.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationPropertySource.java @@ -5,7 +5,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Set; import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException; import org.springframework.core.env.EnumerablePropertySource; @@ -23,6 +22,8 @@ abstract class AppConfigurationPropertySource extends EnumerablePropertySource { /** + * Cache for storing configuration properties retrieved from Azure App Configuration. + */ protected final Map properties = new LinkedHashMap<>(); /** @@ -39,7 +40,7 @@ abstract class AppConfigurationPropertySource extends EnumerablePropertySource keySet = properties.keySet(); - return keySet.toArray(new String[keySet.size()]); + return properties.keySet().toArray(String[]::new); } /** * Returns the value of the specified property. * + * @param name the name of the property to retrieve + * @return the value of the property, or null if not found */ @Override public Object getProperty(String name) { @@ -70,11 +72,19 @@ public Object getProperty(String name) { * @return comma-separated string of labels, or empty string if null/empty */ protected static String getLabelName(String[] labelFilters) { - if (labelFilters == null) { + if (labelFilters == null || labelFilters.length == 0) { return ""; } return String.join(",", labelFilters); } - protected abstract void initProperties(List trim, Context context) throws InvalidConfigurationPropertyValueException; + /** + * Initializes the properties for this property source by loading them from Azure App Configuration. + * + * @param trim list of key prefixes to trim from configuration keys + * @param context the context for loading properties, may contain additional metadata + * @throws InvalidConfigurationPropertyValueException if there are issues with the configuration properties + */ + protected abstract void initProperties(List trim, Context context) + throws InvalidConfigurationPropertyValueException; } diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java index ed9d32b4c985..0cde712d5867 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java @@ -50,6 +50,7 @@ class AppConfigurationReplicaClient { /** * Holds Configuration Client and info needed to manage backoff. * @param endpoint client endpoint + * @param originClient origin client identifier * @param client Configuration Client to App Configuration store */ AppConfigurationReplicaClient(String endpoint, String originClient, ConfigurationClient client) { @@ -105,6 +106,7 @@ String getOriginClient() { * @param label String value of the watch key, use \0 for null. * @param context Azure SDK context for request correlation * @return The first returned configuration. + * @throws HttpResponseException if the request fails */ ConfigurationSetting getWatchKey(String key, String label, Context context) throws HttpResponseException { @@ -212,8 +214,9 @@ List listSettingSnapshot(String snapshotName, Context cont } boolean checkWatchKeys(SettingSelector settingSelector, Context context) { - List> results = client.listConfigurationSettings(settingSelector, context) - .streamByPage().filter(pagedResponse -> pagedResponse.getStatusCode() != 304).toList(); + List> results = client + .listConfigurationSettings(settingSelector, context) + .streamByPage().filter(pagedResponse -> pagedResponse.getStatusCode() != HTTP_NOT_MODIFIED).toList(); return results.size() > 0; } @@ -227,7 +230,14 @@ void updateSyncToken(String syncToken) { } } - private HttpResponseException hanndleHttpResponseException(HttpResponseException e) { + /** + * Handles HTTP response exceptions by determining if they are retryable. + * + * @param e the HTTP response exception to handle + * @return an AppConfigurationStatusException for retryable errors, or the original exception for non-retryable + * errors + */ + private HttpResponseException handleHttpResponseException(HttpResponseException e) { if (e.getResponse() != null) { int statusCode = e.getResponse().getStatusCode(); diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientFactory.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientFactory.java index 6c6221db85ca..63b2fd4db0d7 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientFactory.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClientFactory.java @@ -31,7 +31,7 @@ public class AppConfigurationReplicaClientFactory { AppConfigurationReplicaClientFactory(AppConfigurationReplicaClientsBuilder clientBuilder, List configStores, ReplicaLookUp replicaLookUp) { this.configStores = configStores; - if (CONNECTIONS.size() == 0) { + if (CONNECTIONS.isEmpty()) { for (ConfigStore store : configStores) { ConnectionManager manager = new ConnectionManager(clientBuilder, store, replicaLookUp); CONNECTIONS.put(manager.getMainEndpoint(), manager); @@ -51,9 +51,11 @@ public Map getConnections() { /** * Returns available replica clients for a given configuration store. * + * @param originEndpoint identifier of the store (primary endpoint) + * @return list of available replica clients for the store */ List getAvailableClients(String originEndpoint) { - return CONNECTIONS.get(originEndpoint).getAvailableClients(); + return getAvailableClients(originEndpoint, false); } /** @@ -73,7 +75,7 @@ List getAvailableClients(String originEndpoint, B * @param originEndpoint identifier of the store (primary endpoint) * @param endpoint the specific replica endpoint that failed */ - void backoffClientClient(String originEndpoint, String endpoint) { + void backoffClient(String originEndpoint, String endpoint) { CONNECTIONS.get(originEndpoint).backoffClient(endpoint); } @@ -98,10 +100,9 @@ Map getHealth() { */ String findOriginForEndpoint(String endpoint) { for (ConfigStore store : configStores) { - for (String replica : store.getEndpoints()) { - if (replica.equals(endpoint)) { - return store.getEndpoint(); - } + List replicas = store.getEndpoints(); + if (replicas != null && replicas.contains(endpoint)) { + return store.getEndpoint(); } } return endpoint; diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/BackoffTimeCalculator.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/BackoffTimeCalculator.java index ecedfbbc2a7c..39f378fe2eff 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/BackoffTimeCalculator.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/BackoffTimeCalculator.java @@ -22,28 +22,46 @@ final class BackoffTimeCalculator { private static final long SECONDS_TO_NANOSECONDS = 1_000_000_000L; /** - * Generator for introducing jitter in backoff calculations. Jitter helps prevent multiple - * clients from retrying simultaneously (thundering herd). + * Generator for introducing jitter in backoff calculations. Jitter helps prevent multiple clients from retrying + * simultaneously (thundering herd). */ private static final Random RANDOM = new Random(); - private static Long maxBackoff = (long) 600; + /** + * Maximum backoff time in seconds. Default: 600 seconds (10 minutes) - reasonable maximum to prevent excessively + * long waits. + */ + private static long maxBackoffSeconds = 600; + + /** + * Minimum backoff time in seconds. Default: 30 seconds - ensures proper rate limiting and prevents rapid retry + * loops. + */ + private static long minBackoffSeconds = 30; - private static Long minBackoff = (long) 30; + /** + * + * @param maxBackoff maximum amount of time between requests + * @param minBackoff minimum amount of time between requests + */ + static void setDefaults(Long maxBackoff, Long minBackoff) { + BackoffTimeCalculator.maxBackoffSeconds = maxBackoff != null ? maxBackoff : 600L; + BackoffTimeCalculator.minBackoffSeconds = minBackoff != null ? minBackoff : 30L; + } /** - * Calculates the new Backoff time for requests. - * @param attempts Number of attempts so far - * @return Nano Seconds to the next request - * @throws IllegalArgumentException when back off time or attempt number is invalid + * Calculates the exponential backoff time with jitter for retry operations. + * + * @param attempts the number of retry attempts made so far; must be non-negative + * @return the calculated backoff time in nanoseconds; never negative */ - static Long calculateBackoff(Integer attempts) { + static long calculateBackoff(Integer attempts) { - if (minBackoff < 0) { + if (minBackoffSeconds < 0) { throw new IllegalArgumentException("Minimum Backoff time needs to be greater than or equal to 0."); } - if (maxBackoff < 0) { + if (maxBackoffSeconds < 0) { throw new IllegalArgumentException("Maximum Backoff time needs to be greater than or equal to 0."); } @@ -51,10 +69,11 @@ static Long calculateBackoff(Integer attempts) { throw new IllegalArgumentException("Number of previous attempts needs to be a positive number."); } - long minBackoffNano = minBackoff * SECONDS_TO_NANO_SECONDS; - long maxBackoffNano = maxBackoff * SECONDS_TO_NANO_SECONDS; + final long minBackoffNano = minBackoffSeconds * SECONDS_TO_NANOSECONDS; + final long maxBackoffNano = maxBackoffSeconds * SECONDS_TO_NANOSECONDS; - if (attempts <= 1 || maxBackoff <= minBackoff) { + // For first attempts or when min equals max, return minimum backoff + if (attempts <= 1 || maxBackoffNano <= minBackoffNano) { return minBackoffNano; } diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManagementConfiguration.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManagementConfiguration.java index 8ca875376f42..9096856a8082 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManagementConfiguration.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManagementConfiguration.java @@ -76,8 +76,11 @@ public PercentageFilter percentageFilter() { @Scope("request") @ConditionalOnMissingBean(TargetingFilter.class) @ConditionalOnBean(TargetingContextAccessor.class) - public TargetingFilter targettingFilter(TargetingContextAccessor context) { - return new TargetingFilter(context, new TargetingEvaluationOptions().setIgnoreCase(true)); + public TargetingFilter targetingFilter(TargetingContextAccessor context, + ObjectProvider evaluationOptionsProvider) { + TargetingEvaluationOptions evaluationOptions = evaluationOptionsProvider + .getIfAvailable(() -> new TargetingEvaluationOptions().setIgnoreCase(true)); + return new TargetingFilter(context, evaluationOptions); } @Bean From 56917a5c5b22f9d5741b93eb9a9dbb766925de91 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 3 Jun 2025 10:38:41 -0700 Subject: [PATCH 2/5] Fixing Startup Failover logging --- .../AzureAppConfigDataLoader.java | 71 ++++++++++++++----- 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java index ce848571356e..616056a0a6b1 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java @@ -7,6 +7,7 @@ import java.io.IOException; import java.time.Instant; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import org.apache.commons.logging.Log; @@ -125,6 +126,7 @@ public ConfigData load(ConfigDataLoaderContext context, AzureAppConfigDataResour boolean reloadFailed = false; boolean pushRefresh = false; + Exception lastException = null; PushNotification notification = resource.getMonitoring().getPushNotification(); if ((notification.getPrimaryToken() != null && StringUtils.hasText(notification.getPrimaryToken().getName())) @@ -132,13 +134,13 @@ public ConfigData load(ConfigDataLoaderContext context, AzureAppConfigDataResour && StringUtils.hasText(notification.getPrimaryToken().getName()))) { pushRefresh = true; } - requestContext = new Context("refresh", resource.isRefresh()).addData(PUSH_REFRESH, - pushRefresh); - // Feature Management needs to be set in the last config store. + requestContext = new Context("refresh", resource.isRefresh()).addData(PUSH_REFRESH, pushRefresh); + + Iterator clientIterator = clients.iterator(); - for (AppConfigurationReplicaClient client : clients) { - sourceList = new ArrayList<>(); + while (clientIterator.hasNext()) { + AppConfigurationReplicaClient client = clientIterator.next(); if (reloadFailed && !AppConfigurationRefreshUtil.refreshStoreCheck(client, @@ -167,19 +169,31 @@ public ConfigData load(ConfigDataLoaderContext context, AzureAppConfigDataResour storeState.setState(resource.getEndpoint(), watchKeysSettings, monitoring.getRefreshInterval()); } - storeState.setLoadState(resource.getEndpoint(), true); + storeState.setLoadState(resource.getEndpoint(), true); // Success - configuration loaded, exit loop + lastException = null; + // Break out of the loop since we have successfully loaded configuration + break; } catch (AppConfigurationStatusException e) { reloadFailed = true; - replicaClientFactory.backoffClientClient(resource.getEndpoint(), client.getEndpoint()); + replicaClientFactory.backoffClient(resource.getEndpoint(), client.getEndpoint()); + lastException = e; + // Log the specific replica failure with context + logReplicaFailure(client, "status exception", clientIterator.hasNext(), e); } catch (Exception e) { - failedToGeneratePropertySource(e); - - // Not a retriable error - break; - } - if (sourceList.size() > 0) { - break; + // Store the exception to potentially use if all replicas fail + lastException = e; // Log the specific replica failure with context + logReplicaFailure(client, "exception", clientIterator.hasNext(), e); } + } // Check if all replicas failed + if (lastException != null && !resource.isRefresh()) { + // During startup, if all replicas failed, fail the application + logger.error("Azure App Configuration failed to load configuration during startup for store: " + + resource.getEndpoint() + ". Application cannot start without required configuration."); + failedToGeneratePropertySource(lastException); + } else if (lastException != null && resource.isRefresh()) { + // During refresh, log warning but don't fail the application + logger.warn("Azure App Configuration failed during refresh for store: " + + resource.getEndpoint() + ". Continuing with existing configuration."); } } @@ -256,15 +270,36 @@ private List createFeatureFlags(AppConfigurationReplicaClient clie return featureFlagWatchKeys; } + /** + * Logs a replica failure with contextual information about the failure scenario and available replicas. + * + * @param client the replica client that failed + * @param exceptionType a brief description of the exception type (e.g., "status exception", "exception") + * @param hasMoreReplicas whether there are additional replicas available to try + * @param exception the exception that caused the failure + */ + private void logReplicaFailure(AppConfigurationReplicaClient client, String exceptionType, + boolean hasMoreReplicas, Exception exception) { + String scenario = resource.isRefresh() ? "refresh" : "startup"; + String nextAction = hasMoreReplicas ? "Trying next replica." : "No more replicas available."; + + logger.warn("Azure App Configuration replica " + client.getEndpoint() + + " failed during " + scenario + " with " + exceptionType + ". " + + nextAction + " Store: " + resource.getEndpoint(), exception); + } + + /** + * Introduces a delay before throwing exceptions during startup to prevent fast crash loops. + */ private void delayException() { Instant currentDate = Instant.now(); - Instant preKillTIme = START_DATE.plusSeconds(PREKILL_TIME); - if (currentDate.isBefore(preKillTIme)) { - long diffInMillies = Math.abs(preKillTIme.toEpochMilli() - currentDate.toEpochMilli()); + Instant preKillTime = START_DATE.plusSeconds(PREKILL_TIME); + if (currentDate.isBefore(preKillTime)) { + long diffInMillies = Math.abs(preKillTime.toEpochMilli() - currentDate.toEpochMilli()); try { Thread.sleep(diffInMillies); } catch (InterruptedException e) { - logger.error("Failed to wait before fast fail."); + Thread.currentThread().interrupt(); // Restore interrupted status } } } From 97f84bccb805da8dd4d8193d0bfec783872e45c5 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 3 Jun 2025 10:40:07 -0700 Subject: [PATCH 3/5] Cleaned up resolver validation --- .../AzureAppConfigDataLocationResolver.java | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLocationResolver.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLocationResolver.java index 927b63d9ec72..41acde2f8765 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLocationResolver.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLocationResolver.java @@ -56,22 +56,29 @@ public boolean isResolvable(ConfigDataLocationResolverContext context, ConfigDat if (!location.hasPrefix(PREFIX)) { return false; } - Boolean hasEndpoint = StringUtils.hasText(context.getBinder() - .bind(AppConfigurationProperties.CONFIG_PREFIX + ".stores[0].endpoint", String.class) - .orElse("")); - Boolean hasConnectionString = StringUtils.hasText(context.getBinder() - .bind(AppConfigurationProperties.CONFIG_PREFIX + ".stores[0].connection-string", String.class) - .orElse("")); - Boolean hasEndpoints = StringUtils.hasText(context.getBinder() - .bind(AppConfigurationProperties.CONFIG_PREFIX + ".stores[0].endpoints", String.class) - .orElse("")); - Boolean hasConnectionStrings = StringUtils.hasText(context.getBinder() - .bind(AppConfigurationProperties.CONFIG_PREFIX + ".stores[0].connection-strings", String.class) - .orElse("")); - - return (hasEndpoint || hasConnectionString || hasEndpoints || hasConnectionStrings); + + // Check if the configuration properties for Azure App Configuration are present + return hasValidStoreConfiguration(context.getBinder()); + } + + /** + * Checks if the required configuration properties for Azure App Configuration are present. + * + * @param binder the binder to check for properties + * @return true if at least one of the required properties is present, false otherwise + */ + private boolean hasValidStoreConfiguration(Binder binder) { + // Check if any of the required properties for Azure App Configuration stores are present + String configPrefix = AppConfigurationProperties.CONFIG_PREFIX + ".stores[0]."; + + return hasNonEmptyProperty(binder, configPrefix + "endpoint") + || hasNonEmptyProperty(binder, configPrefix + "connection-string") + || hasNonEmptyProperty(binder, configPrefix + "endpoints") + || hasNonEmptyProperty(binder, configPrefix + "connection-strings"); } + private boolean hasNonEmptyProperty(Binder binder, String propertyPath) { + return StringUtils.hasText(binder.bind(propertyPath, String.class).orElse("")); } /** @@ -105,6 +112,12 @@ public List resolveProfileSpecific( AppConfigurationProperties properties = loadProperties(resolverContext); List locations = new ArrayList<>(); + if (properties.getStores() == null || properties.getStores().isEmpty()) { + throw new ConfigDataLocationNotFoundException(location, + "No Azure App Configuration stores are configured. Please check your application properties.", + new IllegalStateException("No stores configured")); + } + for (ConfigStore store : properties.getStores()) { locations.add( new AzureAppConfigDataResource(store, profiles, START_UP.get(), properties.getRefreshInterval())); From c45892d50083909e41a190bf1ef3754b59d5bd95 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 3 Jun 2025 10:45:46 -0700 Subject: [PATCH 4/5] Update ConnectionManager.java --- .../config/implementation/ConnectionManager.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java index 041e114884d2..87f23a730198 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java @@ -114,7 +114,7 @@ List getAvailableClients(Boolean useCurrent) { if (clients == null) { clients = clientBuilder.buildClients(configStore); - if (clients.size() == 0) { + if (clients.isEmpty()) { this.health = AppConfigurationStoreHealth.NOT_LOADED; } } @@ -183,9 +183,10 @@ void backoffClient(String endpoint) { } /** - * Updates the sync token of the client. Only works if no replicas are being used. - * - * @param syncToken App Configuration sync token + * Updates the synchronization token for the specified client endpoint. + * + * @param endpoint the endpoint URL of the client to update; may be null (method will have no effect if null) + * @param syncToken the new synchronization token; may be null to clear the existing token */ void updateSyncToken(String endpoint, String syncToken) { clients.stream().filter(client -> client.getEndpoint().equals(endpoint)).findFirst() From be99d2f8b2fecf6ba830cfcb6d893ccc8c801328 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 3 Jun 2025 10:54:51 -0700 Subject: [PATCH 5/5] Cleaning up telemetry publishing --- .../implementation/ConnectionManager.java | 2 + .../feature/management/FeatureManager.java | 93 +++++++++++-------- 2 files changed, 58 insertions(+), 37 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java index 87f23a730198..f160649abe06 100644 --- a/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java +++ b/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/ConnectionManager.java @@ -160,6 +160,8 @@ List getAvailableClients(Boolean useCurrent) { } else if (clients.size() > 0) { this.health = AppConfigurationStoreHealth.UP; } + + return availableClients; } /** diff --git a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManager.java b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManager.java index 8bf39dd1115c..d4e49d5e491e 100644 --- a/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManager.java +++ b/sdk/spring/spring-cloud-azure-feature-management/src/main/java/com/azure/spring/cloud/feature/management/FeatureManager.java @@ -14,7 +14,6 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.context.ApplicationContext; -import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; import com.azure.spring.cloud.feature.management.filters.ContextualFeatureFilter; @@ -50,9 +49,10 @@ * Used to evaluate the enabled state of a feature and/or get the assigned variant of a feature, if any. */ public class FeatureManager { - private static final Logger LOGGER = LoggerFactory.getLogger(FeatureManager.class); + private static final double PERCENTILE_MAXIMUM = 100.0; + private transient ApplicationContext context; private final FeatureManagementProperties featureManagementConfigurations; @@ -205,29 +205,32 @@ private Mono checkFeature(String featureName, Object featureCon if (!featureFlag.isEnabled()) { this.assignDefaultDisabledReason(event); event.setEnabled(false); - if (telemetryPublisher != null && featureFlag.getTelemetry() != null - && featureFlag.getTelemetry().isEnabled()) { - telemetryPublisher.publishTelemetry(event); - } + publishTelemetryIfEnabled(event); // If a feature flag is disabled and override can't enable it return Mono.just(event); } Mono result = this.checkFeatureFilters(event, featureContext); - result = assignAllocation(result); - result = result.doOnSuccess(resultEvent -> { - if (telemetryPublisher != null && featureFlag.getTelemetry() != null - && featureFlag.getTelemetry().isEnabled()) { - telemetryPublisher.publishTelemetry(resultEvent); - } - }); + result = result.doOnSuccess(resultEvent -> publishTelemetryIfEnabled(resultEvent)); return result; } - private Mono assignAllocation(Mono monoEvent) { + /** + * Publishes telemetry if enabled for the feature. + * + * @param event Evaluation event + */ + private void publishTelemetryIfEnabled(EvaluationEvent event) { + if (telemetryPublisher != null && event.getFeature() != null + && event.getFeature().getTelemetry() != null + && event.getFeature().getTelemetry().isEnabled()) { + telemetryPublisher.publishTelemetry(event); + } + } + private Mono assignAllocation(Mono monoEvent) { return monoEvent.map(event -> { Feature featureFlag = event.getFeature(); @@ -333,7 +336,8 @@ private void assignVariant(EvaluationEvent event) { double box = FeatureFilterUtils.isTargetedPercentage(contextId); for (PercentileAllocation percentileAllocation : allocation.getPercentile()) { Double to = percentileAllocation.getTo(); - if ((box == 100 && to == 100) || (percentileAllocation.getFrom() <= box && box < to)) { + if ((box == PERCENTILE_MAXIMUM && to == PERCENTILE_MAXIMUM) + || (percentileAllocation.getFrom() <= box && box < to)) { event.setReason(VariantAssignmentReason.PERCENTILE); variantName = percentileAllocation.getVariant(); break; @@ -385,28 +389,7 @@ private Mono checkFeatureFilters(EvaluationEvent event, Object for (FeatureFilterEvaluationContext featureFilter : featureFilters) { String filterName = featureFilter.getName(); - try { - Object filter = context.getBean(filterName); - featureFilter.setFeatureName(featureFlag.getId()); - if (filter instanceof FeatureFilter) { - filterResults.add(Mono.just(((FeatureFilter) filter).evaluate(featureFilter))); - } else if (filter instanceof ContextualFeatureFilter) { - filterResults - .add(Mono.just(((ContextualFeatureFilter) filter).evaluate(featureFilter, featureContext))); - } else if (filter instanceof FeatureFilterAsync) { - filterResults.add(((FeatureFilterAsync) filter).evaluateAsync(featureFilter)); - } else if (filter instanceof ContextualFeatureFilterAsync) { - filterResults - .add(((ContextualFeatureFilterAsync) filter).evaluateAsync(featureFilter, featureContext)); - } - } catch (NoSuchBeanDefinitionException e) { - LOGGER.error("Was unable to find Filter {}. Does the class exist and set as an @Component?", - filterName); - if (properties.isFailFast()) { - String message = "Fail fast is set and a Filter was unable to be found"; - ReflectionUtils.rethrowRuntimeException(new FilterNotFoundException(message, e, featureFilter)); - } - } + filterResults.add(evaluateFilter(featureFilter, featureContext, filterName)); } if (ALL_REQUIREMENT_TYPE.equals(featureFlag.getConditions().getRequirementType())) { @@ -448,4 +431,40 @@ public Set getAllFeatureNames() { return new HashSet( featureManagementConfigurations.getFeatureFlags().stream().map(feature -> feature.getId()).toList()); } + + /** + * Enhanced filter evaluation with better error handling and logging. + * + * @param featureFilter Feature filter evaluation context + * @param featureContext Feature context + * @param filterName Name of the filter for logging + * @return Mono result of filter evaluation + */ + private Mono evaluateFilter(FeatureFilterEvaluationContext featureFilter, Object featureContext, + String filterName) { + try { + Object filter = context.getBean(filterName); + featureFilter.setFeatureName(featureFilter.getFeatureName()); + if (filter instanceof FeatureFilter) { + return Mono.just(((FeatureFilter) filter).evaluate(featureFilter)); + } else if (filter instanceof ContextualFeatureFilter) { + return Mono.just(((ContextualFeatureFilter) filter).evaluate(featureFilter, featureContext)); + } else if (filter instanceof FeatureFilterAsync) { + return ((FeatureFilterAsync) filter).evaluateAsync(featureFilter); + } else if (filter instanceof ContextualFeatureFilterAsync) { + return ((ContextualFeatureFilterAsync) filter).evaluateAsync(featureFilter, featureContext); + } else { + LOGGER.warn("Filter {} does not implement any known filter interface", filterName); + return Mono.just(false); + } + } catch (NoSuchBeanDefinitionException e) { + LOGGER.error("Was unable to find Filter {}. Does the class exist and set as an @Component?", filterName, e); + if (properties.isFailFast()) { + String message = "Fail fast is set and a Filter was unable to be found"; + return Mono.error(new FilterNotFoundException(message, e, featureFilter)); + } + return Mono.just(false); + } + } + }