feat: add GCP Parameter Manager OpenFeature provider#1771
feat: add GCP Parameter Manager OpenFeature provider#1771mahpatil wants to merge 3 commits intoopen-feature:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new OpenFeature provider for Google Cloud Parameter Manager, allowing feature flags to be evaluated from GCP parameters. The provider supports various data types (boolean, string, integer, double, and JSON objects) and includes an in-memory cache for performance. The changes encompass the core provider implementation, configuration options, unit and integration tests, and a sample application demonstrating its usage. Feedback indicates a potential race condition in the FlagCache.get method, suggesting a more robust removal mechanism for expired entries, and points out that the spotbugs-exclusions.xml file contains irrelevant entries that should be removed for clarity.
...r-manager/src/main/java/dev/openfeature/contrib/providers/gcpparametermanager/FlagCache.java
Show resolved
Hide resolved
Adds a new OpenFeature provider for GCP Parameter Manager that enables reading feature flags from GCP Parameter Manager secrets. Includes the provider implementation with flag caching, value conversion, unit tests, integration tests, and a sample application demonstrating usage. Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
1. Fix race condition in the get method 2. Remove spotbugs exclusions that were copy pasted Signed-off-by: Mahesh Patil <maheshfinity@gmail.com>
mahpatil
force-pushed
the
feat/gcp-parameter-manager
branch
from
c884e5e to
54ba9f2
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new OpenFeature provider for Google Cloud Parameter Manager, enabling feature flag management through GCP-native parameters. The implementation includes a thread-safe TTL-based in-memory cache, a converter for handling various data types (including JSON objects), and a sample application with setup scripts. The review feedback suggests correcting dependency versions for SLF4J and Log4j, updating the evaluation reason to accurately reflect the caching mechanism, and ensuring that original exception causes are preserved in error handling.
| <dependency> | ||
| <groupId>org.slf4j</groupId> | ||
| <artifactId>slf4j-api</artifactId> | ||
| <version>2.0.17</version> |
There was a problem hiding this comment.
The version 2.0.17 for slf4j-api is likely a typo or ahead of the current stable release. The latest stable version is 2.0.16.
| <version>2.0.17</version> | |
| <version>2.0.16</version> |
| <dependency> | ||
| <groupId>org.apache.logging.log4j</groupId> | ||
| <artifactId>log4j-slf4j2-impl</artifactId> | ||
| <version>2.25.0</version> |
There was a problem hiding this comment.
The version 2.25.0 for log4j-slf4j2-impl is likely a typo. The latest stable version is 2.24.3.
| <version>2.25.0</version> | |
| <version>2.24.3</version> |
| T value = FlagValueConverter.convert(rawValue, targetType); | ||
| return ProviderEvaluation.<T>builder() | ||
| .value(value) | ||
| .reason(Reason.STATIC.toString()) |
There was a problem hiding this comment.
Since the provider uses an in-memory cache with a TTL, it is more accurate to return Reason.CACHED instead of Reason.STATIC for evaluations.
| .reason(Reason.STATIC.toString()) | |
| .reason(Reason.CACHED.toString()) |
| } catch (NotFoundException e) { | ||
| throw new FlagNotFoundError("Parameter not found: " + parameterName); | ||
| } catch (Exception e) { | ||
| throw new GeneralError("Error fetching parameter '" + parameterName + "': " + e.getMessage()); |
There was a problem hiding this comment.
When throwing a GeneralError due to an exception, it is best practice to include the original cause to aid in debugging.
| throw new GeneralError("Error fetching parameter '" + parameterName + "': " + e.getMessage()); | |
| throw new GeneralError("Error fetching parameter '" + parameterName + "': " + e.getMessage(), e); |
2 participants
Summary
samples/gcp-parameter-manager-sample/with setup/teardown scriptsProvider Details
providers/gcp-parameter-managerGcpParameterManagerProviderTest plan
FlagCache,FlagValueConverter, andGcpParameterManagerProviderGcpParameterManagerProviderIntegrationTest) requires a real GCP project — setGCP_PROJECT_IDenv var to runsamples/gcp-parameter-manager-sample/README.mdto run end-to-end