davidradl commented on code in PR #27432:
URL: https://github.com/apache/flink/pull/27432#discussion_r2707918524


##########
flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporterFactory.java:
##########
@@ -65,4 +66,25 @@ public MetricReporter createMetricReporter(Properties 
config) {
                 tags,
                 useLogicalIdentifier);
     }
+
+    /**
+     * Retrieves the Datadog API key from configuration or environment 
variable.
+     *
+     * <p>The API key is resolved in the following order:
+     *
+     * <ol>
+     *   <li>Configuration property "apikey"
+     *   <li>Environment variable "DATADOG_API_KEY"
+     * </ol>
+     *
+     * @param config the configuration properties
+     * @return the Datadog API key, or null if not found in either location
+     */
+    private String getApiKey(Properties config) {

Review Comment:
   On the whole this change seems reasonable. Some points to raise:
   - would it not make sense for the environment variable to be honoured, so 
you can override the configuration at a more granular level?
   - Could you add this environment variable name and usage to the English and 
Chinese docs. 
   - I notice [the 
docs](https://nightlies.apache.org/flink/flink-docs-master/docs/deployment/config/)
 say 
   "On Docker-based deployments, you can use the FLINK_PROPERTIES environment 
variable for passing configuration values." Does this help you resolve this 
issue without this fix?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to