This is an automated email from the ASF dual-hosted git repository.
damccorm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git
The following commit(s) were added to refs/heads/master by this push:
new cdaa5ff456d [Java] Remove MS_EXPOSE_REP from the global SpotBugs
filter (#39951)
cdaa5ff456d is described below
commit cdaa5ff456dfc0b3550ca92eef7a564d45e5844d
Author: Maksym Tymoshyk <[email protected]>
AuthorDate: Thu Sep 3 20:58:28 2026 +0300
[Java] Remove MS_EXPOSE_REP from the global SpotBugs filter (#39951)
The global filter turns off MS_EXPOSE_REP for every module. That hides 34
findings today, and hides any new one automatically.
Remove that line and mark the existing sites one by one with
@SuppressFBWarnings, each carrying the reason it is exempt. Nothing changes
at runtime. What changes is that the next accessor someone writes that
returns a mutable static will fail spotbugsMain instead of passing quietly.
The three Guava sites cannot be configured away. SpotBugs checks its list of
known immutable types by fully qualified name, so it does not recognise
Guava relocated into org.apache.beam.vendor.guava, and the fallback scan
finds setter-named methods on ImmutableCollection. See
https://github.com/spotbugs/spotbugs/issues/1601.
Addresses #35312.
---
.../apache/beam/runners/spark/metrics/MetricsAccumulator.java | 5 +++++
.../spark/structuredstreaming/metrics/MetricsAccumulator.java | 5 +++++
.../java/build-tools/src/main/resources/beam/spotbugs-filter.xml | 1 -
.../core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java | 9 +++++++++
.../src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java | 9 +++++++++
.../java/org/apache/beam/sdk/util/construction/ModelCoders.java | 7 +++++++
.../apache/beam/sdk/util/construction/PTransformTranslation.java | 7 +++++++
.../main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java | 5 +++++
.../main/java/org/apache/beam/sdk/testutils/NamedTestResult.java | 7 +++++++
.../src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java | 6 ++++++
10 files changed, 60 insertions(+), 1 deletion(-)
diff --git
a/runners/spark/src/main/java/org/apache/beam/runners/spark/metrics/MetricsAccumulator.java
b/runners/spark/src/main/java/org/apache/beam/runners/spark/metrics/MetricsAccumulator.java
index dbdfe11a585..612d71b1aea 100644
---
a/runners/spark/src/main/java/org/apache/beam/runners/spark/metrics/MetricsAccumulator.java
+++
b/runners/spark/src/main/java/org/apache/beam/runners/spark/metrics/MetricsAccumulator.java
@@ -17,6 +17,7 @@
*/
package org.apache.beam.runners.spark.metrics;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.IOException;
import org.apache.beam.runners.core.metrics.MetricsContainerStepMap;
import org.apache.beam.runners.spark.SparkPipelineOptions;
@@ -82,6 +83,10 @@ public class MetricsAccumulator {
}
}
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "Spark merges only the accumulator instance the driver registered. A
copy would collect metrics that nothing reports.")
public static MetricsContainerStepMapAccumulator getInstance() {
if (instance == null) {
throw new IllegalStateException("Metrics accumulator has not been
instantiated");
diff --git
a/runners/spark/src/main/java/org/apache/beam/runners/spark/structuredstreaming/metrics/MetricsAccumulator.java
b/runners/spark/src/main/java/org/apache/beam/runners/spark/structuredstreaming/metrics/MetricsAccumulator.java
index 63407b9f14d..e8cbd895082 100644
---
a/runners/spark/src/main/java/org/apache/beam/runners/spark/structuredstreaming/metrics/MetricsAccumulator.java
+++
b/runners/spark/src/main/java/org/apache/beam/runners/spark/structuredstreaming/metrics/MetricsAccumulator.java
@@ -17,6 +17,7 @@
*/
package org.apache.beam.runners.spark.structuredstreaming.metrics;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.apache.beam.runners.core.metrics.MetricsContainerStepMap;
import
org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.annotations.VisibleForTesting;
import org.apache.spark.sql.SparkSession;
@@ -85,6 +86,10 @@ public class MetricsAccumulator
* Get the {@link MetricsAccumulator} on this driver. If there's no such
accumulator yet, it will
* be created and registered using the provided {@link SparkSession}.
*/
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "Spark merges only the accumulator instance the driver registered. A
copy would collect metrics that nothing reports.")
public static MetricsAccumulator getInstance(SparkSession session) {
MetricsAccumulator current = instance;
if (current != null) {
diff --git a/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml
b/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml
index 4393ec6a624..5f6f368228e 100644
--- a/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml
+++ b/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml
@@ -57,7 +57,6 @@
<!-- TODO(https://github.com/apache/beam/issues/35312) resolve findings-->
<Bug pattern="CT_CONSTRUCTOR_THROW"/>
- <Bug pattern="MS_EXPOSE_REP"/>
<!--
Many test classes are captured by lambdas and marked `implements
Serializable`. They are not
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java
index f845d8fde94..681f022c904 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java
@@ -19,6 +19,7 @@ package org.apache.beam.sdk.metrics;
import static
org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkNotNull;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
@@ -122,6 +123,10 @@ public class Lineage {
}
/** {@link Lineage} representing sources and optionally side inputs. */
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "Every reporter writes into the same metric cell, so all callers
need the one shared instance. A copy would drop the lineage it records.")
public static Lineage getSources() {
Lineage localSources = sources;
if (localSources == null) {
@@ -131,6 +136,10 @@ public class Lineage {
}
/** {@link Lineage} representing sinks. */
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "Every reporter writes into the same metric cell, so all callers
need the one shared instance. A copy would drop the lineage it records.")
public static Lineage getSinks() {
Lineage localSinks = sinks;
if (localSinks == null) {
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java
index e9677626f88..5d033d4a91a 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java
@@ -17,6 +17,7 @@
*/
package org.apache.beam.sdk.metrics;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Joiner;
/** Standard {@link org.apache.beam.sdk.io.Source} Metrics. */
@@ -69,6 +70,10 @@ public class SourceMetrics {
}
/** Gauge for source backlog in bytes. */
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "A Gauge is a handle onto one metric cell, not a value. A copy would
send the reading nowhere.")
public static Gauge backlogBytes() {
return BACKLOG_BYTES_GAUGE;
}
@@ -84,6 +89,10 @@ public class SourceMetrics {
}
/** Gauge for source backlog in elements. */
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "A Gauge is a handle onto one metric cell, not a value. A copy would
send the reading nowhere.")
public static Gauge backlogElements() {
return BACKLOG_ELEMENTS_GAUGE;
}
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoders.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoders.java
index 5059cc1c6b8..f625e376abc 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoders.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoders.java
@@ -22,6 +22,7 @@ import static
org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Pr
import static
org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkState;
import com.google.auto.value.AutoValue;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.Set;
import org.apache.beam.model.pipeline.v1.RunnerApi.Coder;
import org.apache.beam.model.pipeline.v1.RunnerApi.FunctionSpec;
@@ -97,6 +98,12 @@ public class ModelCoders {
SHARDED_KEY_CODER_URN,
NULLABLE_CODER_URN);
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "Returns a Guava ImmutableSet."
+ + " Spotbugs matches its known-immutable list by fully qualified
name, so it cannot recognise collections relocated into
org.apache.beam.vendor.guava."
+ + " See https://github.com/spotbugs/spotbugs/issues/1601.";)
public static Set<String> urns() {
return MODEL_CODER_URNS;
}
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/PTransformTranslation.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/PTransformTranslation.java
index 5f8782265f1..88bfb8b5b53 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/PTransformTranslation.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/PTransformTranslation.java
@@ -22,6 +22,7 @@ import static
org.apache.beam.model.pipeline.v1.ExternalTransforms.ExpansionMeth
import static org.apache.beam.sdk.util.construction.BeamUrns.getUrn;
import static
org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkState;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
@@ -417,6 +418,12 @@ public class PTransformTranslation {
knownPayloadTranslators;
@Internal
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "Returns a Guava ImmutableMap."
+ + " Spotbugs matches its known-immutable list by fully qualified
name, so it cannot recognise collections relocated into
org.apache.beam.vendor.guava."
+ + " See https://github.com/spotbugs/spotbugs/issues/1601.";)
public static Map<Class<? extends PTransform>, TransformPayloadTranslator>
getKnownPayloadTranslators() {
if (knownPayloadTranslators == null) {
diff --git
a/sdks/java/io/thrift/src/main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java
b/sdks/java/io/thrift/src/main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java
index e4e698faffa..b24f132a001 100644
---
a/sdks/java/io/thrift/src/main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java
+++
b/sdks/java/io/thrift/src/main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java
@@ -19,6 +19,7 @@ package org.apache.beam.sdk.io.thrift;
import static java.util.Collections.unmodifiableMap;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
@@ -121,6 +122,10 @@ public final class ThriftSchema extends
GetterBasedSchemaProviderV2 {
*
* @see #custom() for how to manually pass the beam type for container
typedefs
*/
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "The default provider holds an empty typedef map that is never
mutated. Callers needing typedefs go through custom(), which builds a separate
instance.")
public static @NonNull SchemaProvider provider() {
return defaultProvider;
}
diff --git
a/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/NamedTestResult.java
b/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/NamedTestResult.java
index a4f40598d5a..112c29b30ee 100644
---
a/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/NamedTestResult.java
+++
b/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/NamedTestResult.java
@@ -18,6 +18,7 @@
package org.apache.beam.sdk.testutils;
import com.google.cloud.bigquery.LegacySQLTypeName;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.Map;
import org.apache.beam.sdk.testutils.publishing.InfluxDBPublisher;
import
org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.ImmutableMap;
@@ -83,6 +84,12 @@ public class NamedTestResult implements TestResult {
.build();
}
+ @SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "Returns a Guava ImmutableMap."
+ + " Spotbugs matches its known-immutable list by fully qualified
name, so it cannot recognise collections relocated into
org.apache.beam.vendor.guava."
+ + " See https://github.com/spotbugs/spotbugs/issues/1601.";)
public static Map<String, String> getSchema() {
return schema;
}
diff --git
a/sdks/java/testing/tpcds/src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java
b/sdks/java/testing/tpcds/src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java
index a22bab1c89e..763e8164dee 100644
---
a/sdks/java/testing/tpcds/src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java
+++
b/sdks/java/testing/tpcds/src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java
@@ -17,12 +17,18 @@
*/
package org.apache.beam.sdk.tpcds;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.apache.beam.sdk.schemas.Schema;
import
org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.ImmutableMap;
+@SuppressFBWarnings(
+ value = "MS_EXPOSE_REP",
+ justification =
+ "Fixed TPC-DS table definitions, built once at class load."
+ + " Schema is mutable only through setUUID, which no caller here
uses, and copying twenty-four schemas on every accessor call would cost real
time in a benchmark harness.")
public class TpcdsSchemas {
/**
* Get all tpcds table schemas automatically by reading json files. In this
case all field will be
