You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/05/14 00:07:27 UTC
[GitHub] [flink] swhelan091 opened a new pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
swhelan091 opened a new pull request #12138:
URL: https://github.com/apache/flink/pull/12138
## What is the purpose of the change
Datadog limits how large a single request may be. If too large, Datadog returns a 413 and the metrics are not received. This pull request allows a user to set the maximum number of metrics to include in a single request to Datadog so they can control its size.
## Brief change log
*(for example:)*
- Set `maxMetricsPerRequest` from config or default if not found
- Kept track of the number of metrics in each request iteration. Send the request when the max size has been met or no more metrics are left to add.
## Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a6af75021894504c6515472be86f1e490ae32e27 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
}, {
"hash" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795",
"triggerID" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 3fa364afeda36c6e3de19804317d7470ecfcbd21 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
}, {
"hash" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795",
"triggerID" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"triggerType" : "PUSH"
}, {
"hash" : "27d88f5b04869ef3fd68e3fad99dfef3d16856e9",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1879",
"triggerID" : "27d88f5b04869ef3fd68e3fad99dfef3d16856e9",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 3fa364afeda36c6e3de19804317d7470ecfcbd21 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795)
* 27d88f5b04869ef3fd68e3fad99dfef3d16856e9 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1879)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628310224
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit a6af75021894504c6515472be86f1e490ae32e27 (Thu May 14 00:09:25 UTC 2020)
✅no warnings
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zentol commented on a change in pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #12138:
URL: https://github.com/apache/flink/pull/12138#discussion_r424957626
##########
File path: flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporter.java
##########
@@ -157,6 +181,11 @@ private void addGaugesAndUnregisterOnException(DSeries request) {
// Flink uses Gauge to store many types other than Number
g.getMetricValue();
request.addGauge(g);
+ ++currentCount;
+ if (currentCount % maxMetricsPerRequestValue == 0 || currentCount >= totalGauges) {
+ client.send(request);
Review comment:
hmm, I don't really like that we're always sending at least 2 requests, as long as at least one counter and gauge exist.
Ultimately, all we need to do is slice the currently created `DSeries`, which we could do using `ArrayList#subList`.
```
int fromIndex = 0;
ArrayList<DMetric> series = request.getSeries();
while (fromIndex < series.size()) {
int toIndex = Math.Min(currentCount + maxMetricsPerRequestValue, series.size());
client.send(series.subList(fromIndex, toIndex));
fromIndex = toIndex;
}
```
WDYT? We could even wrap this logic into a stateful `Consumer` that resets the `DSeries` on each `client.send`, so we don't have to assemble the full list.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zentol commented on a change in pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #12138:
URL: https://github.com/apache/flink/pull/12138#discussion_r426394581
##########
File path: flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporter.java
##########
@@ -157,6 +181,11 @@ private void addGaugesAndUnregisterOnException(DSeries request) {
// Flink uses Gauge to store many types other than Number
g.getMetricValue();
request.addGauge(g);
+ ++currentCount;
+ if (currentCount % maxMetricsPerRequestValue == 0 || currentCount >= totalGauges) {
+ client.send(request);
Review comment:
yes, you'D of course have to rap the subList in a DSeries again, requiring a new constructor.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] swhelan091 commented on a change in pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
swhelan091 commented on a change in pull request #12138:
URL: https://github.com/apache/flink/pull/12138#discussion_r427013055
##########
File path: flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporter.java
##########
@@ -137,15 +140,21 @@ public void report() {
counters.values().forEach(request::addCounter);
meters.values().forEach(request::addMeter);
- try {
- client.send(request);
- counters.values().forEach(DCounter::ackReport);
- LOGGER.debug("Reported series with size {}.", request.getSeries().size());
- } catch (SocketTimeoutException e) {
- LOGGER.warn("Failed reporting metrics to Datadog because of socket timeout: {}", e.getMessage());
- } catch (Exception e) {
- LOGGER.warn("Failed reporting metrics to Datadog.", e);
+ int totalMetrics = request.getSeries().size();
+ int fromIndex = 0;
+ while (fromIndex < totalMetrics) {
+ int toIndex = Math.min(fromIndex + maxMetricsPerRequestValue, totalMetrics);
+ try {
+ client.send(new DSeries(request.getSeries().subList(fromIndex, toIndex)));
+ } catch (SocketTimeoutException e) {
+ LOGGER.warn("Failed reporting metrics to Datadog because of socket timeout: {}", e.getMessage());
+ } catch (Exception e) {
+ LOGGER.warn("Failed reporting metrics to Datadog.", e);
+ }
+ fromIndex = toIndex;
}
+ LOGGER.debug("Reported series with size {}.", totalMetrics);
+ counters.values().forEach(DCounter::ackReport);
Review comment:
One question here. Should `ackReport` only be called on counters that were successfully emitted to Datadog?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a6af75021894504c6515472be86f1e490ae32e27 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
}, {
"hash" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a6af75021894504c6515472be86f1e490ae32e27 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218)
* 3fa364afeda36c6e3de19804317d7470ecfcbd21 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a6af75021894504c6515472be86f1e490ae32e27 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
}, {
"hash" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795",
"triggerID" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a6af75021894504c6515472be86f1e490ae32e27 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218)
* 3fa364afeda36c6e3de19804317d7470ecfcbd21 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zentol commented on a change in pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #12138:
URL: https://github.com/apache/flink/pull/12138#discussion_r427208124
##########
File path: flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporter.java
##########
@@ -137,15 +140,21 @@ public void report() {
counters.values().forEach(request::addCounter);
meters.values().forEach(request::addMeter);
- try {
- client.send(request);
- counters.values().forEach(DCounter::ackReport);
- LOGGER.debug("Reported series with size {}.", request.getSeries().size());
- } catch (SocketTimeoutException e) {
- LOGGER.warn("Failed reporting metrics to Datadog because of socket timeout: {}", e.getMessage());
- } catch (Exception e) {
- LOGGER.warn("Failed reporting metrics to Datadog.", e);
+ int totalMetrics = request.getSeries().size();
+ int fromIndex = 0;
+ while (fromIndex < totalMetrics) {
+ int toIndex = Math.min(fromIndex + maxMetricsPerRequestValue, totalMetrics);
+ try {
+ client.send(new DSeries(request.getSeries().subList(fromIndex, toIndex)));
+ } catch (SocketTimeoutException e) {
+ LOGGER.warn("Failed reporting metrics to Datadog because of socket timeout: {}", e.getMessage());
+ } catch (Exception e) {
+ LOGGER.warn("Failed reporting metrics to Datadog.", e);
+ }
+ fromIndex = toIndex;
}
+ LOGGER.debug("Reported series with size {}.", totalMetrics);
+ counters.values().forEach(DCounter::ackReport);
Review comment:
yes; if an exception occurred for a sublist that contained a counter then we should not ack it.
As a quick solution (which is somewhat hacky, but I do want this in 1.11.0), we could add a no-op `DMetric#ackReport()` method that is overridden by `DCounter`. You could then ack them in the `try` section by iterating over the sub DSeries.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
}, {
"hash" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795",
"triggerID" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"triggerType" : "PUSH"
}, {
"hash" : "27d88f5b04869ef3fd68e3fad99dfef3d16856e9",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "27d88f5b04869ef3fd68e3fad99dfef3d16856e9",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 3fa364afeda36c6e3de19804317d7470ecfcbd21 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795)
* 27d88f5b04869ef3fd68e3fad99dfef3d16856e9 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628310224
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit cfa7085df16698ec5a746648d6e8c723a94dbe07 (Fri Oct 16 10:51:50 UTC 2020)
✅no warnings
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12138:
URL: https://github.com/apache/flink/pull/12138#issuecomment-628319069
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a6af75021894504c6515472be86f1e490ae32e27",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1218",
"triggerID" : "a6af75021894504c6515472be86f1e490ae32e27",
"triggerType" : "PUSH"
}, {
"hash" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1795",
"triggerID" : "3fa364afeda36c6e3de19804317d7470ecfcbd21",
"triggerType" : "PUSH"
}, {
"hash" : "27d88f5b04869ef3fd68e3fad99dfef3d16856e9",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1879",
"triggerID" : "27d88f5b04869ef3fd68e3fad99dfef3d16856e9",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 27d88f5b04869ef3fd68e3fad99dfef3d16856e9 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1879)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zentol merged pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
zentol merged pull request #12138:
URL: https://github.com/apache/flink/pull/12138
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] swhelan091 commented on a change in pull request #12138: [FLINK-16611] [datadog-metrics] Make number of metrics per request configurable
Posted by GitBox <gi...@apache.org>.
swhelan091 commented on a change in pull request #12138:
URL: https://github.com/apache/flink/pull/12138#discussion_r426019442
##########
File path: flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporter.java
##########
@@ -157,6 +181,11 @@ private void addGaugesAndUnregisterOnException(DSeries request) {
// Flink uses Gauge to store many types other than Number
g.getMetricValue();
request.addGauge(g);
+ ++currentCount;
+ if (currentCount % maxMetricsPerRequestValue == 0 || currentCount >= totalGauges) {
+ client.send(request);
Review comment:
I do like the sublist approach although `DatadogHttpClient` serializes a `DSeries` not a `List`.
maybe something like:
```
List<DMetric> metrics = new ArrayList();
addGaugesAndUnregisterOnException(metrics);
metrics.addAll(counters.values());
metrics.addAll(meters.values());
int fromIndex = 0;
while (fromIndex < metrics.size()) {
int toIndex = Math.Min(fromIndex + maxMetricsPerRequestValue, metrics.size());
client.send(new DSeries(metrics.subList(fromIndex, toIndex)));
fromIndex = toIndex;
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org