Skip to content
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.

Sign up for GitHub

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

[FLINK-30284][flink-metrics] Adding datacenter url property to datadog #25592

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[FLINK-30284][flink-metrics] Adding datacenter url property to datadog #25592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/content/docs/deployment/metric_reporters.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ Parameters:
- `apikey` - the Datadog API key
- `proxyHost` - (optional) The proxy host to use when sending to Datadog.
- `proxyPort` - (optional) The proxy port to use when sending to Datadog, defaults to 8080.
- `dataCenter` - (optional) The data center (`EU`/`US`) to connect to, defaults to `US`.
- `dataCenter` - (deprecated/optional) The data center (`EU`/`US`) to connect to, defaults to `US`. *DEPRECATED* will be removed in a future flink version, please use `dataCenterUrl`.
- `dataCenterUrl`- (optional) The datacenter url to connect to, defaults to `https://app.datadoghq.com`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest removing the deprecated option from the example below.

- `maxMetricsPerRequest` - (optional) The maximum number of metrics to include in each request, defaults to 2000.
- `useLogicalIdentifier` -> (optional) Whether the reporter uses a logical metric identifier, defaults to `false`.

Expand Down
10 changes: 5 additions & 5 deletions ...ink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ public class DatadogHttpClient {
private static final Logger LOGGER = LoggerFactory.getLogger(DatadogHttpClient.class);

private static final String SERIES_URL_FORMAT =
"https://app.datadoghq.%s/api/v1/series?api_key=%s";
"%s/api/v1/series?api_key=%s";
private static final String VALIDATE_URL_FORMAT =
"https://app.datadoghq.%s/api/v1/validate?api_key=%s";
"%s/api/v1/validate?api_key=%s";
private static final MediaType MEDIA_TYPE = MediaType.parse("application/json; charset=utf-8");
private static final int TIMEOUT = 3;
private static final ObjectMapper MAPPER = new ObjectMapper();
Expand All @@ -60,7 +60,7 @@ public DatadogHttpClient(
String dgApiKey,
String dgProxyHost,
int dgProxyPort,
DataCenter dataCenter,
String dataCenterUrl,
boolean validateApiKey) {
if (dgApiKey == null || dgApiKey.isEmpty()) {
throw new IllegalArgumentException("Invalid API key:" + dgApiKey);
Expand All @@ -79,8 +79,8 @@ public DatadogHttpClient(
.proxy(proxy)
.build();

seriesUrl = String.format(SERIES_URL_FORMAT, dataCenter.getDomain(), apiKey);
validateUrl = String.format(VALIDATE_URL_FORMAT, dataCenter.getDomain(), apiKey);
seriesUrl = String.format(SERIES_URL_FORMAT, dataCenterUrl, apiKey);
validateUrl = String.format(VALIDATE_URL_FORMAT, dataCenterUrl, apiKey);

if (validateApiKey) {
validateApiKey();
Expand Down
8 changes: 4 additions & 4 deletions ...k-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,20 @@ public DatadogHttpReporter(
String proxyHost,
int proxyPort,
int maxMetricsPerRequestValue,
DataCenter dataCenter,
String dataCenterUrl,
String tags,
boolean useLogicalIdentifier) {
this.maxMetricsPerRequestValue = maxMetricsPerRequestValue;
this.useLogicalIdentifier = useLogicalIdentifier;
this.client = new DatadogHttpClient(apiKey, proxyHost, proxyPort, dataCenter, true);
this.client = new DatadogHttpClient(apiKey, proxyHost, proxyPort, dataCenterUrl, true);
this.configTags = getTagsFromConfig(tags);

LOGGER.info(
"Configured DatadogHttpReporter with {tags={}, proxyHost={}, proxyPort={}, dataCenter={}, maxMetricsPerRequest={}",
"Configured DatadogHttpReporter with {tags={}, proxyHost={}, proxyPort={}, dataCenterUrl={}, maxMetricsPerRequest={}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking you would keep the original constructor and deprecate it and add a new constructor for the url.
The log here would then come out reflecting the users config.

I see you have refactored the callers to send through the url. deprecating might be cleaner. WDYT ?

The methods should all have @publicEvolving @internal tags as appropriate in all the methods in all the classes

tags,
proxyHost,
proxyPort,
dataCenter,
dataCenterUrl,
maxMetricsPerRequestValue);
}

Expand Down
14 changes: 12 additions & 2 deletions ...cs-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporterFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

private static final String API_KEY = "apikey";
private static final String PROXY_HOST = "proxyHost";
private static final String PROXY_PORT = "proxyPort";
private static final String DATA_CENTER = "dataCenter";
private static final String DATA_CENTER_URL = "dataCenterUrl";
private static final String TAGS = "tags";
private static final String MAX_METRICS_PER_REQUEST = "maxMetricsPerRequest";
private static final String USE_LOGICAL_IDENTIFIER = "useLogicalIdentifier";
Expand All @@ -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()));

final int maxMetricsPerRequestValue =
Integer.valueOf(config.getProperty(MAX_METRICS_PER_REQUEST, "2000"));
final DataCenter dataCenter = DataCenter.valueOf(rawDataCenter);
if (config.containsKey(TAGS)) {
LOG.warn(
"The 'tags' option is deprecated; please use 'scope.variables.additional' instead.");
Expand All @@ -61,7 +71,7 @@ public MetricReporter createMetricReporter(Properties config) {
proxyHost,
proxyPort,
maxMetricsPerRequestValue,
dataCenter,
dataCenterUrl,
tags,
useLogicalIdentifier);
}
Expand Down
9 changes: 5 additions & 4 deletions ...metrics-datadog/src/test/java/org/apache/flink/metrics/datadog/DatadogHttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class DatadogHttpClientTest {
tags.stream().collect(Collectors.joining("\",\"", "[\"", "\"]"));
private static final String HOST = "localhost";
private static final String METRIC = "testMetric";
private static final String DATACENTER_URL = "https://app.datadoghq.com";

private static final ObjectMapper MAPPER;

Expand All @@ -58,27 +59,27 @@ class DatadogHttpClientTest {

@Test
void testClientWithEmptyKey() {
assertThatThrownBy(() -> new DatadogHttpClient("", null, 123, DataCenter.US, false))
assertThatThrownBy(() -> new DatadogHttpClient("", null, 123, DATACENTER_URL, false))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
void testClientWithNullKey() {
assertThatThrownBy(() -> new DatadogHttpClient(null, null, 123, DataCenter.US, false))
assertThatThrownBy(() -> new DatadogHttpClient(null, null, 123, DATACENTER_URL, false))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
void testGetProxyWithNullProxyHost() {
DatadogHttpClient client =
new DatadogHttpClient("anApiKey", null, 123, DataCenter.US, false);
new DatadogHttpClient("anApiKey", null, 123, DATACENTER_URL, false);
assert (client.getProxy() == Proxy.NO_PROXY);
}

@Test
void testGetProxy() {
DatadogHttpClient client =
new DatadogHttpClient("anApiKey", "localhost", 123, DataCenter.US, false);
new DatadogHttpClient("anApiKey", "localhost", 123, DATACENTER_URL, false);

assertThat(client.getProxy().address()).isInstanceOf(InetSocketAddress.class);

Expand Down