bug fixes and remove separate test for per model metrics

Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>

src/main/java/com/ibm/watson/modelmesh/Metrics.java

@@ -407,14 +407,14 @@ public void logRequestMetrics(boolean external, String name, long elapsedNanos,
             int idx = shortNames ? name.indexOf('/') : -1;
             String methodName = idx == -1 ? name : name.substring(idx + 1);
             if (enablePerModelMetrics) {
-                timingHisto.labels(methodName, code.name(), mId, mId).observe(elapsedMillis);
+                timingHisto.labels(methodName, code.name(), mId).observe(elapsedMillis);
             } else {
                 timingHisto.labels(methodName, code.name()).observe(elapsedMillis);
             }
             if (reqPayloadSize != -1) {
                 if (enablePerModelMetrics) {
                     ((Histogram) metricsMap.get(REQUEST_PAYLOAD_SIZE))
-                            .labels(methodName, code.name(), mId, mId).observe(reqPayloadSize);
+                            .labels(methodName, code.name(), mId).observe(reqPayloadSize);
                 } else {
                     ((Histogram) metricsMap.get(REQUEST_PAYLOAD_SIZE))
                             .labels(methodName, code.name()).observe(reqPayloadSize);
@@ -423,7 +423,7 @@ public void logRequestMetrics(boolean external, String name, long elapsedNanos,
             if (respPayloadSize != -1) {
                 if (enablePerModelMetrics) {
                     ((Histogram) metricsMap.get(RESPONSE_PAYLOAD_SIZE))
-                            .labels(methodName, code.name(), mId, mId).observe(respPayloadSize);
+                            .labels(methodName, code.name(), mId).observe(respPayloadSize);
                 } else {
                     ((Histogram) metricsMap.get(RESPONSE_PAYLOAD_SIZE))
                             .labels(methodName, code.name()).observe(respPayloadSize);

src/test/java/com/ibm/watson/modelmesh/ModelMeshMetricsTest.java

@@ -77,7 +77,8 @@ protected int requestCount() {
 
     @Override
     protected Map<String, String> extraEnvVars() {
-        return ImmutableMap.of("MM_METRICS", "prometheus:port=" + METRICS_PORT + ";scheme=" + SCHEME);
+        return ImmutableMap.of("MM_METRICS", "prometheus:port=" + METRICS_PORT + ";scheme=" + SCHEME +
+                ";per_model_metrics=true");
     }
 
     @BeforeAll
@@ -186,23 +187,24 @@ public void verifyMetrics() throws Exception {
         // Spot check some expected metrics and values
 
         // External response time should all be < 2000ms (includes cache hit loading time)
-        assertEquals(40.0, metrics.get("modelmesh_api_request_milliseconds_bucket{method=\"predict\",code=\"OK\",le=\"2000.0\",}"));
+        assertEquals(40.0, metrics.get("modelmesh_api_request_milliseconds_bucket{method=\"predict\",code=\"OK\",modelId=\"\",le=\"2000.0\",}"));
         // External response time should all be < 200ms (includes cache hit loading time)
-        assertEquals(40.0, metrics.get("modelmesh_invoke_model_milliseconds_bucket{method=\"predict\",code=\"OK\",le=\"200.0\",}"));
+        assertEquals(40.0,
+                metrics.get("modelmesh_invoke_model_milliseconds_bucket{method=\"predict\",code=\"OK\",modelId=\"\",le=\"120000.0\",}"));
         // Simulated model sizing time is < 200ms
-        assertEquals(1.0, metrics.get("modelmesh_model_sizing_milliseconds_bucket{le=\"200.0\",}"));
+        assertEquals(1.0, metrics.get("modelmesh_model_sizing_milliseconds_bucket{modelId=\"myModel\",le=\"60000.0\",}"));
         // Simulated model sizing time is > 50ms
-        assertEquals(0.0, metrics.get("modelmesh_model_sizing_milliseconds_bucket{le=\"50.0\",}"));
+        assertEquals(0.0, metrics.get("modelmesh_model_sizing_milliseconds_bucket{modelId=\"myModel\",le=\"50.0\",}"));
         // Simulated model size is between 64MiB and 256MiB
-        assertEquals(0.0, metrics.get("modelmesh_loaded_model_size_bytes_bucket{le=\"6.7108864E7\",}"));
-        assertEquals(1.0, metrics.get("modelmesh_loaded_model_size_bytes_bucket{le=\"2.68435456E8\",}"));
+        assertEquals(0.0, metrics.get("modelmesh_loaded_model_size_bytes_bucket{modelId=\"myModel\",le=\"6.7108864E7\",}"));
+        assertEquals(1.0, metrics.get("modelmesh_loaded_model_size_bytes_bucket{modelId=\"myModel\",le=\"2.68435456E8\",}"));
         // One model is loaded
         assertEquals(1.0, metrics.get("modelmesh_instance_models_total"));
         // Histogram counts should reflect the two payload sizes (30 small, 10 large)
-        assertEquals(30.0, metrics.get("modelmesh_request_size_bytes_bucket{method=\"predict\",code=\"OK\",le=\"128.0\",}"));
-        assertEquals(40.0, metrics.get("modelmesh_request_size_bytes_bucket{method=\"predict\",code=\"OK\",le=\"2097152.0\",}"));
-        assertEquals(30.0, metrics.get("modelmesh_response_size_bytes_bucket{method=\"predict\",code=\"OK\",le=\"128.0\",}"));
-        assertEquals(40.0, metrics.get("modelmesh_response_size_bytes_bucket{method=\"predict\",code=\"OK\",le=\"2097152.0\",}"));
+        assertEquals(30.0, metrics.get("modelmesh_request_size_bytes_bucket{method=\"predict\",code=\"OK\",modelId=\"\",le=\"128.0\",}"));
+        assertEquals(40.0, metrics.get("modelmesh_request_size_bytes_bucket{method=\"predict\",code=\"OK\",modelId=\"\",le=\"2097152.0\",}"));
+        assertEquals(30.0, metrics.get("modelmesh_response_size_bytes_bucket{method=\"predict\",code=\"OK\",modelId=\"\",le=\"128.0\",}"));
+        assertEquals(40.0, metrics.get("modelmesh_response_size_bytes_bucket{method=\"predict\",code=\"OK\",modelId=\"\",le=\"2097152.0\",}"));
 
         // Memory metrics
         assertTrue(metrics.containsKey("netty_pool_mem_allocated_bytes{area=\"direct\",}"));