Skip to content

Commit

Permalink
Fix ClassCastException in DebugMetrics nested structures [hyperledger…
Browse files Browse the repository at this point in the history
…#7383] (hyperledger#7499)

* Fix ClassCastException in DebugMetrics nested structures

This commit resolves an issue where Double values in nested metric
structures were incorrectly cast to Map objects, causing a
ClassCastException. The fix allows for proper handling of both
direct values and nested structures at the same level.

A comprehensive test case has been added to reproduce the bug and
verify the fix, ensuring that complex, dynamically nested metric
structures can be handled without errors.

Resolves: hyperledger#7383

Signed-off-by: Ade Lucas <[email protected]>

---------

Signed-off-by: Ade Lucas <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: Snazzy <[email protected]>
Signed-off-by: Danno Ferrin <[email protected]>
Signed-off-by: Matilda Clerke <[email protected]>
Signed-off-by: Matilda-Clerke <[email protected]>
Signed-off-by: Sally MacFarlane <[email protected]>
Co-authored-by: garyschulte <[email protected]>
Co-authored-by: Fabio Di Fabio <[email protected]>
Co-authored-by: gringsam <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: Danno Ferrin <[email protected]>
Co-authored-by: Matilda-Clerke <[email protected]>
Signed-off-by: gconnect <[email protected]>
  • Loading branch information
7 people authored and gconnect committed Aug 26, 2024
1 parent 01a6158 commit 608de73
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 1 deletion.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## [Unreleased]

### Fixed
- **DebugMetrics**: Fixed a `ClassCastException` occurring in `DebugMetrics` when handling nested metric structures. Previously, `Double` values within these structures were incorrectly cast to `Map` objects, leading to errors. This update allows for proper handling of both direct values and nested structures at the same level. Issue# [#7383]

### Tests
- Added a comprehensive test case to reproduce the bug and verify the fix for the `ClassCastException` in `DebugMetrics`. This ensures that complex, dynamically nested metric structures can be handled without errors.

## Next release

### Upcoming Breaking Changes
Expand All @@ -17,6 +25,7 @@
- Correctly release txpool save and restore lock in case of exceptions [#7473](https://github.com/hyperledger/besu/pull/7473)
- Fix for `eth_gasPrice` could not retrieve block error [#7482](https://github.com/hyperledger/besu/pull/7482)


## 24.8.0

### Upcoming Breaking Changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,26 @@ private void addLabelledObservation(
@SuppressWarnings("unchecked")
private Map<String, Object> getNextMapLevel(
final Map<String, Object> current, final String name) {
// Use compute to either return the existing map or create a new one
return (Map<String, Object>)
current.computeIfAbsent(name, key -> new HashMap<String, Object>());
current.compute(
name,
(k, v) -> {
if (v instanceof Map) {
// If the value is already a Map, return it as is
return v;
} else {
// If the value is not a Map, create a new Map
Map<String, Object> newMap = new HashMap<>();
if (v != null) {
// If v is not null and not a Map, we store it as a leaf value
// If the original value was not null, store it under the "value" key
// This handles cases where a metric value (e.g., Double) was previously stored
// directly
newMap.put("value", v);
}
return newMap;
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.metrics.BesuMetricCategory.BLOCKCHAIN;
import static org.hyperledger.besu.metrics.BesuMetricCategory.PEERS;
import static org.hyperledger.besu.metrics.BesuMetricCategory.RPC;
import static org.mockito.Mockito.mock;
Expand All @@ -28,6 +29,7 @@
import org.hyperledger.besu.metrics.Observation;

import java.util.Collections;
import java.util.List;
import java.util.stream.Stream;

import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -84,6 +86,36 @@ public void shouldNestObservationsByLabel() {
ImmutableMap.of("label2B", "value3")))));
}

@Test
public void shouldHandleDoubleValuesInNestedStructureWithoutClassCastException() {
// Tests fix for issue# 7383: debug_metrics method error
when(metricsSystem.streamObservations())
.thenReturn(
Stream.of(
// This creates a double value for "a"
new Observation(BLOCKCHAIN, "nested_metric", 1.0, List.of("a")),
// This attempts to create a nested structure under "a", which was previously a
// double
new Observation(BLOCKCHAIN, "nested_metric", 2.0, asList("a", "b")),
// This adds another level of nesting
new Observation(BLOCKCHAIN, "nested_metric", 3.0, asList("a", "b", "c"))));

assertResponse(
ImmutableMap.of(
BLOCKCHAIN.getName(),
ImmutableMap.of(
"nested_metric",
ImmutableMap.of(
"a",
ImmutableMap.of(
"value",
1.0,
"b",
ImmutableMap.of(
"value", 2.0,
"c", 3.0))))));
}

private void assertResponse(final ImmutableMap<String, Object> expectedResponse) {
final JsonRpcSuccessResponse response = (JsonRpcSuccessResponse) method.response(REQUEST);
assertThat(response.getResult()).isEqualTo(expectedResponse);
Expand Down

0 comments on commit 608de73

Please sign in to comment.