Bump com.arpnetworking.build:arpnetworking-parent-pom from 3.3.6 to 3.3.9

.junie/guidelines.md

@@ -0,0 +1,182 @@
+# Metrics Cluster Aggregator Development Guidelines
+
+This document provides guidelines and information for developers working on the Metrics Cluster Aggregator project.
+
+## Build/Configuration Instructions
+
+### Prerequisites
+
+- **Java**: The project requires Java 17 or later. The project uses the Maven wrapper (`mvnw`) which will download the appropriate JDK version if needed.
+- **Docker**: Required for building and running integration tests.
+
+### Building the Project
+
+To build the project:
+
+```bash
+./mvnw verify
+```
+
+To build without running tests:
+
+```bash
+./mvnw package -DskipTests
+```
+
+To install the project locally:
+
+```bash
+./mvnw install
+```
+
+### Docker Build
+
+The project includes Docker support. To build the Docker image:
+
+```bash
+./mvnw package docker:build
+```
+
+### Debugging
+
+To debug the server during run on port 9000:
+
+```bash
+./mvnw -Ddebug=true docker:start
+```
+
+To debug the server during integration tests on port 9000:
+
+```bash
+./mvnw -Ddebug=true verify
+```
+
+## Testing Information
+
+### Testing Framework
+
+The project uses:
+- JUnit 4 for unit testing
+- Mockito for mocking
+- Hamcrest for assertions
+- Pekko TestKit for testing actors
+
+### Running Tests
+
+To run all tests:
+
+```bash
+./mvnw test
+```
+
+To run a specific test:
+
+```bash
+./mvnw test -Dtest=ClassName
+```
+
+### Writing Tests
+
+1. **Unit Tests**: Place in `src/test/java` following the same package structure as the main code.
+2. **Actor Tests**: Extend `BaseActorTest` which provides an ActorSystem and Mockito initialization.
+3. **Test Naming**: Use descriptive names that indicate what is being tested.
+4. **Assertions**: Use Hamcrest matchers with `assertThat()` for readable assertions.
+
+### Example Test
+
+Here's a simple example of a test class:
+
+```java
+package com.arpnetworking.utility;
+
+import org.hamcrest.Matchers;
+import org.junit.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class StringUtilsTest {
+
+    @Test
+    public void testReverseWithNormalString() {
+        final String input = "hello";
+        final String expected = "olleh";
+        final String result = StringUtils.reverse(input);
+        assertThat(result, Matchers.equalTo(expected));
+    }
+
+    @Test
+    public void testReverseWithEmptyString() {
+        final String input = "";
+        final String expected = "";
+        final String result = StringUtils.reverse(input);
+        assertThat(result, Matchers.equalTo(expected));
+    }
+
+    @Test
+    public void testReverseWithNullString() {
+        final String input = null;
+        final String result = StringUtils.reverse(input);
+        assertThat(result, Matchers.nullValue());
+    }
+}
+```
+
+## Code Style and Development Practices
+
+### Code Style
+
+1. **Builder Pattern**: Use the `OvalBuilder` for creating objects with many parameters.
+2. **Immutability**: Use immutable collections (Guava's `ImmutableList`, `ImmutableSet`, etc.) and final fields.
+3. **Validation**: Use Oval for validation with annotations like `@NotNull`, `@NotEmpty`, and `@Range`.
+4. **Optional Values**: Use `Optional<T>` for values that might be absent.
+5. **Field Naming**: Private fields are prefixed with underscore (e.g., `_fieldName`).
+6. **Parameters**: Method parameters should be marked as `final`.
+7. **toString()**: Use `MoreObjects.toStringHelper` for implementing `toString()` methods.
+8. **Time Values**: Use `Duration` for representing time values.
+
+### Documentation
+
+1. **JavaDoc**: All public classes and methods should have JavaDoc comments.
+2. **License Header**: All source files should include the Apache 2.0 license header.
+
+### Project Structure
+
+1. **Package Organization**: Code is organized by functionality in packages.
+2. **Configuration**: Configuration is handled through JSON or HOCON files.
+3. **Actors**: The project uses Pekko (formerly Akka) for the actor system.
+
+### Dependency Injection
+
+The project uses Guice for dependency injection.
+
+### Logging
+
+The project uses SLF4J with Logback for logging.
+
+## Additional Development Information
+
+### Metrics
+
+The project uses the Metrics Client library for collecting and reporting metrics.
+
+### Configuration
+
+The application configuration is specified in JSON or HOCON format. Key configuration files:
+
+1. **Main Configuration**: Specifies system settings like ports, hosts, and timeouts.
+2. **Pipeline Configuration**: Defines the data processing pipeline for host and cluster statistics.
+
+### Pekko Configuration
+
+Pekko (formerly Akka) configuration is specified in the main configuration file under the `pekkoConfiguration` key.
+
+### Database Support
+
+The project supports multiple database configurations through the `databaseConfigurations` setting.
+
+### Deployment
+
+The project can be deployed as:
+1. A standalone application
+2. A Docker container
+3. An RPM package (using the `rpm` profile)

pom.xml

@@ -18,7 +18,7 @@
   <parent>
     <groupId>com.arpnetworking.build</groupId>
     <artifactId>arpnetworking-parent-pom</artifactId>
-    <version>3.3.6</version>
+    <version>3.3.9</version>
     <relativePath />
   </parent>
 

spotbugs.exclude.xml

@@ -29,6 +29,33 @@
          </Or>
      </Match>
 
+    <!-- Match THROWS_METHOD_THROWS_RUNTIMEEXCEPTION violations. We commonly
+    use runtime exceptions with lambdas, streams, and promises to indicate
+    failure
+-->
+    <Match>
+        <Bug pattern="THROWS_METHOD_THROWS_RUNTIMEEXCEPTION" />
+    </Match>
+
+    <!-- We often throw Exception in tests to indicate an error. The exception itself is the point, not how to handle it
+    -->
+    <Match>
+        <Or>
+            <Bug pattern="THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION" />
+            <Bug pattern="DE_MIGHT_IGNORE" />
+        </Or>
+        <Or>
+            <Class name="~.*\.[^\.]+Test" />
+            <Class name="~.*\.[^\.]+Test\$.*" />
+            <Class name="~.*\.[^\.]+TestPerf" />
+            <Class name="~.*\.[^\.]+TestPerf\$.*" />
+            <Class name="~.*\.[^\.]+IT" />
+            <Class name="~.*\.[^\.]+IT\$.*" />
+            <Class name="~.*\.[^\.]+ITPerf" />
+            <Class name="~.*\.[^\.]+ITPerf\$.*" />
+        </Or>
+    </Match>
+
      <!-- Match all RV_RETURN_VALUE_IGNORED_BAD_PRACTICE violations on all unit
         test files since Mockito usage can cause this violation when stating
         expectations.
@@ -57,6 +84,7 @@
          <Bug pattern="CI_CONFUSED_INHERITANCE" />
          <Bug pattern="MS_SHOULD_BE_FINAL" />
          <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+         <Bug pattern="AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE" />
        </Or>
        <Or>
          <Class name="~com\.arpnetworking\.clusteraggregator\.models\.ebean.*" />

src/main/java/com/arpnetworking/clusteraggregator/GracefulShutdownActor.java

@@ -21,7 +21,6 @@
 import com.arpnetworking.steno.LoggerFactory;
 import com.google.inject.Inject;
 import com.google.inject.name.Named;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.pekko.actor.AbstractActor;
 import org.apache.pekko.actor.ActorRef;
 import org.apache.pekko.actor.ActorSystem;
@@ -50,7 +49,6 @@ public class GracefulShutdownActor extends AbstractActor {
      * @param ingestActor ingest actor
      */
     @Inject
-    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "Context is safe to use in constructor.")
     public GracefulShutdownActor(
             @Named("aggregator-shard-region") final ActorRef shardRegion,
             @Named("host-emitter") final ActorRef hostEmitter,

src/main/java/com/arpnetworking/clusteraggregator/Status.java

@@ -22,7 +22,6 @@
 import com.arpnetworking.steno.Logger;
 import com.arpnetworking.steno.LoggerFactory;
 import com.arpnetworking.utility.CastMapper;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.pekko.actor.AbstractActor;
 import org.apache.pekko.actor.ActorRef;
 import org.apache.pekko.actor.Props;
@@ -58,7 +57,6 @@ public class Status extends AbstractActor {
      * @param clusterStatusCache The actor holding the cached cluster status.
      * @param localMetrics The actor holding the local node metrics.
      */
-    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "context is safe to be used in constructors")
     public Status(
             final Cluster cluster,
             final ActorRef clusterStatusCache,

src/main/java/com/arpnetworking/clusteraggregator/aggregation/AggregationRouter.java

@@ -83,7 +83,6 @@ public static Props props(
      * @param periodicMetrics The {@link PeriodicMetrics} instance.
      */
     @Inject
-    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "context is safe to be used in constructors")
     public AggregationRouter(
             @Named("periodic-statistics") final ActorRef periodicStatistics,
             @Named("cluster-emitter") final ActorRef emitter,
@@ -139,6 +138,7 @@ public void postStop() {
     }
 
     @Override
+    @SuppressFBWarnings(value = "THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", justification = "Exception is thrown by super method")
     public void preRestart(final Throwable reason, final Optional<Object> message) throws Exception {
         _periodicMetrics.recordCounter("actors/aggregation_router/restarted", 1);
         LOGGER.error()

src/main/java/com/arpnetworking/clusteraggregator/aggregation/StreamingAggregator.java

@@ -100,7 +100,6 @@ public static Props props(
      * @param periodicMetrics The {@link PeriodicMetrics} instance.
      */
     @Inject
-    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "context is safe to be used in constructors")
     public StreamingAggregator(
             @Named("periodic-statistics") final ActorRef periodicStatistics,
             @Named("cluster-emitter") final ActorRef emitter,
@@ -209,6 +208,7 @@ public void postStop() {
     }
 
     @Override
+    @SuppressFBWarnings(value = "THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", justification = "Exception is thrown by super method")
     public void preRestart(final Throwable reason, final Optional<Object> message) throws Exception {
         _periodicMetrics.recordCounter("actors/streaming_aggregator/restarted", 1);
         LOGGER.error()

src/main/java/com/arpnetworking/clusteraggregator/aggregation/package-info.java

@@ -14,6 +14,17 @@
  * limitations under the License.
  */
 
+/**
+ * Package containing components for metrics data aggregation.
+ * <p>
+ * This package provides classes that handle the aggregation of metrics data
+ * across a cluster, including routing of aggregation messages, extraction of
+ * message data, and streaming aggregation of metrics. These components form
+ * the core aggregation functionality of the cluster aggregator service.
+ * </p>
+ *
+ * @author Inscope Metrics
+ */
 @ParametersAreNonnullByDefault
 @ReturnValuesAreNonnullByDefault
 package com.arpnetworking.clusteraggregator.aggregation;

src/main/java/com/arpnetworking/clusteraggregator/client/AggClientConnection.java

@@ -29,7 +29,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.protobuf.GeneratedMessage;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.pekko.actor.AbstractActor;
 import org.apache.pekko.actor.ActorRef;
 import org.apache.pekko.actor.Props;
@@ -78,7 +77,6 @@ public static Props props(
      * @param maxConnectionAge The maximum duration to keep a connection open before cycling it.
      * @param calculateAggregates True to compute cluster aggregations, false to only publish host aggregations
      */
-    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "context is safe to be used in constructors")
     public AggClientConnection(
             final ActorRef connection,
             final InetSocketAddress remote,

src/main/java/com/arpnetworking/clusteraggregator/client/AggClientServer.java

@@ -67,7 +67,7 @@ public void preStart() throws Exception {
     }
 
     @Override
-    public void onReceive(final Object message) throws Exception {
+    public void onReceive(final Object message) {
         if (message instanceof Tcp.Bound) {
             final Tcp.Bound bound = (Tcp.Bound) message;
             LOGGER.debug()

src/main/java/com/arpnetworking/clusteraggregator/client/HttpSourceActor.java

@@ -37,7 +37,6 @@
 import com.google.inject.Inject;
 import com.google.inject.name.Named;
 import com.google.protobuf.GeneratedMessage;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.http.HttpHeaders;
 import org.apache.pekko.Done;
 import org.apache.pekko.NotUsed;
@@ -104,7 +103,6 @@ public final class HttpSourceActor extends AbstractActor {
      * @param periodicMetrics The periodic metrics instance.
      */
     @Inject
-    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "context is safe to be used in constructors")
     public HttpSourceActor(
             @Named("aggregator-shard-region") final ActorRef shardRegion,
             @Named("host-emitter") final ActorRef emitter,

src/main/java/com/arpnetworking/clusteraggregator/client/package-info.java

@@ -14,6 +14,16 @@
  * limitations under the License.
  */
 
+/**
+ * Package containing client connection and communication components.
+ * <p>
+ * This package provides classes for managing client connections to the cluster
+ * aggregator service, including TCP server components, client connection handlers,
+ * and HTTP source actors that receive and process metrics data from clients.
+ * </p>
+ *
+ * @author Inscope Metrics
+ */
 @ParametersAreNonnullByDefault
 @ReturnValuesAreNonnullByDefault
 package com.arpnetworking.clusteraggregator.client;

src/main/java/com/arpnetworking/clusteraggregator/configuration/package-info.java

@@ -14,6 +14,18 @@
  * limitations under the License.
  */
 
+/**
+ * Package containing configuration classes specific to the cluster aggregator.
+ * <p>
+ * This package provides configuration classes for various components of the
+ * cluster aggregator service, including the main service configuration,
+ * database configuration, emitter configuration, and rebalance configuration.
+ * These classes define the structure and validation of configuration data
+ * used to initialize and operate the cluster aggregator.
+ * </p>
+ *
+ * @author Inscope Metrics
+ */
 @ParametersAreNonnullByDefault
 @ReturnValuesAreNonnullByDefault
 package com.arpnetworking.clusteraggregator.configuration;