[test][venice-common][router]: Add diff-coverage tests for new close paths

Cover the new close() entry points exercised by the prior commit so CI's
diffCoverage gate (45% branch threshold) is satisfied:

- RetryManagerTest.testCloseIsSafeWhenDisabled — exercises
  RetryManager.close() when no RetryManagerStats was wired in; verifies
  idempotency.
- RouterStatsTest (new) — covers RouterStats.close() for both Closeable
  and non-Closeable STAT_TYPE parameterisations, idempotency, and the
  per-request-type getter.
- TestVeniceDelegateMode.testCloseIsIdempotent — exercises
  VeniceDelegateMode.close().

Author: m-nagarajan

clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/AggVersionedDIVStats.java

@@ -50,14 +50,9 @@ public class AggVersionedDIVStats extends AbstractVeniceAggVersionedStats<DIVSta
   private final Map<VeniceMetricsDimensions, String> baseDimensionsMap;
 
   /**
-   * Per-store entry map. Each entry bundles the per-store {@link AbstractStatsCloseable#resources}
-   * with lazily-populated wrappers for the four per-store OTel metrics. A single {@code remove()} in
-   * {@link #handleStoreDeleted(String)} closes all wrappers atomically — there is no
-   * cross-map race window where a concurrent record could resurrect parallel entries.
-   *
-   * <p>Map grows lazily via {@code computeIfAbsent} and is bounded by the number of stores the
-   * server is actively ingesting. Each map is OTel-only; Tehuti recording is handled by the parent
-   * class via {@code recordVersionedAndTotalStat}.
+   * Per-store entry bundling the four per-store OTel wrappers; one {@code remove()} in
+   * {@link #handleStoreDeleted} closes them atomically. Bounded by the number of stores ingested.
+   * Tehuti recording stays on the parent via {@code recordVersionedAndTotalStat}.
    */
   private final Map<String, PerStoreEntry> perStore = new VeniceConcurrentHashMap<>();
 
@@ -100,10 +95,7 @@ private static final class PerStoreEntry extends AbstractStatsCloseable {
 
   /**
    * Per-store version info for classifying versions as CURRENT, FUTURE, or BACKUP.
-   * Updated via {@link #onVersionInfoUpdated(String, int, int)}. This map is intentionally
-   * separate from {@link #perStore}: it stores plain data (no {@link java.io.Closeable}), so it
-   * is not vulnerable to the delete-vs-record resurrection race that motivated bundling the
-   * per-store closeable wrappers into a single entry holder.
+   * Updated via {@link #onVersionInfoUpdated(String, int, int)}.
    */
   private final Map<String, OtelVersionedStatsUtils.VersionInfo> versionInfoMap = new VeniceConcurrentHashMap<>();
 

clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/AggVersionedDaVinciRecordTransformerStats.java

@@ -30,13 +30,7 @@ public class AggVersionedDaVinciRecordTransformerStats
   private final Map<VeniceMetricsDimensions, String> baseDimensionsMap;
   private final boolean emitOtelMetrics;
 
-  /**
-   * Per-store entry map. Each entry bundles the per-store {@link AbstractStatsCloseable#resources}
-   * with the latency and error-count wrappers. A single {@code remove()} in
-   * {@link #handleStoreDeleted(String)} closes both wrappers atomically — there is no
-   * cross-map race window where a concurrent record could resurrect parallel entries.
-   * Bounded by the number of stores on this host.
-   */
+  /** Per-store entry bundling latency + error-count wrappers; one {@code remove()} in {@link #handleStoreDeleted} closes them. */
   private final Map<String, PerStoreEntry> perStore = new VeniceConcurrentHashMap<>();
 
   /** Per-store state held by {@link #perStore}. */

clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/NativeMetadataRepositoryStats.java

@@ -34,16 +34,9 @@ public class NativeMetadataRepositoryStats extends AbstractVeniceStats {
   private final Map<String, Long> metadataCacheTimestampMapInMs = new VeniceConcurrentHashMap<>();
   private final Clock clock;
 
-  // OTel: per-store ASYNC_DOUBLE_GAUGE for staleness. Bounded by the number of stores currently subscribed.
-  // Per-store entries are removed and closed in {@link #handleStoreDeleted}, deregistering the callback
-  // from the SDK so it stops polling the gauge.
   private final VeniceOpenTelemetryMetricsRepository otelRepository;
   private final Map<VeniceMetricsDimensions, String> baseDimensionsMap;
-  /**
-   * Per-store entry map. Each entry bundles the per-store registry with the async-gauge wrapper
-   * (registered eagerly in the {@link PerStoreEntry} constructor), so a single {@code remove()} in
-   * {@link #handleStoreDeleted(String)} is race-free w.r.t. concurrent recordings.
-   */
+  /** Per-store ASYNC_DOUBLE_GAUGE; entries are closed in {@link #handleStoreDeleted} so the SDK stops polling. */
   private final Map<String, PerStoreEntry> perStore = new VeniceConcurrentHashMap<>();
 
   /** Per-store state held by {@link #perStore}. */
@@ -105,11 +98,7 @@ public void updateCacheTimestamp(String storeName, String clusterName, long cach
     registerOtelGaugeIfAbsent(storeName, clusterName);
   }
 
-  /**
-   * Removes the Tehuti cache-timestamp entry and closes the per-store OTel ASYNC gauge wrapper,
-   * deregistering the SDK callback so it stops polling. Called by the owning
-   * {@code NativeMetadataRepository} from its store-removal path.
-   */
+  /** Removes the Tehuti cache-timestamp entry and closes the per-store OTel async-gauge wrapper. */
   public void handleStoreDeleted(String storeName) {
     metadataCacheTimestampMapInMs.remove(storeName);
     MetricEntityStateUtils.closeQuietly(perStore.remove(storeName));

clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/ServerMetadataServiceStats.java

@@ -41,16 +41,11 @@ public class ServerMetadataServiceStats extends AbstractVeniceStats {
   private final VeniceOpenTelemetryMetricsRepository otelRepository;
   private final Map<VeniceMetricsDimensions, String> baseDimensionsMap;
   /**
-   * Per-store entry map. Entries live for the process lifetime and are drained on {@link #close()}.
-   *
-   * <p>When the requested store does not exist (e.g., {@link VeniceNoStoreException}),
-   * {@link OpenTelemetryMetricsSetup#UNKNOWN_STORE_NAME} is used as a sentinel so that all
-   * unknown-store failures share one entry. Cardinality is bounded by stores deployed to this
-   * server + the sentinel.
+   * Per-store entry map; process-lifetime, drained on {@link #close()}. Unknown stores share the
+   * {@link OpenTelemetryMetricsSetup#UNKNOWN_STORE_NAME} sentinel.
    */
   private final Map<String, PerStoreEntry> perStore = new VeniceConcurrentHashMap<>();
 
-  /** Per-store state held by {@link #perStore}. */
   private static final class PerStoreEntry extends AbstractStatsCloseable {
     final MetricEntityStateOneEnum<VeniceResponseStatusCategory> wrapper;
 

internal/venice-client-common/src/main/java/com/linkedin/venice/stats/AbstractVeniceAggStats.java

@@ -73,10 +73,7 @@ public T getTotalStats() {
     return totalStats;
   }
 
-  /**
-   * Closes {@link #totalStats} and every per-store entry in {@link #storeStats}. Subclasses with
-   * additional cleanup should override and call {@code super.close()}.
-   */
+  /** Closes {@link #totalStats} and every per-store entry in {@link #storeStats}. */
   @Override
   public void close() {
     MetricEntityStateUtils.closeQuietly(totalStats);

internal/venice-client-common/src/main/java/com/linkedin/venice/stats/AbstractVeniceStats.java

@@ -325,10 +325,7 @@ protected final MeasurableStat[] avgAndTotal() {
     return new MeasurableStat[] { new Avg(), new Total() };
   }
 
-  /**
-   * Drains every {@link Closeable} registered into {@link #resources}. Idempotent. Subclasses with
-   * additional cleanup should override and call {@code super.close()}.
-   */
+  /** Drains every {@link Closeable} registered into {@link #resources}. Idempotent. */
   @Override
   public void close() {
     resources.close();

internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityState.java

@@ -50,11 +50,7 @@ public abstract class AsyncMetricEntityState implements Closeable {
   private final Map<VeniceMetricsDimensions, String> baseDimensionsMap;
   protected final MetricEntity metricEntity;
 
-  /**
-   * The OTel SDK instrument handle. Volatile so that {@link #close()} setting it to {@code null} is
-   * promptly visible to recording threads, and so that recording threads see a consistent snapshot
-   * (read once into a local) when the cast happens.
-   */
+  /** OTel SDK instrument handle. Volatile so {@link #close()} nulling it is promptly visible to recording threads. */
   protected volatile Object otelMetric = null;
   /** The Tehuti sensor. Volatile for the same reason as {@link #otelMetric}. */
   protected volatile Sensor tehutiSensor = null;
@@ -405,33 +401,15 @@ public Object getOtelMetric() {
   }
 
   /**
-   * Releases this wrapper's OTel resources. <b>For async wrappers</b> (the OTel instrument is an
-   * {@link AutoCloseable} {@code Observable*Gauge} / {@code Observable*Counter}), this deregisters
-   * the SDK callback so it stops being polled. <b>For sync wrappers</b> ({@code LongCounter},
-   * {@code DoubleHistogram}, {@code LongUpDownCounter}, sync {@code LongGauge}) the SDK-side
-   * instrument and aggregator persist until the MeterProvider is closed; this method only releases
-   * the wrapper-side reference to the SDK instrument and the Tehuti sensor reference. Idempotent.
-   * Best-effort: SDK close exceptions are logged at WARN and swallowed so a misbehaving close
-   * cannot break shutdown.
-   *
-   * <p><b>Behaviour after close:</b> {@code record()} is a silent no-op (both Tehuti and OTel
-   * sides). For {@code ASYNC_COUNTER_FOR_HIGH_PERF_CASES}, any pending un-collected values held in
-   * the per-attribute {@code LongAdder}s are dropped. The Tehuti {@code Sensor} stays registered in
-   * the underlying {@code MetricsRepository}; only the wrapper's reference is released. For sync
-   * counters/histograms, recreating a wrapper for the same {@link MetricEntity} binds to the SDK's
-   * existing aggregator, so values continue to accumulate across close/recreate.
-   *
-   * <p><b>Caller contract:</b> not concurrent-safe with {@code record()} — callers must coordinate
-   * so that close happens after this wrapper is no longer in use. Direct subclasses
-   * (e.g., {@link AsyncMetricEntityStateBase}) may override to also clear cached attribute maps
-   * and other wrapper-side state, calling {@code super.close()} first.
+   * Releases this wrapper's OTel resources. For async wrappers this deregisters the SDK callback so
+   * it stops being polled; for sync wrappers it only releases the wrapper-side reference (the SDK
+   * aggregator persists until MeterProvider close). Idempotent and best-effort — SDK close
+   * exceptions are logged at WARN and swallowed. Post-close {@code record()} is a silent no-op.
+   * Not concurrent-safe with {@code record()}; callers must coordinate.
    */
   @Override
   public void close() {
-    // Snapshot the volatile field before the helper call so a second concurrent close() cannot
-    // observe the field non-null in instanceof and then invoke close() on a now-null reference
-    // and emit a misleading "OTel SDK close threw" WARN. Idempotency is preserved: the second
-    // close sees null here and skips the SDK call.
+    // Snapshot before the helper call so a concurrent second close() sees null here and skips.
     Object localInstrument = otelMetric;
     otelMetric = null;
     tehutiSensor = null;

internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateBase.java

@@ -77,12 +77,7 @@ private void validateBaseAttributes(
 
   // --- LongSupplier factory methods (for ASYNC_GAUGE) ---
 
-  /**
-   * Factory method for OTel-only ASYNC_GAUGE with LongSupplier callback.
-   *
-   * @param registry the {@link CompositeCloseable} that closes the returned wrapper at shutdown.
-   *                 Pass {@link CompositeCloseable#NONE} at test or ad-hoc callsites without lifecycle.
-   */
+  /** Factory method for OTel-only ASYNC_GAUGE with LongSupplier callback. */
   public static AsyncMetricEntityStateBase create(
       MetricEntity metricEntity,
       VeniceOpenTelemetryMetricsRepository otelRepository,
@@ -102,12 +97,7 @@ public static AsyncMetricEntityStateBase create(
             asyncCallback));
   }
 
-  /**
-   * Factory method for joint Tehuti+OTel ASYNC_GAUGE with LongSupplier callback.
-   *
-   * @param registry the {@link CompositeCloseable} that closes the returned wrapper at shutdown.
-   *                 Pass {@link CompositeCloseable#NONE} at test or ad-hoc callsites without lifecycle.
-   */
+  /** Factory method for joint Tehuti+OTel ASYNC_GAUGE with LongSupplier callback. */
   public static AsyncMetricEntityStateBase create(
       MetricEntity metricEntity,
       VeniceOpenTelemetryMetricsRepository otelRepository,
@@ -132,12 +122,7 @@ public static AsyncMetricEntityStateBase create(
 
   // --- DoubleSupplier factory methods (for ASYNC_DOUBLE_GAUGE) ---
 
-  /**
-   * Factory method for OTel-only ASYNC_DOUBLE_GAUGE with DoubleSupplier callback.
-   *
-   * @param registry the {@link CompositeCloseable} that closes the returned wrapper at shutdown.
-   *                 Pass {@link CompositeCloseable#NONE} at test or ad-hoc callsites without lifecycle.
-   */
+  /** Factory method for OTel-only ASYNC_DOUBLE_GAUGE with DoubleSupplier callback. */
   public static AsyncMetricEntityStateBase create(
       MetricEntity metricEntity,
       VeniceOpenTelemetryMetricsRepository otelRepository,
@@ -157,12 +142,7 @@ public static AsyncMetricEntityStateBase create(
             asyncDoubleCallback));
   }
 
-  /**
-   * Factory method for joint Tehuti+OTel ASYNC_DOUBLE_GAUGE with DoubleSupplier callback.
-   *
-   * @param registry the {@link CompositeCloseable} that closes the returned wrapper at shutdown.
-   *                 Pass {@link CompositeCloseable#NONE} at test or ad-hoc callsites without lifecycle.
-   */
+  /** Factory method for joint Tehuti+OTel ASYNC_DOUBLE_GAUGE with DoubleSupplier callback. */
   public static AsyncMetricEntityStateBase create(
       MetricEntity metricEntity,
       VeniceOpenTelemetryMetricsRepository otelRepository,

internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnum.java

@@ -76,8 +76,7 @@ private AsyncMetricEntityStateOneEnum(
    *
    * @param <S> the state type returned by {@code liveStateResolver}. Can be any reference type
    *            (wrapper, task, counter, etc.) — the infra never inspects it beyond null-check.
-   * @param registry the {@link CompositeCloseable} that closes the returned wrapper at shutdown.
-   *                 Pass {@link CompositeCloseable#NONE} at test or ad-hoc callsites without lifecycle.
+   * @param registry closes the returned wrapper at shutdown; pass {@link CompositeCloseable#NONE} for tests.
    */
   public static <E extends Enum<E> & VeniceDimensionInterface, S> AsyncMetricEntityStateOneEnum<E> create(
       MetricEntity metricEntity,
@@ -192,17 +191,9 @@ public Object getInstrument() {
   }
 
   /**
-   * Deregisters the underlying SDK observable gauge (if registered) and releases the cached
-   * per-enum {@link Attributes} so the wrapper can be GC'd. Idempotent. Best-effort: SDK close
-   * exceptions are logged at WARN and swallowed.
-   *
-   * <p><b>Caller contract:</b> closing this wrapper deregisters the callback for ALL enum values
-   * (the entire multi-emit instrument is retired). Use {@code liveStateResolver} returning
-   * {@code null} for per-combo dormancy; reserve {@code close()} for full retirement of the
-   * wrapper (e.g., process shutdown or removal of the owning per-store/per-version stats class).
-   * Not concurrent-safe with the SDK collection callback in flight; the SDK's {@code close()}
-   * deregisters the callback but does not block in-flight invocations — those use the local
-   * {@code attributesByEnum} captured at registration time, so the field-nulling here is safe.
+   * Deregisters the underlying SDK observable gauge and releases the cached per-enum {@link Attributes}.
+   * Closes the callback for ALL enum values — use {@code liveStateResolver} returning {@code null}
+   * for per-combo dormancy. Idempotent and best-effort.
    */
   @Override
   public void close() {

internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnums.java

@@ -76,8 +76,7 @@ private AsyncMetricEntityStateTwoEnums(
    *
    * @param <S> the state type returned by {@code liveStateResolver}. Any reference type — the
    *            infra never inspects it beyond null-check.
-   * @param registry the {@link CompositeCloseable} that closes the returned wrapper at shutdown.
-   *                 Pass {@link CompositeCloseable#NONE} at test or ad-hoc callsites without lifecycle.
+   * @param registry closes the returned wrapper at shutdown; pass {@link CompositeCloseable#NONE} for tests.
    */
   public static <E1 extends Enum<E1> & VeniceDimensionInterface, E2 extends Enum<E2> & VeniceDimensionInterface, S> AsyncMetricEntityStateTwoEnums<E1, E2> create(
       MetricEntity metricEntity,
@@ -203,20 +202,12 @@ public Object getInstrument() {
   }
 
   /**
-   * Deregisters the underlying SDK observable gauge (if registered) and releases the cached
-   * per-pair {@link Attributes} so the wrapper can be GC'd. Idempotent. Best-effort: SDK close
-   * exceptions are logged at WARN and swallowed.
-   *
-   * <p><b>Caller contract:</b> closing this wrapper deregisters the callback for ALL enum-pair
-   * combinations. Use {@code liveStateResolver} returning {@code null} for per-combo dormancy;
-   * reserve {@code close()} for full retirement of the wrapper.
+   * Deregisters the underlying SDK observable gauge and releases the cached per-pair {@link Attributes}.
+   * Closes the callback for ALL enum-pair combinations. Idempotent and best-effort.
    */
   @Override
   public void close() {
-    // Snapshot the volatile field before the helper call so a second concurrent close() cannot
-    // observe the field non-null in instanceof and then invoke close() on a now-null reference
-    // and emit a misleading "OTel SDK close threw" WARN. Idempotency is preserved: the second
-    // close sees null here and skips the SDK call.
+    // Snapshot before the helper call so a concurrent second close() sees null here and skips.
     Object localInstrument = instrument;
     instrument = null;
     attributesByEnum = null;