-
Notifications
You must be signed in to change notification settings - Fork 6
feat: create server status plugin #1137
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.
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
// SPDX-License-Identifier: Apache-2.0 | ||
plugins { id("org.hiero.gradle.module.library") } | ||
|
||
description = "Hiero Block Node - Block Node Server Status API Module" | ||
|
||
// Remove the following line to enable all 'javac' lint checks that we have turned on by default | ||
// and then fix the reported issues. | ||
tasks.withType<JavaCompile>().configureEach { options.compilerArgs.add("-Xlint:-exports") } | ||
|
||
mainModuleInfo { | ||
runtimeOnly("com.swirlds.config.impl") | ||
runtimeOnly("io.helidon.logging.jul") | ||
runtimeOnly("com.hedera.pbj.grpc.helidon.config") | ||
} | ||
|
||
testModuleInfo { | ||
requires("org.junit.jupiter.api") | ||
requires("org.hiero.block.node.app.test.fixtures") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// SPDX-License-Identifier: Apache-2.0 | ||
import org.hiero.block.node.server.status.ServerStatusServicePlugin; | ||
|
||
module org.hiero.block.node.server.status { | ||
uses com.swirlds.config.api.spi.ConfigurationBuilderFactory; | ||
|
||
requires transitive com.hedera.pbj.runtime; | ||
requires transitive org.hiero.block.node.spi; | ||
requires com.swirlds.metrics.api; | ||
requires org.hiero.block.protobuf; | ||
requires com.github.spotbugs.annotations; | ||
|
||
provides org.hiero.block.node.spi.BlockNodePlugin with | ||
ServerStatusServicePlugin; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
// SPDX-License-Identifier: Apache-2.0 | ||
package org.hiero.block.node.server.status; | ||
|
||
import static java.lang.System.Logger.Level.DEBUG; | ||
import static java.util.Objects.requireNonNull; | ||
|
||
import com.hedera.pbj.runtime.grpc.GrpcException; | ||
import com.hedera.pbj.runtime.grpc.Pipeline; | ||
import com.hedera.pbj.runtime.grpc.Pipelines; | ||
import com.hedera.pbj.runtime.grpc.ServiceInterface; | ||
import com.hedera.pbj.runtime.io.buffer.Bytes; | ||
import com.swirlds.metrics.api.Counter; | ||
import edu.umd.cs.findbugs.annotations.NonNull; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import org.hiero.block.api.ServerStatusRequest; | ||
import org.hiero.block.api.ServerStatusResponse; | ||
import org.hiero.block.api.protoc.BlockNodeServiceGrpc; | ||
import org.hiero.block.node.spi.BlockNodeContext; | ||
import org.hiero.block.node.spi.BlockNodePlugin; | ||
import org.hiero.block.node.spi.ServiceBuilder; | ||
import org.hiero.block.node.spi.historicalblocks.HistoricalBlockFacility; | ||
|
||
/** | ||
* Plugin that implements the BlockNodeService and provides the 'serverStatus' RPC. | ||
*/ | ||
public class ServerStatusServicePlugin implements BlockNodePlugin, ServiceInterface { | ||
/** The logger for this class. */ | ||
private final System.Logger LOGGER = System.getLogger(getClass().getName()); | ||
/** The block provider */ | ||
private HistoricalBlockFacility blockProvider; | ||
/** The block node context, used to provide access to facilities */ | ||
private BlockNodeContext context; | ||
Check warning on line 33 in block-node/server-status/src/main/java/org/hiero/block/node/server/status/ServerStatusServicePlugin.java
|
||
/** Counter for the number of requests */ | ||
private Counter requestCounter; | ||
|
||
/** | ||
* Handle a request for server status | ||
* | ||
* @param request the request containing the available blocks, state snapshot status and software version | ||
* @return the response containing the block or an error status | ||
*/ | ||
private ServerStatusResponse handleServiceStatusRequest(@NonNull final ServerStatusRequest request) { | ||
Check warning on line 43 in block-node/server-status/src/main/java/org/hiero/block/node/server/status/ServerStatusServicePlugin.java
|
||
LOGGER.log(DEBUG, "Handling ServerStatusRequest"); | ||
requestCounter.increment(); | ||
|
||
final ServerStatusResponse.Builder serverStatusResponse = ServerStatusResponse.newBuilder(); | ||
final long firstAvailableBlock = blockProvider.availableBlocks().min(); | ||
final long lastAvailableBlock = blockProvider.availableBlocks().max(); | ||
Comment on lines
+47
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a general question. Do we have to think about gaps? Here a hypothetical scenario:
Should we generally extend these requests to return ranges in some form which then could be parsed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, I think it's a valid consideration; however, this endpoint wouldn't care. I mean, the StatusServer query only aims to provide information about what should be available in the block-node. If something is missing, it's the responsibility of another plugin. There's currently an issue related to filling gaps—generally, if we say we have data between 0 and 7, then we should have it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Gaps are the responsibility of the backfill plugin (to be developed). The status service is intended to provide the edges (earliest and latest); not every detail. That said, nothing prevents us from adding an extension in the future if there is demand. |
||
|
||
// TODO(#579) Should get from state config or status, which would be provided by the context from responsible | ||
// facility | ||
boolean onlyLatestState = false; | ||
Comment on lines
+50
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: what is the meaning of this boolean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is indicating whether this block node offers the latest state snapshot. However we don't have state implemented currently, so it's irrelevant. If we don't set it, it will automatically be false. I left it here for visibility with the TODO comment. |
||
|
||
// TODO(#1139) Should get construct a block node version object from application config, which would be provided | ||
// by | ||
// the context from responsible facility | ||
Comment on lines
+54
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this plugin is the responsible facility. I don't think that becomes config so much as the build can insert the value into the code (as it's always constant for a given build output). That might require some build changes, however, so perhaps it would make sense to have it in config at least temporarily. Thoughts? |
||
|
||
return serverStatusResponse | ||
.firstAvailableBlock(firstAvailableBlock) | ||
.lastAvailableBlock(lastAvailableBlock) | ||
.onlyLatestState(onlyLatestState) | ||
.build(); | ||
} | ||
|
||
// ==== BlockNodePlugin Methods ==================================================================================== | ||
@Override | ||
public String name() { | ||
return "ServerStatusServicePlugin"; | ||
} | ||
|
||
@Override | ||
public void init(@NonNull final BlockNodeContext context, @NonNull final ServiceBuilder serviceBuilder) { | ||
requireNonNull(serviceBuilder); | ||
this.context = requireNonNull(context); | ||
this.blockProvider = requireNonNull(context.historicalBlockProvider()); | ||
|
||
// Create the metrics | ||
requestCounter = context.metrics() | ||
.getOrCreate(new Counter.Config(METRICS_CATEGORY, "server-status-requests") | ||
.withDescription("Number of server status requests")); | ||
|
||
// Register this service | ||
serviceBuilder.registerGrpcService(this); | ||
} | ||
// ==== ServiceInterface Methods =================================================================================== | ||
/** | ||
* BlockNodeService methods define the gRPC methods available on the BlockNodeService. | ||
*/ | ||
enum BlockNodeServiceMethod implements Method { | ||
/** | ||
* The serverStatus method retrieves the status of this Block Node. | ||
*/ | ||
serverStatus | ||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
*/ | ||
@NonNull | ||
@Override | ||
public String serviceName() { | ||
String[] parts = fullName().split("\\."); | ||
return parts[parts.length - 1]; | ||
Comment on lines
+102
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen this in some other places, I think this should become an utility maybe? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, opened a ticket to track this #1158 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally we'd follow the pattern in the stream subscriber plugin and only calculate the value once when the class is first loaded; storing it in a static constant. |
||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
*/ | ||
@NonNull | ||
@Override | ||
public String fullName() { | ||
return BlockNodeServiceGrpc.SERVICE_NAME; | ||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
*/ | ||
@NonNull | ||
@Override | ||
public List<Method> methods() { | ||
return Arrays.asList(BlockNodeServiceMethod.values()); | ||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
* | ||
* This is called each time a new request is received. | ||
*/ | ||
@NonNull | ||
@Override | ||
public Pipeline<? super Bytes> open( | ||
@NonNull Method method, @NonNull RequestOptions requestOptions, @NonNull Pipeline<? super Bytes> pipeline) | ||
throws GrpcException { | ||
final BlockNodeServiceMethod blockNodeServiceMethod = (BlockNodeServiceMethod) method; | ||
return switch (blockNodeServiceMethod) { | ||
case serverStatus: | ||
yield Pipelines.<ServerStatusRequest, ServerStatusResponse>unary() | ||
.mapRequest(ServerStatusRequest.PROTOBUF::parse) | ||
.method(this::handleServiceStatusRequest) | ||
.mapResponse(ServerStatusResponse.PROTOBUF::toBytes) | ||
.respondTo(pipeline) | ||
.build(); | ||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
// SPDX-License-Identifier: Apache-2.0 | ||
package org.hiero.block.node.server.status; | ||
|
||
import static org.hiero.block.node.app.fixtures.TestUtils.enableDebugLogging; | ||
import static org.hiero.block.node.app.fixtures.blocks.BlockItemUtils.toBlockItemsUnparsed; | ||
import static org.hiero.block.node.app.fixtures.blocks.SimpleTestBlockItemBuilder.createNumberOfVerySimpleBlocks; | ||
import static org.hiero.block.node.server.status.ServerStatusServicePlugin.BlockNodeServiceMethod.serverStatus; | ||
import static org.hiero.block.node.spi.BlockNodePlugin.UNKNOWN_BLOCK_NUMBER; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertFalse; | ||
import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
|
||
import com.hedera.hapi.block.stream.BlockItem; | ||
import com.hedera.pbj.runtime.ParseException; | ||
import com.hedera.pbj.runtime.grpc.ServiceInterface; | ||
import java.util.List; | ||
import org.hiero.block.api.ServerStatusRequest; | ||
import org.hiero.block.api.ServerStatusResponse; | ||
import org.hiero.block.node.app.fixtures.plugintest.GrpcPluginTestBase; | ||
import org.hiero.block.node.app.fixtures.plugintest.SimpleInMemoryHistoricalBlockFacility; | ||
import org.hiero.block.node.spi.blockmessaging.BlockItems; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
|
||
/** | ||
* Tests for the ServerStatusServicePlugin class. | ||
* Validates the functionality of the server status service and its responses | ||
* under different conditions. | ||
*/ | ||
public class ServerStatusServicePluginTest extends GrpcPluginTestBase<ServerStatusServicePlugin> { | ||
|
||
public ServerStatusServicePluginTest() { | ||
super(); | ||
start(new ServerStatusServicePlugin(), serverStatus, new SimpleInMemoryHistoricalBlockFacility()); | ||
} | ||
|
||
/** | ||
* Enable debug logging for each test. | ||
*/ | ||
@BeforeEach | ||
void setup() { | ||
enableDebugLogging(); | ||
} | ||
|
||
/** | ||
* Verifies that the service interface correctly registers and exposes | ||
* the server status method. | ||
*/ | ||
@Test | ||
@DisplayName("Should return correct method for ServerStatusServicePlugin") | ||
void shouldReturnCorrectMethod() { | ||
assertNotNull(serviceInterface); | ||
List<ServiceInterface.Method> methods = serviceInterface.methods(); | ||
assertNotNull(methods); | ||
assertEquals(1, methods.size()); | ||
assertEquals(serverStatus, methods.getFirst()); | ||
} | ||
|
||
/** | ||
* Tests that the server status response is valid when no blocks are available. | ||
* Verifies the first and last available block numbers and other response properties. | ||
* | ||
* @throws ParseException if there is an error parsing the response | ||
*/ | ||
@Test | ||
@DisplayName("Should return valid Server Status when no blocks available") | ||
void shouldReturnValidServerStatus() throws ParseException { | ||
// sendBlocks(5); | ||
final ServerStatusRequest request = ServerStatusRequest.newBuilder().build(); | ||
toPluginPipe.onNext(ServerStatusRequest.PROTOBUF.toBytes(request)); | ||
assertEquals(1, fromPluginBytes.size()); | ||
|
||
final ServerStatusResponse response = ServerStatusResponse.PROTOBUF.parse(fromPluginBytes.getFirst()); | ||
|
||
assertNotNull(response); | ||
assertEquals(0, response.firstAvailableBlock()); | ||
assertEquals(0, response.lastAvailableBlock()); | ||
Comment on lines
+77
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, thank you for noticing. Should be -1L, yes. I forgot it from some previous testing that I did 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally We use |
||
assertFalse(response.onlyLatestState()); | ||
|
||
// TODO() Remove when block node version information is implemented. | ||
assertFalse(response.hasVersionInformation()); | ||
} | ||
|
||
/** | ||
* Tests the server status response after adding a new batch of blocks. | ||
* Verifies that the first and last available block numbers are correctly updated. | ||
* | ||
* @throws ParseException if there is an error parsing the response | ||
*/ | ||
@Test | ||
@DisplayName("Should return valid Server Status, after new batch of blocks") | ||
void shouldReturnValidServerStatusForNewBlockBatch() throws ParseException { | ||
final int blocks = 5; | ||
sendBlocks(blocks); | ||
final ServerStatusRequest request = ServerStatusRequest.newBuilder().build(); | ||
toPluginPipe.onNext(ServerStatusRequest.PROTOBUF.toBytes(request)); | ||
assertEquals(1, fromPluginBytes.size()); | ||
|
||
final ServerStatusResponse response = ServerStatusResponse.PROTOBUF.parse(fromPluginBytes.getLast()); | ||
|
||
assertNotNull(response); | ||
assertEquals(0, response.firstAvailableBlock()); | ||
assertEquals(blocks - 1, response.lastAvailableBlock()); | ||
assertFalse(response.onlyLatestState()); | ||
|
||
// TODO() Remove when block node version information is implemented. | ||
assertFalse(response.hasVersionInformation()); | ||
} | ||
|
||
/** | ||
* Helper method to send a specified number of test blocks to the block messaging system. | ||
* | ||
* @param numberOfBlocks the number of test blocks to create and send | ||
*/ | ||
private void sendBlocks(int numberOfBlocks) { | ||
BlockItem[] blockItems = createNumberOfVerySimpleBlocks(numberOfBlocks); | ||
// Send some blocks | ||
for (BlockItem blockItem : blockItems) { | ||
long blockNumber = | ||
blockItem.hasBlockHeader() ? blockItem.blockHeader().number() : UNKNOWN_BLOCK_NUMBER; | ||
blockMessaging.sendBlockItems(new BlockItems(toBlockItemsUnparsed(blockItem), blockNumber)); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// SPDX-License-Identifier: Apache-2.0 | ||
package org.hiero.block.suites.server.status; | ||
|
||
import org.hiero.block.suites.server.status.positive.PositiveServerStatusTests; | ||
import org.junit.platform.suite.api.SelectClasses; | ||
import org.junit.platform.suite.api.Suite; | ||
|
||
@Suite | ||
@SelectClasses({PositiveServerStatusTests.class}) | ||
public class ServerStatusTestSuites { | ||
/** | ||
* Default constructor for the {@link ServerStatusTestSuites} class.This constructor is | ||
* empty as it does not need to perform any initialization. | ||
*/ | ||
public ServerStatusTestSuites() {} | ||
Check notice on line 15 in suites/src/main/java/org/hiero/block/suites/server/status/ServerStatusTestSuites.java
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps take a quick look at Jasper's update for how to remove the use of protoc here?