Re: [PR] [FLINK-30284][flink-metrics] Adding datacenter url property to datadog [flink]

["pedromazala (via GitHub)" <gi...@apache.org>]

pedromazala commented on code in PR #25592:
URL: https://github.com/apache/flink/pull/25592#discussion_r2006170024


##########
flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporterFactory.java:
##########
@@ -44,10 +46,18 @@ public MetricReporter createMetricReporter(Properties config) {
         final String apiKey = config.getProperty(API_KEY, null);
         final String proxyHost = config.getProperty(PROXY_HOST, null);
         final int proxyPort = Integer.valueOf(config.getProperty(PROXY_PORT, "8080"));
+
+        if (config.containsKey(DATA_CENTER)) {
+            LOG.warn(
+                    "The 'dataCenter' option is deprecated; please use 'dataCenterUrl' instead.");

Review Comment:
   ```suggestion
                       "The '{}' option is deprecated; please use '{}' instead.", DATA_CENTER, DATA_CENTER_URL);
   ```



##########
flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporterFactory.java:
##########
@@ -44,10 +46,18 @@ public MetricReporter createMetricReporter(Properties config) {
         final String apiKey = config.getProperty(API_KEY, null);
         final String proxyHost = config.getProperty(PROXY_HOST, null);
         final int proxyPort = Integer.valueOf(config.getProperty(PROXY_PORT, "8080"));
+
+        if (config.containsKey(DATA_CENTER)) {
+            LOG.warn(
+                    "The 'dataCenter' option is deprecated; please use 'dataCenterUrl' instead.");
+        }
         final String rawDataCenter = config.getProperty(DATA_CENTER, "US");
+        final String dataCenterUrl = config.getProperty(
+                DATA_CENTER_URL,
+                String.format(DEFAULT_DATACENTER_DOMAIN,  DataCenter.valueOf(rawDataCenter).getDomain()));

Review Comment:
   And then have something like `DatadogHttpClient.getDefaultDomain(dataCenter)`



##########
flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporterFactory.java:
##########
@@ -30,11 +30,13 @@
 public class DatadogHttpReporterFactory implements MetricReporterFactory {
 
     private static final Logger LOG = LoggerFactory.getLogger(DatadogHttpReporterFactory.class);
+    private static final String DEFAULT_DATACENTER_DOMAIN = "https://app.datadoghq.%s";

Review Comment:
   Perhaps we could make all HTTP-related data live on `DatadogHttpClient`? This way we centralize HTTP knowledge in a single place. 



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org