Introduce Dropwizard Metrics integration test (#1426)

.github/workflows/run-integration-tests.yml

@@ -1,9 +1,9 @@
 # If requested by means of a pull request comment, runs integration tests
 # against the project, using the code found on the pull request branch.
-# XXX: Generalize this to a matrix build of multiple integration tests,
+# XXX: Review whether then build matrix should also vary JDK or OS versions.
-# possibly using multiple JDK or OS versions.
+# XXX: Support `/integration-test [name...]` comment syntax to specify the
-# XXX: Investigate whether the comment can specify which integration tests run
+# subset of integration tests to run.
-# run. See this example of a dynamic build matrix:
+# See this example of a dynamic build matrix:
 # https://docs.github.com/en/actions/learn-github-actions/expressions#example-returning-a-json-object
 name: "Integration tests"
 on:
@@ -17,6 +17,9 @@ jobs:
     if: |
       github.event.issue.pull_request && contains(github.event.comment.body, '/integration-test')
     runs-on: ubuntu-24.04
+    strategy:
+      matrix:
+        integration-test: [ "checkstyle", "metrics" ]
     steps:
       - name: Install Harden-Runner
         uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
@@ -26,6 +29,7 @@ jobs:
           allowed-endpoints: >
             api.adoptium.net:443
             checkstyle.org:443
+            example.com:80
             github.com:443
             objects.githubusercontent.com:443
             oss.sonatype.org:443
@@ -42,12 +46,12 @@ jobs:
       - name: Install project to local Maven repository
         run: mvn -T1C install -DskipTests -Dverification.skip
       - name: Run integration test
-        run: xvfb-run ./integration-tests/checkstyle.sh "${{ runner.temp }}/artifacts"
+        run: xvfb-run "./integration-tests/${{ matrix.integration-test }}.sh" "${{ runner.temp }}/artifacts"
       - name: Upload artifacts on failure
         if: ${{ failure() }}
         uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
         with:
-          name: integration-test-checkstyle
+          name: "integration-test-${{ matrix.integration-test }}"
           path: "${{ runner.temp }}/artifacts"
       - name: Remove installed project artifacts
         run: mvn dependency:purge-local-repository -DmanualInclude='${project.groupId}' -DresolutionFuzziness=groupId

integration-tests/checkstyle.sh

@@ -8,6 +8,7 @@ repository='https://github.com/checkstyle/checkstyle.git'
 revision='checkstyle-10.20.1'
 additional_build_flags='-Perror-prone-compile,error-prone-test-compile -Dmaven.compiler.failOnError=true'
 additional_source_directories='${project.basedir}${file.separator}src${file.separator}it${file.separator}java,${project.basedir}${file.separator}src${file.separator}xdocs-examples${file.separator}java'
+shared_error_prone_flags='-XepExcludedPaths:(\Q${project.basedir}${file.separator}src${file.separator}\E(it|test|xdocs-examples)\Q${file.separator}resources\E|\Q${project.build.directory}${file.separator}\E).*'
 patch_error_prone_flags=''
 validation_error_prone_flags=''
 # Validation skips some tests:
@@ -29,7 +30,8 @@ fi
   "${revision}" \
   "${additional_build_flags}" \
   "${additional_source_directories}" \
+  "${shared_error_prone_flags}" \
   "${patch_error_prone_flags}" \
   "${validation_error_prone_flags}" \
   "${validation_build_flags}" \
-  ${@}
+  $@

integration-tests/metrics-expected-warnings.txt

@@ -0,0 +1,48 @@
+metrics-collectd/src/main/java/io/dropwizard/metrics5/collectd/CollectdReporter.java:[306,57] [rawtypes] found raw type: Gauge
+metrics-collectd/src/main/java/io/dropwizard/metrics5/collectd/PacketWriter.java:[164,12] [cast] redundant cast to ByteBuffer
+metrics-collectd/src/main/java/io/dropwizard/metrics5/collectd/PacketWriter.java:[169,11] [cast] redundant cast to ByteBuffer
+metrics-collectd/src/main/java/io/dropwizard/metrics5/collectd/PacketWriter.java:[175,17] [cast] redundant cast to ByteBuffer
+metrics-collectd/src/main/java/io/dropwizard/metrics5/collectd/PacketWriter.java:[200,8] [cast] redundant cast to ByteBuffer
+metrics-collectd/src/main/java/io/dropwizard/metrics5/collectd/PacketWriter.java:[203,21] [cast] redundant cast to ByteBuffer
+metrics-collectd/src/main/java/io/dropwizard/metrics5/collectd/PacketWriter.java:[206,11] [cast] redundant cast to ByteBuffer
+metrics-collectd/src/main/java/io/dropwizard/metrics5/collectd/PacketWriter.java:[250,36] [cast] redundant cast to ByteBuffer
+metrics-core/src/main/java/io/dropwizard/metrics5/CsvReporter.java:[390,35] [FormatStringConcatenation] Defer string concatenation to the invoked method
+metrics-core/src/main/java/io/dropwizard/metrics5/InstrumentedExecutorService.java:[244,25] [try] auto-closeable resource durationContext is never referenced in body of corresponding try statement
+metrics-core/src/main/java/io/dropwizard/metrics5/InstrumentedExecutorService.java:[266,25] [try] auto-closeable resource context is never referenced in body of corresponding try statement
+metrics-graphite/src/main/java/io/dropwizard/metrics5/graphite/GraphiteReporter.java:[431,14] [Slf4jLogStatement] Log statement contains 0 placeholders, but specifies 1 matching argument(s)
+metrics-graphite/src/main/java/io/dropwizard/metrics5/graphite/GraphiteReporter.java:[436,16] [Slf4jLogStatement] Log statement contains 0 placeholders, but specifies 1 matching argument(s)
+metrics-graphite/src/main/java/io/dropwizard/metrics5/graphite/GraphiteReporter.java:[449,17] [Slf4jLogStatement] Log statement contains 0 placeholders, but specifies 1 matching argument(s)
+metrics-healthchecks/src/test/java/io/dropwizard/metrics5/health/HealthCheckTest.java:[189,46] [TimeZoneUsage] Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone
+metrics-healthchecks/src/test/java/io/dropwizard/metrics5/health/HealthCheckTest.java:[203,46] [TimeZoneUsage] Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone
+metrics-httpclient/src/test/java/io/dropwizard/metrics5/httpclient/HttpClientMetricNameStrategiesTest.java:[124,22] [deprecation] rewriteURI(URI,HttpHost,boolean) in URIUtils has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedAsyncClientConnectionManager.java:[29,62] [deprecation] getDefault() in DefaultClientTlsStrategy has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientConnectionManager.java:[116,50] [deprecation] ConnectionSocketFactory in org.apache.hc.client5.http.socket has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientConnectionManager.java:[160,12] [deprecation] DefaultHttpClientConnectionOperator(Lookup<ConnectionSocketFactory>,SchemePortResolver,DnsResolver) in DefaultHttpClientConnectionOperator has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientConnectionManager.java:[30,28] [deprecation] ConnectionSocketFactory in org.apache.hc.client5.http.socket has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientConnectionManager.java:[31,28] [deprecation] ConnectionSocketFactory in org.apache.hc.client5.http.socket has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientConnectionManager.java:[32,37] [deprecation] PlainConnectionSocketFactory in org.apache.hc.client5.http.socket has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientConnectionManager.java:[33,38] [deprecation] SSLConnectionSocketFactory in org.apache.hc.client5.http.ssl has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientConnectionManager.java:[96,21] [deprecation] ConnectionSocketFactory in org.apache.hc.client5.http.socket has been deprecated
+metrics-httpclient5/src/main/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpRequestExecutor.java:[49,4] [deprecation] HttpRequestExecutor(Timeout,ConnectionReuseStrategy,Http1StreamListener) in HttpRequestExecutor has been deprecated
+metrics-httpclient5/src/test/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientsTest.java:[46,10] [deprecation] execute(ClassicHttpRequest) in HttpClient has been deprecated
+metrics-httpclient5/src/test/java/io/dropwizard/metrics5/httpclient5/InstrumentedHttpClientsTest.java:[68,12] [deprecation] execute(ClassicHttpRequest) in HttpClient has been deprecated
+metrics-influxdb/src/main/java/io/dropwizard/metrics5/influxdb/InfluxDbReporter.java:[282,14] [Slf4jLogStatement] Log statement contains 0 placeholders, but specifies 1 matching argument(s)
+metrics-influxdb/src/main/java/io/dropwizard/metrics5/influxdb/InfluxDbReporter.java:[287,16] [Slf4jLogStatement] Log statement contains 0 placeholders, but specifies 1 matching argument(s)
+metrics-jakarta-servlets/src/test/java/io/dropwizard/metrics5/servlets/HealthCheckServletTest.java:[31,67] [TimeZoneUsage] Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone
+metrics-jdbi3/src/test/java/io/dropwizard/metrics5/jdbi3/strategies/SmartNameStrategyTest.java:[18,10] [deprecation] InstrumentedTimingCollector in io.dropwizard.metrics5.jdbi3 has been deprecated
+metrics-jdbi3/src/test/java/io/dropwizard/metrics5/jdbi3/strategies/SmartNameStrategyTest.java:[24,20] [deprecation] InstrumentedTimingCollector in io.dropwizard.metrics5.jdbi3 has been deprecated
+metrics-jvm/src/test/java/io/dropwizard/metrics5/jvm/BufferPoolMetricSetTest.java:[101,74] [unchecked] unchecked cast
+metrics-jvm/src/test/java/io/dropwizard/metrics5/jvm/BufferPoolMetricSetTest.java:[56,74] [unchecked] unchecked cast
+metrics-jvm/src/test/java/io/dropwizard/metrics5/jvm/BufferPoolMetricSetTest.java:[65,74] [unchecked] unchecked cast
+metrics-jvm/src/test/java/io/dropwizard/metrics5/jvm/BufferPoolMetricSetTest.java:[74,74] [unchecked] unchecked cast
+metrics-jvm/src/test/java/io/dropwizard/metrics5/jvm/BufferPoolMetricSetTest.java:[83,74] [unchecked] unchecked cast
+metrics-jvm/src/test/java/io/dropwizard/metrics5/jvm/BufferPoolMetricSetTest.java:[92,74] [unchecked] unchecked cast
+metrics-legacy-adapter/src/main/java/com/codahale/metrics/MetricRegistry.java:[108,27] [rawtypes] found raw type: Gauge
+metrics-legacy-adapter/src/main/java/com/codahale/metrics/MetricRegistry.java:[112,27] [rawtypes] found raw type: Gauge
+metrics-legacy-adapter/src/main/java/com/codahale/metrics/MetricRegistry.java:[51,49] [rawtypes] found raw type: Gauge
+metrics-legacy-adapter/src/main/java/com/codahale/metrics/MetricRegistry.java:[51,9] [rawtypes] found raw type: Gauge
+metrics-legacy-adapter/src/test/java/com/codahale/metrics/MetricRegistryTest.java:[367,22] [rawtypes] found raw type: Gauge
+metrics-legacy-adapter/src/test/java/com/codahale/metrics/MetricRegistryTest.java:[50,4] [rawtypes] found raw type: Gauge
+metrics-log4j2/src/main/java/io/dropwizard/metrics5/log4j2/InstrumentedAppender.java:[85,4] [deprecation] AbstractAppender(String,Filter,Layout<? extends Serializable>,boolean) in AbstractAppender has been deprecated
+metrics-log4j2/src/main/java/io/dropwizard/metrics5/log4j2/InstrumentedAppender.java:[96,4] [deprecation] AbstractAppender(String,Filter,Layout<? extends Serializable>,boolean) in AbstractAppender has been deprecated
+metrics-servlets/src/test/java/io/dropwizard/metrics5/servlets/HealthCheckServletTest.java:[31,67] [TimeZoneUsage] Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone

integration-tests/metrics-init.patch

@@ -0,0 +1,98 @@
+--- a/metrics-jakarta-servlets/src/main/java/io/dropwizard/metrics5/servlets/MetricsServlet.java
++++ b/metrics-jakarta-servlets/src/main/java/io/dropwizard/metrics5/servlets/MetricsServlet.java
+@@ -188,6 +188,9 @@ public class MetricsServlet extends HttpServlet {
+         return mapper.writer();
+     }
+ 
++    // Here `value` may be `null`, while `TimeUnit#valueOf` requires a non-`null` argument.
++    // XXX: Investigate better nullness handling by `IdentityConversion`.
++    @SuppressWarnings("IdentityConversion")
+     protected TimeUnit parseTimeUnit(String value, TimeUnit defaultValue) {
+         try {
+             return TimeUnit.valueOf(String.valueOf(value).toUpperCase(Locale.US));
+--- a/metrics-servlets/src/main/java/io/dropwizard/metrics5/servlets/MetricsServlet.java
++++ b/metrics-servlets/src/main/java/io/dropwizard/metrics5/servlets/MetricsServlet.java
+@@ -188,6 +188,9 @@ public class MetricsServlet extends HttpServlet {
+         return mapper.writer();
+     }
+ 
++    // Here `value` may be `null`, while `TimeUnit#valueOf` requires a non-`null` argument.
++    // XXX: Investigate better nullness handling by `IdentityConversion`.
++    @SuppressWarnings("IdentityConversion")
+     protected TimeUnit parseTimeUnit(String value, TimeUnit defaultValue) {
+         try {
+             return TimeUnit.valueOf(String.valueOf(value).toUpperCase(Locale.US));
+--- a/pom.xml
++++ b/pom.xml
+@@ -136,6 +136,27 @@
+         </repository>
+     </distributionManagement>
+ 
++    <dependencyManagement>
++        <dependencies>
++            <dependency>
++                <groupId>com.google.errorprone</groupId>
++                <artifactId>error_prone_annotations</artifactId>
++                <version>${error-prone.version}</version>
++            </dependency>
++            <dependency>
++                <groupId>com.google.guava</groupId>
++                <artifactId>guava</artifactId>
++                <version>33.3.1-jre<!-- Renovate: com.google.guava:guava-bom --></version>
++            </dependency>
++        </dependencies>
++    </dependencyManagement>
++    <dependencies>
++        <dependency>
++            <groupId>com.google.guava</groupId>
++            <artifactId>guava</artifactId>
++        </dependency>
++    </dependencies>
++
+     <profiles>
+         <profile>
+             <id>jdk8</id>
+@@ -218,7 +239,7 @@
+                             <compilerArgs>
+                                 <arg>-Xlint:all</arg>
+                                 <arg>-XDcompilePolicy=simple</arg>
+-                                <arg>-Xplugin:ErrorProne -XepExcludedPaths:.*/target/generated-sources/.*</arg>
++                                <arg>-Xplugin:ErrorProne ${error-prone.configuration-args}</arg>
+                                 <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
+                                 <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
+                                 <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
+@@ -229,12 +250,24 @@
+                                 <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
+                                 <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
+                                 <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
++                                <arg>-Xmaxwarns</arg>
++                                <arg>1000000</arg>
+                             </compilerArgs>
+                             <annotationProcessorPaths>
+                                 <path>
+                                     <groupId>com.google.errorprone</groupId>
+                                     <artifactId>error_prone_core</artifactId>
+-                                    <version>${errorprone.version}</version>
++                                    <version>${error-prone.version}</version>
++                                </path>
++                                <path>
++                                    <groupId>tech.picnic.error-prone-support</groupId>
++                                    <artifactId>error-prone-contrib</artifactId>
++                                    <version>${error-prone-support.version}</version>
++                                </path>
++                                <path>
++                                    <groupId>tech.picnic.error-prone-support</groupId>
++                                    <artifactId>refaster-runner</artifactId>
++                                    <version>${error-prone-support.version}</version>
+                                 </path>
+                             </annotationProcessorPaths>
+                         </configuration>
+@@ -364,7 +397,7 @@
+                 <groupId>org.apache.maven.plugins</groupId>
+                 <artifactId>maven-compiler-plugin</artifactId>
+                 <configuration>
+-                    <release>8</release>
++                    <release>11</release>
+                     <fork>true</fork>
+                     <parameters>true</parameters>
+                     <showWarnings>true</showWarnings>

integration-tests/metrics.sh

@@ -0,0 +1,36 @@
+#!/usr/bin/env bash
+
+set -e -u -o pipefail
+
+test_name="$(basename "${0}" .sh)"
+project='metrics'
+repository='https://github.com/dropwizard/metrics.git'
+revision='v5.0.0-rc22'
+additional_build_flags=''
+additional_source_directories=''
+# XXX: Minimize the diff by including
+# `-XepOpt:Slf4jLoggerDeclaration:CanonicalStaticLoggerName=LOGGER` once such
+# flags are supported in patch mode. See
+# https://github.com/google/error-prone/pull/4699.
+shared_error_prone_flags='-XepExcludedPaths:.*/target/generated-sources/.*'
+patch_error_prone_flags=''
+validation_error_prone_flags=''
+validation_build_flags=''
+
+if [ "${#}" -gt 2 ] || ([ "${#}" = 2 ] && [ "${1:---sync}" != '--sync' ]); then
+  >&2 echo "Usage: ${0} [--sync] [<report_directory>]"
+  exit 1
+fi
+
+"$(dirname "${0}")/run-integration-test.sh" \
+  "${test_name}" \
+  "${project}" \
+  "${repository}" \
+  "${revision}" \
+  "${additional_build_flags}" \
+  "${additional_source_directories}" \
+  "${shared_error_prone_flags}" \
+  "${patch_error_prone_flags}" \
+  "${validation_error_prone_flags}" \
+  "${validation_build_flags}" \
+  $@

integration-tests/run-integration-test.sh

@@ -12,8 +12,8 @@ integration_test_root="$(cd "$(dirname -- "${0}")" && pwd)"
 error_prone_support_root="${integration_test_root}/.."
 repos_root="${integration_test_root}/.repos"
 
-if [ "${#}" -lt 9 ] || [ "${#}" -gt 11 ] || ([ "${#}" = 11 ] && [ "${10:---sync}" != '--sync' ]); then
+if [ "${#}" -lt 10 ] || [ "${#}" -gt 12 ] || ([ "${#}" = 12 ] && [ "${11:---sync}" != '--sync' ]); then
-  >&2 echo "Usage: $(basename "${0}") <test_name> <project> <repository> <revision> <additional_build_flags> <additional_source_directories> <patch_error_prone_flags> <validation_error_prone_flags> <validation_build_flags> [--sync] [<report_directory>]"
+  >&2 echo "Usage: $(basename "${0}") <test_name> <project> <repository> <revision> <additional_build_flags> <additional_source_directories> <shared_error_prone_flags> <patch_error_prone_flags> <validation_error_prone_flags> <validation_build_flags> [--sync] [<report_directory>]"
   exit 1
 fi
 
@@ -23,11 +23,12 @@ repository="${3}"
 revision="${4}"
 additional_build_flags="${5}"
 additional_source_directories="${6}"
-patch_error_prone_flags="${7}"
+shared_error_prone_flags="${7}"
-validation_error_prone_flags="${8}"
+patch_error_prone_flags="${8}"
-validation_build_flags="${9}"
+validation_error_prone_flags="${9}"
-do_sync="$([ "${#}" = 9 ] || [ "${10:-}" != '--sync' ] || echo 1)"
+validation_build_flags="${10}"
-report_directory="$([ "${#}" = 9 ] || ([ -z "${do_sync}" ] && echo "${10}") || ([ "${#}" = 10 ] || echo "${11}"))"
+do_sync="$([ "${#}" = 10 ] || [ "${11:-}" != '--sync' ] || echo 1)"
+report_directory="$([ "${#}" = 10 ] || ([ -z "${do_sync}" ] && echo "${11}") || ([ "${#}" = 11 ] || echo "${12}"))"
 
 if [ -n "${report_directory}" ]; then
   mkdir -p "${report_directory}"
@@ -64,9 +65,7 @@ shared_build_flags="
 
 format_goal='com.spotify.fmt:fmt-maven-plugin:2.25:format'
 
-error_prone_shared_flags='-XepExcludedPaths:(\Q${project.basedir}${file.separator}src${file.separator}\E(it|test|xdocs-examples)\Q${file.separator}resources\E|\Q${project.build.directory}${file.separator}\E).*'
+error_prone_patch_flags="${shared_error_prone_flags} -XepPatchLocation:IN_PLACE -XepPatchChecks:$(
-
-error_prone_patch_flags="${error_prone_shared_flags} -XepPatchLocation:IN_PLACE -XepPatchChecks:$(
    find "${error_prone_support_root}" \
       -path "*/META-INF/services/com.google.errorprone.bugpatterns.BugChecker" \
       -not -path "*/error-prone-experimental/*" \
@@ -76,7 +75,7 @@ error_prone_patch_flags="${error_prone_shared_flags} -XepPatchLocation:IN_PLACE
     | paste -s -d ',' -
 ) ${patch_error_prone_flags}"
 
-error_prone_validation_flags="${error_prone_shared_flags} -XepDisableAllChecks $(
+error_prone_validation_flags="${shared_error_prone_flags} -XepDisableAllChecks $(
    find "${error_prone_support_root}" \
       -path "*/META-INF/services/com.google.errorprone.bugpatterns.BugChecker" \
       -not -path "*/error-prone-experimental/*" \