cleanup and new test

Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>

Author: jaydeluca

prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java

@@ -60,7 +60,6 @@ default MetricSnapshot collect(
    * This is called in two places:
    *
    * <ol>
-   *   <li>During registration to check if a metric with that name already exists.
    *   <li>During scrape to check if this collector can be skipped because a name filter is present
    *       and the metric name is excluded.
    * </ol>

prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java

@@ -55,7 +55,6 @@ default MetricSnapshots collect(
    * This is called in two places:
    *
    * <ol>
-   *   <li>During registration to check if a metric with that name already exists.
    *   <li>During scrape to check if the collector can be skipped because a name filter is present
    *       and all names are excluded.
    * </ol>

prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java

@@ -1,14 +1,13 @@
 package io.prometheus.metrics.model.registry;
 
 import static java.util.Arrays.asList;
-import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import io.prometheus.metrics.model.snapshots.CounterSnapshot;
 import io.prometheus.metrics.model.snapshots.GaugeSnapshot;
-import io.prometheus.metrics.model.snapshots.HistogramSnapshot;
 import io.prometheus.metrics.model.snapshots.Labels;
 import io.prometheus.metrics.model.snapshots.MetricSnapshot;
 import io.prometheus.metrics.model.snapshots.MetricSnapshots;
@@ -538,4 +537,173 @@ public String getPrometheusName() {
     assertThat(registry.scrape().size()).isEqualTo(0);
   }
 
+  @Test
+  void testPartialUnregister_scrapeStillWorks() {
+    PrometheusRegistry registry = new PrometheusRegistry();
+
+    Collector counter1 =
+        new Collector() {
+          @Override
+          public MetricSnapshot collect() {
+            return CounterSnapshot.builder()
+                .name("api_requests")
+                .dataPoint(
+                    CounterSnapshot.CounterDataPointSnapshot.builder()
+                        .labels(Labels.of("endpoint", "/api/v1"))
+                        .value(100)
+                        .build())
+                .build();
+          }
+
+          @Override
+          public String getPrometheusName() {
+            return "api_requests_total";
+          }
+        };
+
+    Collector counter2 =
+        new Collector() {
+          @Override
+          public MetricSnapshot collect() {
+            return CounterSnapshot.builder()
+                .name("api_requests")
+                .dataPoint(
+                    CounterSnapshot.CounterDataPointSnapshot.builder()
+                        .labels(Labels.of("endpoint", "/api/v2"))
+                        .value(200)
+                        .build())
+                .build();
+          }
+
+          @Override
+          public String getPrometheusName() {
+            return "api_requests_total";
+          }
+        };
+
+    Collector counter3 =
+        new Collector() {
+          @Override
+          public MetricSnapshot collect() {
+            return CounterSnapshot.builder()
+                .name("api_requests")
+                .dataPoint(
+                    CounterSnapshot.CounterDataPointSnapshot.builder()
+                        .labels(Labels.of("endpoint", "/api/v3"))
+                        .value(300)
+                        .build())
+                .build();
+          }
+
+          @Override
+          public String getPrometheusName() {
+            return "api_requests_total";
+          }
+        };
+
+    registry.register(counter1);
+    registry.register(counter2);
+    registry.register(counter3);
+    assertThat(registry.scrape().size()).isEqualTo(3);
+
+    registry.unregister(counter2);
+
+    MetricSnapshots snapshots = registry.scrape();
+    assertThat(snapshots.size()).isEqualTo(2);
+
+    int totalDataPoints = snapshots.stream().mapToInt(s -> s.getDataPoints().size()).sum();
+    assertThat(totalDataPoints).isEqualTo(2);
+  }
+
+  @Test
+  void testCollectorAndMultiCollectorNameOverlap_sameType() {
+    PrometheusRegistry registry = new PrometheusRegistry();
+
+    Collector singleCounter =
+        new Collector() {
+          @Override
+          public MetricSnapshot collect() {
+            return CounterSnapshot.builder()
+                .name("shared_metric")
+                .dataPoint(
+                    CounterSnapshot.CounterDataPointSnapshot.builder()
+                        .labels(Labels.of("source", "collector"))
+                        .value(100)
+                        .build())
+                .build();
+          }
+
+          @Override
+          public String getPrometheusName() {
+            return "shared_metric_total";
+          }
+        };
+
+    MultiCollector multi =
+        new MultiCollector() {
+          @Override
+          public MetricSnapshots collect() {
+            return new MetricSnapshots(
+                CounterSnapshot.builder()
+                    .name("shared_metric")
+                    .dataPoint(
+                        CounterSnapshot.CounterDataPointSnapshot.builder()
+                            .labels(Labels.of("source", "multicollector"))
+                            .value(200)
+                            .build())
+                    .build(),
+                GaugeSnapshot.builder().name("other_metric").build());
+          }
+
+          @Override
+          public List<String> getPrometheusNames() {
+            return asList("shared_metric_total", "other_metric");
+          }
+        };
+
+    registry.register(singleCounter);
+    registry.register(multi);
+
+    MetricSnapshots snapshots = registry.scrape();
+    assertThat(snapshots.size()).isEqualTo(3);
+  }
+
+  @Test
+  void testCollectorAndMultiCollectorNameOverlap_differentType() {
+    PrometheusRegistry registry = new PrometheusRegistry();
+
+    Collector counter =
+        new Collector() {
+          @Override
+          public MetricSnapshot collect() {
+            return CounterSnapshot.builder().name("conflict").build();
+          }
+
+          @Override
+          public String getPrometheusName() {
+            return "conflict_metric";
+          }
+        };
+
+    MultiCollector multi =
+        new MultiCollector() {
+          @Override
+          public MetricSnapshots collect() {
+            return new MetricSnapshots(GaugeSnapshot.builder().name("conflict").build());
+          }
+
+          @Override
+          public List<String> getPrometheusNames() {
+            return singletonList("conflict_metric");
+          }
+        };
+
+    registry.register(counter);
+    registry.register(multi);
+
+    assertThatThrownBy(registry::scrape)
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("Conflicting metric types")
+        .hasMessageContaining("conflict");
+  }
 }