review fix

openvinotoolkit · Dec 23, 2024 · 8174bf7 · 8174bf7
1 parent 8493488
commit 8174bf7
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 43 deletions.
diff --git a/src/metric_config.cpp b/src/metric_config.cpp
@@ -59,7 +59,7 @@ const std::string METRIC_NAME_REQUESTS_ACCEPTED = "ovms_requests_accepted";
 const std::string METRIC_NAME_REQUESTS_REJECTED = "ovms_requests_rejected";
 
 const std::string METRIC_NAME_GRAPH_ERROR = "ovms_graph_error";
-const std::string METRIC_NAME_PROCESSING_TIME = "ovms_processing_time";
+const std::string METRIC_NAME_PROCESSING_TIME = "ovms_graph_processing_time_us";
 
 bool MetricConfig::validateEndpointPath(const std::string& endpoint) {
     std::regex valid_endpoint_regex("^/[a-zA-Z0-9]*$");

diff --git a/src/model_metric_reporter.cpp b/src/model_metric_reporter.cpp
@@ -564,38 +564,23 @@ MediapipeServableMetricReporter::MediapipeServableMetricReporter(const MetricCon
 
         // KFS
         this->processingTimeGrpcModelInfer = family->addMetric({{"name", graphName},
-                                                                   {"api", "KServe"},
-                                                                   {"method", "ModelInfer"},
-                                                                   {"interface", "gRPC"}},
+                                                                   {"method", "ModelInfer"}},
             this->buckets);
         THROW_IF_NULL(this->processingTimeGrpcModelInfer, "cannot create metric");
 
         this->processingTimeGrpcModelInferStream = family->addMetric({{"name", graphName},
-                                                                         {"api", "KServe"},
-                                                                         {"method", "ModelInferStream"},
-                                                                         {"interface", "gRPC"}},
+                                                                         {"method", "ModelInferStream"}},
             this->buckets);
         THROW_IF_NULL(this->processingTimeGrpcModelInfer, "cannot create metric");
 
-        this->processingTimeRestModelInfer = family->addMetric({{"name", graphName},
-                                                                   {"api", "KServe"},
-                                                                   {"method", "ModelInfer"},
-                                                                   {"interface", "REST"}},
-            this->buckets);
-        THROW_IF_NULL(this->processingTimeRestModelInfer, "cannot create metric");
-
         // V3
         this->processingTimeRestV3Unary = family->addMetric({{"name", graphName},
-                                                                {"api", "V3"},
-                                                                {"method", "Unary"},
-                                                                {"interface", "REST"}},
+                                                                {"method", "Unary"}},
             this->buckets);
         THROW_IF_NULL(this->processingTimeRestV3Unary, "cannot create metric");
 
         this->processingTimeRestV3Stream = family->addMetric({{"name", graphName},
-                                                                 {"api", "V3"},
-                                                                 {"method", "Stream"},
-                                                                 {"interface", "REST"}},
+                                                                 {"method", "Stream"}},
             this->buckets);
         THROW_IF_NULL(this->processingTimeRestV3Stream, "cannot create metric");
     }

diff --git a/src/model_metric_reporter.hpp b/src/model_metric_reporter.hpp
@@ -215,23 +215,14 @@ class MediapipeServableMetricReporter {
     std::unique_ptr<MetricHistogram> processingTimeRestV3Stream;
 
     inline MetricHistogram* getProcessingTimeMetric(const ExecutionContext& context) {
-        if (context.interface == ExecutionContext::Interface::GRPC) {
-            if (context.method == ExecutionContext::Method::ModelInfer)
-                return this->processingTimeGrpcModelInfer.get();
-            if (context.method == ExecutionContext::Method::ModelInferStream)
-                return this->processingTimeGrpcModelInferStream.get();
-            return nullptr;
-        } else if (context.interface == ExecutionContext::Interface::REST) {
-            if (context.method == ExecutionContext::Method::ModelInfer)
-                return this->processingTimeRestModelInfer.get();
-            if (context.method == ExecutionContext::Method::V3Unary)
-                return this->processingTimeRestV3Unary.get();
-            if (context.method == ExecutionContext::Method::V3Stream)
-                return this->processingTimeRestV3Stream.get();
-            return nullptr;
-        } else {
-            return nullptr;
-        }
+        if (context.method == ExecutionContext::Method::ModelInfer)
+            return this->processingTimeGrpcModelInfer.get();
+        if (context.method == ExecutionContext::Method::ModelInferStream)
+            return this->processingTimeGrpcModelInferStream.get();
+        if (context.method == ExecutionContext::Method::V3Unary)
+            return this->processingTimeRestV3Unary.get();
+        if (context.method == ExecutionContext::Method::V3Stream)
+            return this->processingTimeRestV3Stream.get();
         return nullptr;
     }
 

diff --git a/src/test/metrics_flow_test.cpp b/src/test/metrics_flow_test.cpp
@@ -396,8 +396,7 @@ TEST_F(MetricFlowTest, GrpcModelInfer) {
 
     checkMediapipeRequestsCounter(server.collect(), METRIC_NAME_RESPONSES, mpName, "gRPC", "ModelInfer", "KServe", numberOfAcceptedRequests);
 
-    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{api=\"KServe\",interface=\"gRPC\",method=\"ModelInfer\",name=\""} + mpName + std::string{"\"} "} + std::to_string(numberOfAcceptedRequests)));
-    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{api=\"KServe\",interface=\"REST\",method=\"ModelInfer\",name=\""} + mpName + std::string{"\"} "} + std::to_string(0)));
+    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{method=\"ModelInfer\",name=\""} + mpName + std::string{"\"} "} + std::to_string(numberOfAcceptedRequests)));
 #endif
 
     EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_REQUEST_TIME + std::string{"_count{interface=\"gRPC\",name=\""} + modelName + std::string{"\",version=\"1\"} "} + std::to_string(numberOfSuccessRequests)));
@@ -712,8 +711,7 @@ TEST_F(MetricFlowTest, RestModelInfer) {
 
     checkMediapipeRequestsCounter(server.collect(), METRIC_NAME_RESPONSES, mpName, "REST", "ModelInfer", "KServe", numberOfAcceptedRequests);
 
-    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{api=\"KServe\",interface=\"gRPC\",method=\"ModelInfer\",name=\""} + mpName + std::string{"\"} "} + std::to_string(0)));
-    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{api=\"KServe\",interface=\"REST\",method=\"ModelInfer\",name=\""} + mpName + std::string{"\"} "} + std::to_string(numberOfAcceptedRequests)));
+    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{method=\"ModelInfer\",name=\""} + mpName + std::string{"\"} "} + std::to_string(numberOfAcceptedRequests)));
 #endif
 
     EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_REQUEST_TIME + std::string{"_count{interface=\"gRPC\",name=\""} + modelName + std::string{"\",version=\"1\"} "} + std::to_string(0)));
@@ -835,7 +833,7 @@ TEST_F(MetricFlowTest, RestV3Unary) {
     // checkMediapipeRequestsCounter(server.collect(), METRIC_NAME_REQUESTS_REJECTED, "dummy_gpt", "REST", "Unary", "V3", numberOfRejectedRequests);
     checkMediapipeRequestsCounter(server.collect(), METRIC_NAME_RESPONSES, "dummy_gpt", "REST", "Unary", "V3", numberOfAcceptedRequests * 2);
 
-    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{api=\"V3\",interface=\"REST\",method=\"Unary\",name=\""} + "dummy_gpt" + std::string{"\"} "} + std::to_string(numberOfAcceptedRequests * 2)));
+    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{method=\"Unary\",name=\""} + "dummy_gpt" + std::string{"\"} "} + std::to_string(numberOfAcceptedRequests * 2)));
 }
 #endif
 
@@ -889,7 +887,7 @@ TEST_F(MetricFlowTest, RestV3Stream) {
     const int numberOfMockedChunksPerRequest = 9;  // Defined in openai_chat_completions_mock_calculator.cpp
     checkMediapipeRequestsCounter(server.collect(), METRIC_NAME_RESPONSES, "dummy_gpt", "REST", "Stream", "V3", numberOfAcceptedRequests * numberOfMockedChunksPerRequest * 2);
 
-    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{api=\"V3\",interface=\"REST\",method=\"Stream\",name=\""} + "dummy_gpt" + std::string{"\"} "} + std::to_string(numberOfAcceptedRequests * 2)));
+    EXPECT_THAT(server.collect(), HasSubstr(METRIC_NAME_PROCESSING_TIME + std::string{"_count{method=\"Stream\",name=\""} + "dummy_gpt" + std::string{"\"} "} + std::to_string(numberOfAcceptedRequests * 2)));
 
     SPDLOG_ERROR(server.collect());
 }