Skip to content

Commit

Permalink
ENG-25947: metrics empty response bug (#13)
Browse files Browse the repository at this point in the history
* ENG-25948: Added descriptive message when port conflict occurs (instead of stacktrace). Prometheus output includes more useful host name as host_name label, e.g. tomaszs-mbp-2_localdomain instead of localhost. Logging of errors that could happen during Prometheus response generation.

* ENG-25947: Fixed concurrency issue when trying to iterate over synchronised HdrHistogram.
  • Loading branch information
tkowalcz authored May 20, 2024
1 parent 66d2d3a commit 41fee9e
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 28 deletions.
11 changes: 10 additions & 1 deletion src/main/java/org/voltdb/meshmonitor/HistogramWithDelta.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

import org.HdrHistogram.SynchronizedHistogram;

import java.util.function.Consumer;

public class HistogramWithDelta {

private final ConsoleLogger logger;
Expand All @@ -34,7 +36,14 @@ public void recordValueWithExpectedInterval(long value, long expectedIntervalBet
}
}

public SynchronizedHistogram getCumulativeHistogram() {
public void getCumulativeHistogram(Consumer<SynchronizedHistogram> consumer) {
synchronized (histogram) {
consumer.accept(histogram);
}
}

// @VisibleForTesting
SynchronizedHistogram getCumulativeHistogram() {
return histogram;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
*/
package org.voltdb.meshmonitor.metrics;

import java.net.InetSocketAddress;

import org.voltdb.meshmonitor.MeshMonitorTimings;
import org.voltdb.meshmonitor.Monitor;

import java.net.InetSocketAddress;

public class MonitorStatsPrinter {

private final HistogramPrinter histogramPrinter;
Expand All @@ -24,19 +24,25 @@ public void print(StringBuilder output, Monitor monitor) {
MeshMonitorTimings timings = monitor.getTimings();
InetSocketAddress remoteId = monitor.getRemoteId();

histogramPrinter.printHistogram(output,
timings.receiveHistogram().getCumulativeHistogram(),
remoteId,
"receive_seconds");

histogramPrinter.printHistogram(output,
timings.deltaHistogram().getCumulativeHistogram(),
remoteId,
"delta_seconds");

histogramPrinter.printHistogram(output,
timings.sendHistogram().getCumulativeHistogram(),
remoteId,
"send_seconds");
timings.receiveHistogram().getCumulativeHistogram(histogram ->
histogramPrinter.printHistogram(output,
histogram,
remoteId,
"receive_seconds")
);

timings.deltaHistogram().getCumulativeHistogram(histogram ->
histogramPrinter.printHistogram(output,
histogram,
remoteId,
"delta_seconds")
);

timings.sendHistogram().getCumulativeHistogram(histogram -> {
histogramPrinter.printHistogram(output,
histogram,
remoteId,
"send_seconds");
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
package org.voltdb.meshmonitor.metrics;

import org.HdrHistogram.SynchronizedHistogram;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.voltdb.meshmonitor.ConsoleLoggerTest;
Expand All @@ -20,25 +19,30 @@
public class HistogramPrinterTest {

private static final InetSocketAddress REMOTE_ID = new InetSocketAddress("remote.host.com", 8080);
private SynchronizedHistogram histogram;

private MeshMonitorTimings meshMonitorTimings;

@BeforeEach
void setUp() {
histogram = MeshMonitorTimings.createDefault(ConsoleLoggerTest.loggerForTest())
.deltaHistogram()
.getCumulativeHistogram();
meshMonitorTimings = MeshMonitorTimings.createDefault(ConsoleLoggerTest.loggerForTest());
}

@Test
public void shouldPrintBasicHistogram() {
// Given
histogram.recordValue(42L);
meshMonitorTimings
.deltaHistogram()
.recordValueWithExpectedInterval(42L, 42L);

HistogramPrinter printer = new HistogramPrinter("host_name");
StringBuilder actual = new StringBuilder();

// When
printer.printHistogram(actual, histogram, REMOTE_ID, "empty_histogram");
meshMonitorTimings
.deltaHistogram()
.getCumulativeHistogram(histogram ->
printer.printHistogram(actual, histogram, REMOTE_ID, "empty_histogram")
);

// Then
String result = actual.toString();
Expand All @@ -52,15 +56,27 @@ public void shouldPrintBasicHistogram() {
@Test
public void shouldFormatCumulativeBuckets() {
// Given
histogram.recordValue(42L);
histogram.recordValue(420L);
histogram.recordValue(4200L);
meshMonitorTimings
.deltaHistogram()
.recordValueWithExpectedInterval(42L, 42L);

meshMonitorTimings
.deltaHistogram()
.recordValueWithExpectedInterval(420L, 420L);

meshMonitorTimings
.deltaHistogram()
.recordValueWithExpectedInterval(4200L, 4200L);

HistogramPrinter printer = new HistogramPrinter("host_name");
StringBuilder actual = new StringBuilder();

// When
printer.printHistogram(actual, histogram, REMOTE_ID, "histogram");
meshMonitorTimings
.deltaHistogram()
.getCumulativeHistogram(histogram ->
printer.printHistogram(actual, histogram, REMOTE_ID, "histogram")
);

// Then
String result = actual.toString();
Expand Down

0 comments on commit 41fee9e

Please sign in to comment.