Refactor/metric save as ppl #1151

pjfitzgibbons · 2023-10-23T17:46:35Z

Description

Modify Metric Export to produce PPL based Metric query on Prometheus datasource.
Example query : source = prom_connector.prometheus_http_request_total | ...

Issues Resolved

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Fix countDistribution access error when countDistribution not available. Signed-off-by: Peter Fitzgibbons <[email protected]>

Signed-off-by: Peter Fitzgibbons <[email protected]>

codecov · 2023-10-23T18:08:14Z

Codecov Report

Merging #1151 (f40d4f0) into main (ddb697a) will decrease coverage by 0.20%.
Report is 6 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1151      +/-   ##
==========================================
- Coverage   44.21%   44.02%   -0.20%     
==========================================
  Files         329      329              
  Lines       19542    19554      +12     
  Branches     4719     4719              
==========================================
- Hits         8640     8608      -32     
- Misses      10335    10378      +43     
- Partials      567      568       +1

Flag	Coverage Δ
dashboards-observability	`44.02% <ø> (-0.20%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 10 files with indirect coverage changes

pjfitzgibbons · 2023-10-23T20:07:37Z

common/types/metrics.ts

  x: number;
  y: number;
  w: number;
  h: number;
-  metricType: 'savedCustomMetric' | 'prometheusMetric';
+  catalog: string;


This logic around choosing if a record is "customMetric" or "prometheus" is simplified to just knowinng what catalog it is from.

pjfitzgibbons · 2023-10-23T20:09:38Z

public/components/custom_panels/helpers/utils.tsx

-
-// Creates a catalogVisualization for a runtime catalog based PPL query and runs getQueryResponse
-export const renderCatalogVisualization = async ({
+export const renderSavedVisualization = async ({


Git diff split this one wierdly.
renderSavedVisualization is retained, diff is below.
renderCatalogVisualization, updateCatalogVisualizationQuery, createCatalogVisualizationMetaData are removed entirely, as they are specific to PromQL query processing, which is no longer needed.

why is it no longer needed?

pjfitzgibbons · 2023-10-23T20:11:30Z

public/components/custom_panels/helpers/utils.tsx

 }: {
  http: CoreStart['http'];
  pplService: PPLService;
-  catalogSource: string;
+  savedVisualizationId: string;
+  inputMetaData: SavedVisualization;


In a certain case (metric grid) the VisualizationConainer, will be passed the meta-data directly (inputMetaData), instead of having a saved record to render. It is passed here for processing.

pjfitzgibbons · 2023-10-23T20:13:03Z

...c/components/custom_panels/panel_modules/visualization_container/visualization_container.tsx

-        setIsLoading,
-        setIsError
-      );
+    await renderSavedVisualization({


Argument list converted to object to avoid variable-placement errors in long argument list.

pjfitzgibbons · 2023-10-23T20:13:41Z

public/components/custom_panels/redux/panel_slice.ts

@@ -84,8 +84,6 @@ const normalizedPanel = (panel: CustomPanelType): CustomPanelType => ({

 export const selectPanelList = (rootState): CustomPanelType[] => rootState.customPanel.panelList;

-const {setToast} = useToast();


linter says "useToast" can't be called at top-level.
Integrated into each method using it.

pjfitzgibbons · 2023-10-23T20:15:01Z

public/components/custom_panels/redux/panel_slice.ts

-export const addMultipleVizToPanels = (panels, vizIds) => async (dispatch, getState) => {
-  forEach(panels, (oldPanel) => {
-    const panel = getState().customPanel.panelList.find((p) => p.id === oldPanel.panel.id);
+export const addMultipleVizToPanels = async (panels, vizIds) => {


This method re-written to capture the actual logic used in VisualizationFlyout

pjfitzgibbons · 2023-10-23T20:16:11Z

public/components/custom_panels/redux/panel_slice.ts

 };

-export const replaceVizInPanel = (oldPanel, oldVizId, vizId, newVisualizationTitle) => async (dispatch, getState) => {
+export const replaceVizInPanel = (oldPanel, oldVizId, vizId, newVisualizationTitle) => async (


Just some auto-formatting. Nothing to see here.

public/components/event_analytics/explorer/explorer.tsx

pjfitzgibbons · 2023-10-23T20:19:11Z

public/components/metrics/top_menu/top_menu.tsx

+    };
+  };
+
+  const updateSavedVisualization = async (metricLayout: MetricType, name: string): string => {


This method written to overcome the incongruent API of OSDSavedVisualizationClient, which returns the raw saved-object on get() and expects a modified UI-ready object on update()

Signed-off-by: Peter Fitzgibbons <[email protected]>

kavithacm · 2023-10-24T20:45:08Z

public/components/metrics/redux/slices/metrics_slice.ts

-const updateLayoutBySelection = (state: any, newMetric: any) => {
-  const newDimensions = getNewVizDimensions(state.metricsLayout);
+const metricQueryFromId = (id: string) => {
+  return `source = ${id} | stats avg(@value) by span(@timestamp, 1d)`;


What's the purpose of this function?

kavithacm

can you resolve the merge conflicts ?

…) (opensearch-project#1168) Signed-off-by: Yan Zeng <[email protected]> Co-authored-by: Chang Liu <[email protected]> (cherry picked from commit 6236fe2a34f06fd2c104f4b7f86cfce4e55d761d) Co-authored-by: Yan Zeng <[email protected]>

pjfitzgibbons added 3 commits October 20, 2023 15:40

Quiet react-dnd draggableId/droppableId warnings.

cc84d25

Fix countDistribution access error when countDistribution not available. Signed-off-by: Peter Fitzgibbons <[email protected]>

Refactor Metric save-as-ppl

9fc7e41

Signed-off-by: Peter Fitzgibbons <[email protected]>

Fix Metric-as-Visualization update from MetricExport

37a963b

Signed-off-by: Peter Fitzgibbons <[email protected]>

pjfitzgibbons force-pushed the refactor/metric-save-as-ppl branch from a6f8435 to 37a963b Compare October 23, 2023 17:48

pjfitzgibbons commented Oct 23, 2023

View reviewed changes

pjfitzgibbons marked this pull request as ready for review October 23, 2023 20:49

pjfitzgibbons requested review from ps48, kavithacm, derek-ho, joshuali925, dai-chen, YANG-DB, rupal-bq, mengweieric, vmmusings, Swiddis, penghuo, seankao-az and anirudha as code owners October 23, 2023 20:49

Cleanup logging

f40d4f0

Signed-off-by: Peter Fitzgibbons <[email protected]>

kavithacm reviewed Oct 24, 2023

View reviewed changes

pjfitzgibbons closed this Nov 9, 2023

pjfitzgibbons deleted the refactor/metric-save-as-ppl branch November 9, 2023 23:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/metric save as ppl #1151

Refactor/metric save as ppl #1151

pjfitzgibbons commented Oct 23, 2023

codecov bot commented Oct 23, 2023 •

edited

Loading

pjfitzgibbons Oct 23, 2023

pjfitzgibbons Oct 23, 2023

kavithacm Oct 24, 2023

pjfitzgibbons Oct 23, 2023

pjfitzgibbons Oct 23, 2023

pjfitzgibbons Oct 23, 2023

pjfitzgibbons Oct 23, 2023

pjfitzgibbons Oct 23, 2023

pjfitzgibbons Oct 23, 2023

kavithacm Oct 24, 2023

kavithacm left a comment

		@@ -84,8 +84,6 @@ const normalizedPanel = (panel: CustomPanelType): CustomPanelType => ({

		export const selectPanelList = (rootState): CustomPanelType[] => rootState.customPanel.panelList;

		const {setToast} = useToast();

Refactor/metric save as ppl #1151

Refactor/metric save as ppl #1151

Conversation

pjfitzgibbons commented Oct 23, 2023

Description

Issues Resolved

Check List

codecov bot commented Oct 23, 2023 • edited Loading

Codecov Report

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

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

kavithacm left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 23, 2023 •

edited

Loading