Implemented visualizations for the NIM metrics #3612

LinoyBitan1 · 2025-01-02T19:58:31Z

Description

JIRA - https://issues.redhat.com/browse/NVPE-31

Implemented visualizations for the NIM metrics. the graphs added to Nim tab-

1 - Graph for KV Cache usage over time
2 - Line graph for Running, Waiting and Max Request Count
3 - Line graph with Total Prompt Token Count and Total Generation Token Count
4 - Area chart with Time to First Token
5 - Area chart with Time per Output Token
6 - Donut chart with Success Request and Failed Requests
odh-model-controller creates a ConfigMap next to the InferenceService called -metrics-dashboard with a data indicating if metrics are available for the runtime and if so, the ConfigMap will also contain a JSON object specifying the queries required for the graphs mentioned above.
odh-dashboard grabs the above-mentioned ConfigMap based on its name, and if metrics are available for the runtime, it will parse the JSON object and create the required graphs.

NIM Metrics tab-

NIM Metrics graphs-

How Has This Been Tested?

Use the following doc -
NIM Metrics_ frontend.pdf

Test Impact

Cypress tests added
Unit tests of utility functions
Tests of new hooks to follow after.

Request review criteria:

Self checklist (all need to be checked):

The developer has manually tested the changes and verified that the changes work
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)

If you have UI changes:

Included any necessary screenshots or gifs if it was a UI change.
Included tags to the UX team if it was a UI/UX change.

After the PR is posted & before it merges:

The developer has tested their solution on a cluster by using the image produced by the PR to main

openshift-ci · 2025-01-02T19:58:43Z

Hi @LinoyBitan1. Thanks for your PR.

I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

andrewballantyne · 2025-01-02T20:20:32Z

FYI @emilys314

LinoyBitan1 · 2025-01-05T12:26:35Z

This is our (Matan Talvi and Linoy Bitan) first PR.

emilys314 · 2025-01-07T13:16:31Z

/ok-to-test

emilys314 · 2025-01-07T19:57:05Z

Can you run the linter / formatter? The CI checks will not pass unless it's following the linting rules See https://github.com/opendatahub-io/odh-dashboard/blob/main/CONTRIBUTING.md#linter-testing
vs code can show the errors. Or run npm run test:lint

emilys314 · 2025-01-07T20:38:48Z

Can you also add in the PR description the jira item this is associated with?

LinoyBitan1 · 2025-01-08T17:15:57Z

done

LinoyBitan1 · 2025-01-09T09:04:12Z

Can you run the linter / formatter? The CI checks will not pass unless it's following the linting rules See https://github.com/opendatahub-io/odh-dashboard/blob/main/CONTRIBUTING.md#linter-testing vs code can show the errors. Or run npm run test:lint

The linter and formatter were run, and we've added the updates in the commit along with a few changes in the tests. Let us know if there's anything else needed!

emilys314 · 2025-01-09T19:35:21Z

...src/pages/projects/screens/detail/overview/serverModels/deployedModels/DeployedModelCard.tsx

  const isKServeNIMEnabled = isProjectNIMSupported(currentProject);

-  const modelMetricsSupported =
-    modelMetricsEnabled && (modelMesh || kserveMetricsEnabled) && !isKServeNIMEnabled;
+  const modelMetricsSupported = modelMetricsEnabled && (modelMesh || kserveMetricsEnabled);


There is another linter error since isKserveNIMEnabled is no longer being used
https://github.com/opendatahub-io/odh-dashboard/actions/runs/12686647659/job/35374595034?pr=3612#step:9:48

We performed the needed changes.

codecov · 2025-01-13T16:56:16Z

Codecov Report

Attention: Patch coverage is 83.80952% with 51 lines in your changes missing coverage. Please review.

Project coverage is 85.82%. Comparing base (3ea00fd) to head (eebe460).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...end/src/api/prometheus/kservePerformanceMetrics.ts	45.16%	34 Missing ⚠️
...metrics/kserve/content/NIMCurrentRequestsGraph.tsx	80.00%	3 Missing ⚠️
...pts/metrics/kserve/useNimMetricsGraphDefinition.ts	84.21%	3 Missing ⚠️
...s/modelServing/screens/metrics/MetricsPageTabs.tsx	62.50%	3 Missing ⚠️
...etrics/kserve/content/NIMRequestsOutcomesGraph.tsx	84.61%	2 Missing ⚠️
...ontend/src/api/prometheus/NimPerformanceMetrics.ts	98.63%	1 Missing ⚠️
.../src/concepts/metrics/kserve/NimMetricsContext.tsx	96.00%	1 Missing ⚠️
...pts/metrics/kserve/content/NIMTokensCountGraph.tsx	92.30%	1 Missing ⚠️
...ts/metrics/kserve/content/NimPerformanceGraphs.tsx	95.65%	1 Missing ⚠️
frontend/src/concepts/metrics/kserve/utils.ts	75.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3612      +/-   ##
==========================================
- Coverage   85.82%   85.82%   -0.01%     
==========================================
  Files        1419     1433      +14     
  Lines       32644    32957     +313     
  Branches     9167     9222      +55     
==========================================
+ Hits        28018    28284     +266     
- Misses       4626     4673      +47

Files with missing lines	Coverage Δ
frontend/src/concepts/metrics/kserve/const.ts	`100.00% <100.00%> (ø)`
...ts/metrics/kserve/content/NIMKVCacheUsageGraph.tsx	`100.00% <100.00%> (ø)`
...rics/kserve/content/NIMTimeForFirstTokenGraphs.tsx	`100.00% <100.00%> (ø)`
...rics/kserve/content/NIMTimePerOutputTokenGraph.tsx	`100.00% <100.00%> (ø)`
...cepts/metrics/kserve/content/NimMetricsContent.tsx	`100.00% <100.00%> (ø)`
...ages/modelServing/screens/metrics/MetricsChart.tsx	`89.24% <100.00%> (+0.61%)`	⬆️
...s/modelServing/screens/metrics/nim/ModelGraphs.tsx	`100.00% <100.00%> (ø)`
...es/modelServing/screens/metrics/nim/NimMetrics.tsx	`100.00% <100.00%> (ø)`
...nd/src/pages/modelServing/screens/metrics/types.ts	`100.00% <100.00%> (ø)`
...rving/screens/metrics/useMetricsPageEnabledTabs.ts	`100.00% <100.00%> (ø)`
... and 12 more

... and 2 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ea00fd...eebe460. Read the comment docs.

emilys314

You also need to add a link to the metrics view in the model list for NIM projects, currently there is no way to get there? frontend/src/pages/modelServing/screens/global/InferenceServiceTableRow.tsx

emilys314 · 2025-01-13T18:32:15Z

frontend/src/pages/modelServing/screens/metrics/nim/ModelGraphs.tsx

Is it even possible to deploy a NIM model using modelmesh? I assume it's not so isModelMesh(model) ? <ModelMeshMetrics /> and the entire file is not needed.

emilys314 · 2025-01-13T18:32:38Z

backend/src/utils/constants.ts

@@ -69,7 +69,7 @@ export const blankDashboardCR: DashboardConfig = {
      disableServingRuntimeParams: false,
      disableConnectionTypes: false,
      disableStorageClasses: false,
-      disableNIMModelServing: true,
+      disableNIMModelServing: false,


Is this intentional? I don't see in the JIRA or PR description that we should be changing this. Can you revert this?

emilys314 · 2025-01-13T18:33:16Z

frontend/src/pages/modelServing/useModelMetricsEnabled.ts

@@ -6,8 +6,10 @@ const useModelMetricsEnabled = (): [modelMetricsEnabled: boolean] => {
  ).status;
  const biasMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.BIAS_METRICS).status;

+  const nimMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.BIAS_METRICS).status;


Suggested change

const nimMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.BIAS_METRICS).status;

const nimMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.NIM_MODEL).status;

I think you intended to check NIM_MODEL?

emilys314 · 2025-01-13T18:38:54Z

frontend/src/pages/modelServing/screens/metrics/useMetricsPageEnabledTabs.ts

  if (performanceMetricsAreaAvailable) {
    enabledTabs.push(MetricsTabKeys.PERFORMANCE);
  }
  if (biasMetricsAreaAvailable) {
    enabledTabs.push(MetricsTabKeys.BIAS);
  }
+  if (nimMetricsAreaAvailable) {


We probably shouldn't show the NIM tab for models / projects that are not NIM. If you make a standard kserve deployment it will be a blank page

We changed the disableNIMModelServing flag back to true. In order to test the tab isolation, the flag needs to be changed to false.

emilys314 · 2025-01-13T19:51:41Z

frontend/src/api/prometheus/kservePerformanceMetrics.ts

+};
+
+export const useFetchKserveKVCacheUsageData = (
+  metricsDef: KserveMetricGraphDefinition,


Suggested change

metricsDef: KserveMetricGraphDefinition,

metricsDef: KserveMetricGraphDefinition | NimMetricGraphDefinition,

I don't think we need to duplicate files (frontend/src/api/prometheus/NimPerformanceMetrics.ts and frontend/src/api/prometheus/kservePerformanceMetrics.ts) with just a different metricsDef type. Can we remove one of them and union the metricsDef type to contain both?

emilys314 · 2025-01-13T19:52:18Z

frontend/src/pages/modelServing/screens/metrics/MetricsChart.tsx

@@ -189,7 +182,8 @@ const MetricsChart: React.FC<MetricsChartProps> = ({
          {hasSomeData ? (
            <Chart
              ariaTitle={title}
-              containerComponent={containerComponent}
+              // containerComponent={containerComponent}


nitpick: remove the old commented out code

openshift-ci · 2025-01-14T14:14:30Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign manosnoam for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

frontend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

emilys314 · 2025-01-14T19:47:43Z

It appears there is another linter error https://github.com/opendatahub-io/odh-dashboard/actions/runs/12769888698/job/35594011880?pr=3612

emilys314 · 2025-01-15T16:33:46Z

frontend/src/pages/modelServing/screens/metrics/MetricsPageTabs.tsx

+  //check availability of NIM metrics
+  const nimMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.NIM_MODEL).status;


This shouldn't be needed because const isNIMAvailable = servingPlatformStatuses.kServeNIM.enabled; will already check it for you and return false if the area is not available

We think that it is needed because that just says if NIM is available or not available in the cluster but not by project. We've verified it with @olavtar .

useIsAreaAvailable(SupportedArea.NIM_MODEL).status; is only checking if it's it's available in the cluster too.

const isNIMAvailable = servingPlatformStatuses.kServeNIM.enabled; is the check for cluster wide nim enablement. And const isKServeNIMEnabled = project ? isProjectNIMSupported(project) : false is the one checking for nim project enablement.

useServingPlatformStatuses() for nim checks const { isNIMAvailable } = React.useContext(NIMAvailabilityContext); and that has

const isNIMModelServingAvailable = useIsAreaAvailable(SupportedArea.NIM_MODEL).status; const fetchNIMAvailability = React.useCallback(async () => { if (!isNIMModelServingAvailable) { return false; }

You are totally right!

emilys314 · 2025-01-15T16:34:35Z

frontend/src/pages/modelServing/screens/metrics/MetricsPageTabs.tsx

+  const servingPlatformStatuses = useServingPlatformStatuses();
+  const isNIMAvailable = servingPlatformStatuses.kServeNIM.enabled;
+  const { projects } = React.useContext(ProjectsContext);
+  const project = projects.find(byName(model.metadata.namespace)) ?? null;


Suggested change

const project = projects.find(byName(model.metadata.namespace)) ?? null;

const project = projects.find(byName(model.metadata.namespace));

small nitpick but ?? null is not needed

Signed-off-by: LinoyBitan1 <lbitan@redhat.com>

openshift-ci bot added the do-not-merge/work-in-progress This PR is in WIP state label Jan 2, 2025

openshift-ci bot added the needs-ok-to-test The openshift bot needs to label PRs from non members to avoid strain on the CI label Jan 2, 2025

LinoyBitan1 changed the title ~~Add nim metrics~~ Implemented visualizations for the NIM metrics Jan 5, 2025

LinoyBitan1 marked this pull request as ready for review January 5, 2025 12:26

openshift-ci bot removed the do-not-merge/work-in-progress This PR is in WIP state label Jan 5, 2025

openshift-ci bot requested review from lucferbux and manaswinidas January 5, 2025 12:26

openshift-ci bot added ok-to-test The openshift bot needs `ok-to-test` to allow non member PRs to run the tests. and removed needs-ok-to-test The openshift bot needs to label PRs from non members to avoid strain on the CI labels Jan 7, 2025

TomerFi mentioned this pull request Jan 7, 2025

chore: added new graph object for nim runtimes opendatahub-io/odh-model-controller#334

Merged

3 tasks

LinoyBitan1 closed this Jan 8, 2025

LinoyBitan1 reopened this Jan 8, 2025

emilys314 reviewed Jan 9, 2025

View reviewed changes

LinoyBitan1 force-pushed the add-nim-metrics branch from 522c605 to eebe460 Compare January 13, 2025 08:34

emilys314 reviewed Jan 13, 2025

View reviewed changes

LinoyBitan1 force-pushed the add-nim-metrics branch 3 times, most recently from d36865b to 859f097 Compare January 15, 2025 13:04

emilys314 reviewed Jan 15, 2025

View reviewed changes

LinoyBitan1 force-pushed the add-nim-metrics branch from 050e6c9 to d76e983 Compare January 16, 2025 17:56

LinoyBitan1 and others added 18 commits January 16, 2025 19:58

added metrics graphs for the new NIM integration

cfa0108

Signed-off-by: LinoyBitan1 <lbitan@redhat.com>

added tests for the NIM integration tests

19d7f75

Signed-off-by: LinoyBitan1 <lbitan@redhat.com>

added default value to nim flag

5522efd

apply lint fixes and additional tests

b03ff3c

fix before nim tab issue

3b63188

remove unuse line

d3a19fd

fix the nim tab broblem

a4ae45a

all but tests

5671f0b

Update mockProjectK8sResource.ts Tomer Olga PR

0ab23f7

Update modelMetrics.ts Tomer Olga PR

26560aa

Update modelMetrics.cy.ts Tomer Olga PR

661b216

Update NimTab.tsx Tomer Olga PR

055e0c4

tests updates

f931360

updated mocks for tests

1c63531

deleted comments

9ba44f2

added changes for thr PR

e15002e

change nim flag

51f8503

fixes for lint tests

bf2f0fd

LinoyBitan1 force-pushed the add-nim-metrics branch from d76e983 to bf2f0fd Compare January 16, 2025 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented visualizations for the NIM metrics #3612

Implemented visualizations for the NIM metrics #3612

LinoyBitan1 commented Jan 2, 2025 •

edited

Loading

openshift-ci bot commented Jan 2, 2025

andrewballantyne commented Jan 2, 2025

LinoyBitan1 commented Jan 5, 2025

emilys314 commented Jan 7, 2025

emilys314 commented Jan 7, 2025

emilys314 commented Jan 7, 2025

LinoyBitan1 commented Jan 8, 2025 •

edited

Loading

LinoyBitan1 commented Jan 9, 2025

emilys314 Jan 9, 2025

LinoyBitan1 Jan 13, 2025

codecov bot commented Jan 13, 2025 •

edited

Loading

emilys314 left a comment

emilys314 Jan 13, 2025

emilys314 Jan 13, 2025

emilys314 Jan 13, 2025

emilys314 Jan 13, 2025

mtalvi Jan 14, 2025 •

edited

Loading

emilys314 Jan 13, 2025

emilys314 Jan 13, 2025

openshift-ci bot commented Jan 14, 2025

emilys314 commented Jan 14, 2025

emilys314 Jan 15, 2025

mtalvi Jan 16, 2025

emilys314 Jan 16, 2025

mtalvi Jan 16, 2025

emilys314 Jan 15, 2025

	const nimMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.BIAS_METRICS).status;
	const nimMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.NIM_MODEL).status;

	metricsDef: KserveMetricGraphDefinition,
	metricsDef: KserveMetricGraphDefinition \| NimMetricGraphDefinition,

		//check availability of NIM metrics
		const nimMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.NIM_MODEL).status;

	const project = projects.find(byName(model.metadata.namespace)) ?? null;
	const project = projects.find(byName(model.metadata.namespace));

Implemented visualizations for the NIM metrics #3612

Are you sure you want to change the base?

Implemented visualizations for the NIM metrics #3612

Conversation

LinoyBitan1 commented Jan 2, 2025 • edited Loading

Description

How Has This Been Tested?

Test Impact

Request review criteria:

openshift-ci bot commented Jan 2, 2025

andrewballantyne commented Jan 2, 2025

LinoyBitan1 commented Jan 5, 2025

emilys314 commented Jan 7, 2025

emilys314 commented Jan 7, 2025

emilys314 commented Jan 7, 2025

LinoyBitan1 commented Jan 8, 2025 • edited Loading

LinoyBitan1 commented Jan 9, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 13, 2025 • edited Loading

Codecov Report

emilys314 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtalvi Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Jan 14, 2025

emilys314 commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LinoyBitan1 commented Jan 2, 2025 •

edited

Loading

LinoyBitan1 commented Jan 8, 2025 •

edited

Loading

codecov bot commented Jan 13, 2025 •

edited

Loading

mtalvi Jan 14, 2025 •

edited

Loading