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

[hotfix] Report empty string as value for partition label for non-partitioned table #316

Merged
merged 6 commits into from
Jan 9, 2025
Merged

[hotfix] Report empty string as value for partition label for non-partitioned table #316

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f7716aa
[hotfix] Metric reporting for non-partitioned tables
michaelkoepf Jan 8, 2025
aac7408
[hotfix] Metric reporting for non-partitioned tables
michaelkoepf Jan 8, 2025
6c082e2
[hotfix] Metric reporting for non-partitioned tables
michaelkoepf Jan 8, 2025
7de0227
[hotfix] Metric reporting for non-partitioned tables
michaelkoepf Jan 8, 2025
bb9806f
[hotfix] Metric reporting for non-partitioned tables
michaelkoepf Jan 8, 2025
d5a3a8c
[observability] Add Grafana and Loki as log monitoring stack
michaelkoepf Jan 9, 2025
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
4 changes: 4 additions & 0 deletions ...etrics/fluss-metrics-jmx/src/test/java/com/alibaba/fluss/metrics/jmx/JMXReporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class JMXReporterTest {
static {
variables = new HashMap<>();
variables.put("<host>", "localhost");
variables.put("key1", "value1");
variables.put("key2", "");

metricGroup =
TestMetricGroup.newBuilder()
Expand Down Expand Up @@ -91,13 +93,15 @@ void testGenerateTable() {
vars.put("key0", "value0");
vars.put("key1", "value1");
vars.put("\"key2,=;:?'", "\"value2 (test),=;:?'");
vars.put("key3", "");

Hashtable<String, String> jmxTable = JMXReporter.generateJmxTable(vars);

assertThat(jmxTable).containsEntry("key0", "value0");
assertThat(jmxTable).containsEntry("key0", "value0");
assertThat(jmxTable).containsEntry("key1", "value1");
assertThat(jmxTable).containsEntry("key2------", "value2_(test)------");
assertThat(jmxTable).containsEntry("key3", "");
}

/**
Expand Down
137 changes: 103 additions & 34 deletions .../java/com/alibaba/fluss/metrics/prometheus/PrometheusReporterDifferentLabelValueTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@
import com.mashape.unirest.http.exceptions.UnirestException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.Arrays;
import java.util.stream.Stream;

import static com.alibaba.fluss.metrics.prometheus.PrometheusReporterTest.pollMetrics;
import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -42,18 +45,9 @@
class PrometheusReporterDifferentLabelValueTest {

private static final String[] LABEL_NAMES = {"label1", "label2"};
private static final String[] LABEL_VALUES_1 = new String[] {"value1_1", "value1_2"};
private static final String[] LABEL_VALUES_2 = new String[] {"value2_1", "value2_2"};
private static final String LOGICAL_SCOPE = "logical_scope";
private static final String METRIC_NAME = "myMetric";

private final MetricGroup metricGroup1 =
TestUtils.createTestMetricGroup(
LOGICAL_SCOPE, TestUtils.toMap(LABEL_NAMES, LABEL_VALUES_1));
private final MetricGroup metricGroup2 =
TestUtils.createTestMetricGroup(
LOGICAL_SCOPE, TestUtils.toMap(LABEL_NAMES, LABEL_VALUES_2));

private PrometheusReporter reporter;

@BeforeEach
Expand All @@ -68,8 +62,39 @@ void tearDown() {
}
}

@Test
void countersCanBeAddedSeveralTimesIfTheyDifferInLabels() {
private static Stream<Arguments> provideParameters() {
final String[] labelValues1 = new String[] {"value1_1", "value1_2"};
final String[] labelValues2 = new String[] {"value2_1", "value2_2"};
final String[] labelValues3 = new String[] {"value3_1", ""};
final String[] labelValues4 = new String[] {"values4_1", ""};

final MetricGroup metricGroup1 =
TestUtils.createTestMetricGroup(
LOGICAL_SCOPE, TestUtils.toMap(LABEL_NAMES, labelValues1));
final MetricGroup metricGroup2 =
TestUtils.createTestMetricGroup(
LOGICAL_SCOPE, TestUtils.toMap(LABEL_NAMES, labelValues2));
final MetricGroup metricGroup3 =
TestUtils.createTestMetricGroup(
LOGICAL_SCOPE, TestUtils.toMap(LABEL_NAMES, labelValues3));
final MetricGroup metricGroup4 =
TestUtils.createTestMetricGroup(
LOGICAL_SCOPE, TestUtils.toMap(LABEL_NAMES, labelValues4));

return Stream.of(
Arguments.of(metricGroup1, labelValues1, metricGroup2, labelValues2),
Arguments.of(metricGroup1, labelValues1, metricGroup3, labelValues3),
Arguments.of(metricGroup3, labelValues3, metricGroup2, labelValues2),
Arguments.of(metricGroup3, labelValues3, metricGroup4, labelValues4));
}

@ParameterizedTest
@MethodSource("provideParameters")
void countersCanBeAddedSeveralTimesIfTheyDifferInLabelValues(
MetricGroup metricGroup1,
String[] expectedLabelValues1,
MetricGroup metricGroup2,
String[] expectedLabelValues2) {
Counter counter1 = new SimpleCounter();
counter1.inc(1);
Counter counter2 = new SimpleCounter();
Expand All @@ -80,16 +105,21 @@ void countersCanBeAddedSeveralTimesIfTheyDifferInLabels() {

assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_1))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues1))
.isEqualTo(1.);
assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_2))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues2))
.isEqualTo(2.);
}

@Test
void gaugesCanBeAddedSeveralTimesIfTheyDifferInLabels() {
@ParameterizedTest
@MethodSource("provideParameters")
void gaugesCanBeAddedSeveralTimesIfTheyDifferInLabelValues(
MetricGroup metricGroup1,
String[] expectedLabelValues1,
MetricGroup metricGroup2,
String[] expectedLabelValues2) {
Gauge<Integer> gauge1 = () -> 3;
Gauge<Integer> gauge2 = () -> 4;

Expand All @@ -98,16 +128,21 @@ void gaugesCanBeAddedSeveralTimesIfTheyDifferInLabels() {

assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_1))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues1))
.isEqualTo(3.);
assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_2))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues2))
.isEqualTo(4.);
}

@Test
void metersCanBeAddedSeveralTimesIfTheyDifferInLabels() {
@ParameterizedTest
@MethodSource("provideParameters")
void metersCanBeAddedSeveralTimesIfTheyDifferInLabelValues(
MetricGroup metricGroup1,
String[] expectedLabelValues1,
MetricGroup metricGroup2,
String[] expectedLabelValues2) {
Meter meter1 = new TestMeter(1, 1.0);
Meter meter2 = new TestMeter(2, 2.0);

Expand All @@ -116,16 +151,22 @@ void metersCanBeAddedSeveralTimesIfTheyDifferInLabels() {

assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_1))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues1))
.isEqualTo(meter1.getRate());
assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_2))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues2))
.isEqualTo(meter2.getRate());
}

@Test
void histogramsCanBeAddedSeveralTimesIfTheyDifferInLabels() throws UnirestException {
@ParameterizedTest
@MethodSource("provideParameters")
void histogramsCanBeAddedSeveralTimesIfTheyDifferInLabelValues(
MetricGroup metricGroup1,
String[] expectedLabelValues1,
MetricGroup metricGroup2,
String[] expectedLabelValues2)
throws UnirestException {
TestHistogram histogram1 = new TestHistogram();
histogram1.setCount(1);
TestHistogram histogram2 = new TestHistogram();
Expand All @@ -135,28 +176,35 @@ void histogramsCanBeAddedSeveralTimesIfTheyDifferInLabels() throws UnirestExcept
reporter.notifyOfAddedMetric(histogram2, METRIC_NAME, metricGroup2);

final String exportedMetrics = pollMetrics(reporter.getPort()).getBody();
assertThat(exportedMetrics).contains("label2=\"value1_2\",} 1.0");
assertThat(exportedMetrics).contains("label2=\"value2_2\",} 2.0");
assertThat(exportedMetrics)
.contains(formatAsPrometheusLabels(LABEL_NAMES, expectedLabelValues1) + " 1.0");
assertThat(exportedMetrics)
.contains(formatAsPrometheusLabels(LABEL_NAMES, expectedLabelValues2) + " 2.0");

final String[] labelNamesWithQuantile = addToArray(LABEL_NAMES, "quantile");
for (Double quantile : PrometheusReporter.HistogramSummaryProxy.QUANTILES) {
assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME),
labelNamesWithQuantile,
addToArray(LABEL_VALUES_1, "" + quantile)))
addToArray(expectedLabelValues1, "" + quantile)))
.isEqualTo(quantile);
assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME),
labelNamesWithQuantile,
addToArray(LABEL_VALUES_2, "" + quantile)))
addToArray(expectedLabelValues2, "" + quantile)))
.isEqualTo(quantile);
}
}

@Test
void removingSingleInstanceOfMetricDoesNotBreakOtherInstances() {
@ParameterizedTest
@MethodSource("provideParameters")
void removingSingleInstanceOfMetricDoesNotBreakOtherInstances(
MetricGroup metricGroup1,
String[] expectedLabelValues1,
MetricGroup metricGroup2,
String[] expectedLabelValues2) {
Counter counter1 = new SimpleCounter();
counter1.inc(1);
Counter counter2 = new SimpleCounter();
Expand All @@ -167,23 +215,23 @@ void removingSingleInstanceOfMetricDoesNotBreakOtherInstances() {

assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_1))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues1))
.isEqualTo(1.);
assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_2))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues2))
.isEqualTo(2.);

reporter.notifyOfRemovedMetric(counter2, METRIC_NAME, metricGroup2);
assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_1))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues1))
.isEqualTo(1.);

reporter.notifyOfRemovedMetric(counter1, METRIC_NAME, metricGroup1);
assertThat(
reporter.registry.getSampleValue(
getLogicalScope(METRIC_NAME), LABEL_NAMES, LABEL_VALUES_1))
getLogicalScope(METRIC_NAME), LABEL_NAMES, expectedLabelValues2))
.isNull();
}

Expand All @@ -199,4 +247,25 @@ private String[] addToArray(String[] array, String element) {
labelNames[LABEL_NAMES.length] = element;
return labelNames;
}

private static String formatAsPrometheusLabels(String[] labelNames, String[] labelValues) {
if (labelNames == null
|| labelValues == null
|| labelNames.length == 0
|| labelValues.length == 0
|| labelNames.length != labelValues.length) {
throw new IllegalStateException("Erroneous test setup!");
}

final StringBuilder sb = new StringBuilder();
for (int i = 0; i < labelNames.length; i++) {
sb.append(labelNames[i]);
sb.append("=\"");
sb.append(labelValues[i]);
sb.append("\",");
}
sb.append("}");

return sb.toString();
}
}
4 changes: 4 additions & 0 deletions ...server/src/main/java/com/alibaba/fluss/server/metrics/group/PhysicalTableMetricGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,12 @@ public PhysicalTableMetricGroup(
protected void putVariables(Map<String, String> variables) {
variables.put("database", physicalTablePath.getDatabaseName());
variables.put("table", physicalTablePath.getTableName());

if (physicalTablePath.getPartitionName() != null) {
variables.put("partition", physicalTablePath.getPartitionName());
} else {
LOG.debug("Setting variable 'partition' for non-partitioned table to empty string.");
michaelkoepf marked this conversation as resolved.
Show resolved Hide resolved
variables.put("partition", "");
}
}

Expand Down