Skip to content

Commit 7bf730a

Browse files
ldematteldematte
authored
Fix: HierarchyCircuitBreakerTelemetryTests testCircuitBreakerTripCountMetric failing (#111831)
* Cleanup code and teardown for testCircuitBreakerTripCountMetric * Move to a more appropriate location
1 parent ca6d41c commit 7bf730a

File tree

2 files changed

+24
-80
lines changed

2 files changed

+24
-80
lines changed

‎muted-tests.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,6 @@ tests:
137137
- class: org.elasticsearch.xpack.restart.CoreFullClusterRestartIT
138138
method: testSnapshotRestore {cluster=UPGRADED}
139139
issue: https://github.com/elastic/elasticsearch/issues/111799
140-
- class: org.elasticsearch.indices.breaker.HierarchyCircuitBreakerTelemetryTests
141-
method: testCircuitBreakerTripCountMetric
142-
issue: https://github.com/elastic/elasticsearch/issues/111778
143140
- class: org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT
144141
method: test {comparison.RangeVersion SYNC}
145142
issue: https://github.com/elastic/elasticsearch/issues/111814

‎server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerTelemetryTests.java‎ renamed to ‎server/src/internalClusterTest/java/org/elasticsearch/indices/memory/breaker/HierarchyCircuitBreakerTelemetryIT.java‎

Lines changed: 24 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,23 @@
66
* Side Public License, v 1.
77
*/
88

9-
package org.elasticsearch.indices.breaker;
9+
package org.elasticsearch.indices.memory.breaker;
1010

1111
import org.elasticsearch.cluster.metadata.IndexMetadata;
1212
import org.elasticsearch.common.breaker.CircuitBreakingException;
1313
import org.elasticsearch.common.settings.Settings;
1414
import org.elasticsearch.common.unit.ByteSizeUnit;
15+
import org.elasticsearch.indices.breaker.CircuitBreakerMetrics;
16+
import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService;
1517
import org.elasticsearch.plugins.Plugin;
1618
import org.elasticsearch.plugins.PluginsService;
1719
import org.elasticsearch.rest.RestStatus;
1820
import org.elasticsearch.telemetry.Measurement;
19-
import org.elasticsearch.telemetry.RecordingInstruments;
20-
import org.elasticsearch.telemetry.RecordingMeterRegistry;
2121
import org.elasticsearch.telemetry.TestTelemetryPlugin;
22-
import org.elasticsearch.telemetry.metric.LongCounter;
23-
import org.elasticsearch.telemetry.metric.MeterRegistry;
2422
import org.elasticsearch.test.ESIntegTestCase;
2523
import org.hamcrest.Matchers;
24+
import org.junit.After;
2625

27-
import java.util.Arrays;
2826
import java.util.Collection;
2927
import java.util.List;
3028
import java.util.Map;
@@ -41,54 +39,11 @@
4139
import static org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService.TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING;
4240

4341
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, numClientNodes = 0, supportsDedicatedMasters = true)
44-
public class HierarchyCircuitBreakerTelemetryTests extends ESIntegTestCase {
42+
public class HierarchyCircuitBreakerTelemetryIT extends ESIntegTestCase {
4543

4644
@Override
4745
protected Collection<Class<? extends Plugin>> nodePlugins() {
48-
return List.of(TestCircuitBreakerTelemetryPlugin.class);
49-
}
50-
51-
public static class TestCircuitBreakerTelemetryPlugin extends TestTelemetryPlugin {
52-
protected final MeterRegistry meter = new RecordingMeterRegistry() {
53-
private final LongCounter tripCount = new RecordingInstruments.RecordingLongCounter(
54-
CircuitBreakerMetrics.ES_BREAKER_TRIP_COUNT_TOTAL,
55-
recorder
56-
) {
57-
@Override
58-
public void incrementBy(long inc) {
59-
throw new UnsupportedOperationException();
60-
}
61-
62-
@Override
63-
public void incrementBy(long inc, Map<String, Object> attributes) {
64-
throw new UnsupportedOperationException();
65-
}
66-
};
67-
68-
@Override
69-
protected LongCounter buildLongCounter(String name, String description, String unit) {
70-
if (name.equals(tripCount.getName())) {
71-
return tripCount;
72-
}
73-
throw new IllegalArgumentException("Unknown counter metric name [" + name + "]");
74-
}
75-
76-
@Override
77-
public LongCounter registerLongCounter(String name, String description, String unit) {
78-
assertCircuitBreakerName(name);
79-
return super.registerLongCounter(name, description, unit);
80-
}
81-
82-
@Override
83-
public LongCounter getLongCounter(String name) {
84-
assertCircuitBreakerName(name);
85-
return super.getLongCounter(name);
86-
}
87-
88-
private void assertCircuitBreakerName(final String name) {
89-
assertThat(name, Matchers.oneOf(CircuitBreakerMetrics.ES_BREAKER_TRIP_COUNT_TOTAL));
90-
}
91-
};
46+
return List.of(TestTelemetryPlugin.class);
9247
}
9348

9449
public void testCircuitBreakerTripCountMetric() {
@@ -142,37 +97,29 @@ public void testCircuitBreakerTripCountMetric() {
14297
fail("Expected exception not thrown");
14398
}
14499

145-
private List<Measurement> getMeasurements(String dataNodeName) {
146-
final TestTelemetryPlugin dataNodeTelemetryPlugin = internalCluster().getInstance(PluginsService.class, dataNodeName)
147-
.filterPlugins(TestCircuitBreakerTelemetryPlugin.class)
100+
@After
101+
public void resetClusterSetting() {
102+
final var circuitBreakerSettings = Settings.builder()
103+
.putNull(FIELDDATA_CIRCUIT_BREAKER_LIMIT_SETTING.getKey())
104+
.putNull(FIELDDATA_CIRCUIT_BREAKER_OVERHEAD_SETTING.getKey())
105+
.putNull(REQUEST_CIRCUIT_BREAKER_LIMIT_SETTING.getKey())
106+
.putNull(REQUEST_CIRCUIT_BREAKER_OVERHEAD_SETTING.getKey())
107+
.putNull(IN_FLIGHT_REQUESTS_CIRCUIT_BREAKER_LIMIT_SETTING.getKey())
108+
.putNull(IN_FLIGHT_REQUESTS_CIRCUIT_BREAKER_OVERHEAD_SETTING.getKey())
109+
.putNull(TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING.getKey())
110+
.putNull(HierarchyCircuitBreakerService.USE_REAL_MEMORY_USAGE_SETTING.getKey());
111+
updateClusterSettings(circuitBreakerSettings);
112+
}
113+
114+
private List<Measurement> getMeasurements(String nodeName) {
115+
final TestTelemetryPlugin telemetryPlugin = internalCluster().getInstance(PluginsService.class, nodeName)
116+
.filterPlugins(TestTelemetryPlugin.class)
148117
.toList()
149118
.get(0);
150119
return Measurement.combine(
151-
Stream.of(dataNodeTelemetryPlugin.getLongCounterMeasurement(CircuitBreakerMetrics.ES_BREAKER_TRIP_COUNT_TOTAL).stream())
120+
Stream.of(telemetryPlugin.getLongCounterMeasurement(CircuitBreakerMetrics.ES_BREAKER_TRIP_COUNT_TOTAL).stream())
152121
.flatMap(Function.identity())
153122
.toList()
154123
);
155124
}
156-
157-
// Make sure circuit breaker telemetry on trip count reports the same values as circuit breaker stats
158-
private void assertCircuitBreakerTripCount(
159-
final HierarchyCircuitBreakerService circuitBreakerService,
160-
final String circuitBreakerName,
161-
int firstBytesEstimate,
162-
int secondBytesEstimate,
163-
long expectedTripCountValue
164-
) {
165-
try {
166-
circuitBreakerService.getBreaker(circuitBreakerName).addEstimateBytesAndMaybeBreak(firstBytesEstimate, randomAlphaOfLength(5));
167-
circuitBreakerService.getBreaker(circuitBreakerName).addEstimateBytesAndMaybeBreak(secondBytesEstimate, randomAlphaOfLength(5));
168-
} catch (final CircuitBreakingException cbex) {
169-
final CircuitBreakerStats circuitBreakerStats = Arrays.stream(circuitBreakerService.stats().getAllStats())
170-
.filter(stats -> circuitBreakerName.equals(stats.getName()))
171-
.findAny()
172-
.get();
173-
assertThat(circuitBreakerService.getBreaker(circuitBreakerName).getTrippedCount(), Matchers.equalTo(expectedTripCountValue));
174-
assertThat(circuitBreakerStats.getTrippedCount(), Matchers.equalTo(expectedTripCountValue));
175-
}
176-
}
177-
178125
}

0 commit comments

Comments
 (0)