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:
[email protected]
