-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Collection monitoring #47648
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
base: main
Are you sure you want to change the base?
Collection monitoring #47648
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds collection monitoring as a new refresh option for Azure App Configuration, introducing the spring.cloud.azure.appconfiguration.stores[0].monitoring.refresh-all configuration property. When enabled, this feature uses page-based ETags to monitor for changes across entire configuration collections rather than individual watch keys. The PR also includes important refactoring that renames the FeatureFlags class to CollectionMonitoring to better represent its generalized purpose, and fixes several bugs related to push notification token validation.
Key Changes:
- Introduces collection-based monitoring as an alternative to individual watch key monitoring
- Refactors
FeatureFlagstoCollectionMonitoringclass with package relocation - Adds
refreshAllproperty to enable collection monitoring without requiring explicit triggers - Fixes bug where secondary token name validation incorrectly checked primary token
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| CollectionMonitoring.java | Renamed from FeatureFlags, moved to configuration package to support both feature flags and general configuration monitoring |
| AppConfigurationStoreMonitoring.java | Added refreshAll property and updated validation to allow triggers to be optional when refreshAll is enabled; fixes spelling error |
| FeatureFlagState.java | Updated to use CollectionMonitoring instead of FeatureFlags |
| State.java | Added collectionWatchKeys field, improved immutability by making refreshAttempt final and adding withIncrementedRefreshAttempt method |
| StateHolder.java | Added overloaded setState method to support collection monitoring, enhanced documentation throughout |
| FeatureFlagClient.java | Updated to use CollectionMonitoring type |
| AzureAppConfigDataLoader.java | Added createCollectionMonitoring method, implements refreshAll logic, fixes secondary token validation bug |
| AppConfigurationSnapshotPropertySource.java | Updated to use CollectionMonitoring type |
| AppConfigurationReplicaClient.java | Added new collectionMonitoring method for retrieving configuration with page ETags |
| AppConfigurationRefreshUtil.java | Added refreshWithoutTimeCollectionMonitoring method, refactored duplicate retry logic into executeRefreshWithRetry, fixes secondary token bug and improves logging |
| AppConfigurationEventListener.java | Reordered static imports (no functional change) |
| FeatureFlagClientTest.java | Updated tests to use CollectionMonitoring |
| AppConfigurationReplicaClientTest.java | Updated test to use CollectionMonitoring |
| AppConfigurationRefreshUtilTest.java | Updated tests to use CollectionMonitoring |
Comments suppressed due to low confidence (1)
sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/configuration/CollectionMonitoring.java:36
- The JavaDoc comment still says "the featureFlags" but the method has been renamed to getConfigurations and the field renamed to configurations. Update the JavaDoc to reference "configurations" instead of "featureFlags".
...azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java
Show resolved
Hide resolved
.../com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java
Show resolved
Hide resolved
...m/azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationRefreshUtil.java
Show resolved
Hide resolved
...cloud/appconfiguration/config/implementation/properties/AppConfigurationStoreMonitoring.java
Outdated
Show resolved
Hide resolved
...cloud/appconfiguration/config/implementation/properties/AppConfigurationStoreMonitoring.java
Outdated
Show resolved
Hide resolved
.../com/azure/spring/cloud/appconfiguration/config/implementation/AzureAppConfigDataLoader.java
Show resolved
Hide resolved
| if (Boolean.TRUE.equals(monitoring.getRefreshAll())) { | ||
| // Use collection monitoring for refresh | ||
| List<CollectionMonitoring> collectionMonitoringList = createCollectionMonitoring(currentClient); | ||
| storeState.setState(resource.getEndpoint(), null, collectionMonitoringList, monitoring.getRefreshInterval()); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When refreshAll is enabled and no triggers are configured, setting the watchKeys parameter to null in setState could cause issues if the refresh logic attempts to iterate over watchKeys. Consider passing an empty list instead of null for better null-safety, or ensure all code paths that access watchKeys handle null appropriately.
Description
Adds collection monitoring as a refresh option.
spring.cloud.azure.appconfiguration.stores[0].monitoring.refresh-allwhen set to true will use page based etags to monitor for changes. When this refresh all is set you can't also set watch keys.Re-works the existing feature flag page monitoring to be more generic, so there is a bit of re-naming.
Fixes a found but where the wrong token was checked for non-null.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines