Dependency Plugin Additional Options #200

Dhruv-J · 2023-03-28T16:14:57Z

This PR allows for users to edit the panel and choose to
group pods by labels or not. The user can also change the
color of the boxes to red, yellow, green, or blue.

Fixes issue #164

codecov · 2023-03-28T16:22:36Z

Codecov Report

Merging #200 (558967c) into main (5b1fb15) will increase coverage by 0.36%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #200      +/-   ##
==========================================
+ Coverage   69.76%   70.12%   +0.36%     
==========================================
  Files          32       34       +2     
  Lines        3545     3776     +231     
  Branches        0       43      +43     
==========================================
+ Hits         2473     2648     +175     
- Misses        924      973      +49     
- Partials      148      155       +7

Flag	Coverage Δ
unit-tests	`69.81% <ø> (+0.05%)`	⬆️

see 4 files with indirect coverage changes

yuntanghsu · 2023-04-06T21:03:48Z

build/charts/theia/README.md

@@ -52,7 +52,7 @@ Kubernetes: `>= 1.16.0-0`
 | grafana.enable | bool | `true` | Determine whether to install Grafana. It is used as a data visualization and monitoring tool.   |
 | grafana.homeDashboard | string | `"homepage.json"` | Default home dashboard. |
 | grafana.image | object | `{"pullPolicy":"IfNotPresent","repository":"projects.registry.vmware.com/antrea/theia-grafana","tag":"8.3.3"}` | Container image used by Grafana. |
-| grafana.installPlugins | list | `["https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-sankey-plugin-1.0.2.zip;theia-grafana-sankey-plugin","https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-chord-plugin-1.0.1.zip;theia-grafana-chord-plugin","https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-dependency-plugin-1.0.0.zip;theia-grafana-dependency-plugin","grafana-clickhouse-datasource 1.0.1"]` | Grafana plugins to install. |
+| grafana.installPlugins | list | `["https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-sankey-plugin-1.0.2.zip;theia-grafana-sankey-plugin","https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-chord-plugin-1.0.1.zip;theia-grafana-chord-plugin","https://github.com/Dhruv-J/grafana-dependency-plugin/archive/refs/tags/pre3.zip;theia-grafana-dependency-plugin","grafana-clickhouse-datasource 1.0.1"]` | Grafana plugins to install. |


Just curious, why we change the path here?

That's just the link I use for the test release, so if anyone wants to run my PR branch, they can without any additional setup. I change it back before the final merge occurs.

heanlan

Would you mind adding either more explanation in the PR description or your feature design in the related issue? Including example screenshots of the new feature, for both the pod label toggle and the color choice.

It is not very clear to me

how does they look like in Grafana
how we expect the pod label toggle to work? why we only look for "k8s"/"k8s-app" labels?

I thought we had the discussion before on the previous PR: #142 (comment)

heanlan · 2023-04-17T18:00:49Z

plugins/grafana-custom-plugins/grafana-dependency-plugin/src/DependencyPanel.tsx

+      }
+      console.log('attempting to log JSON: '+labelJSON);
+      let labels = JSON.parse(labelJSON);
+      if(labels['app'] !== undefined) {


little confused, why we only look for "app" and "k8s-app" labels?

In order to group by labels, I added support for the two basic ones I saw that had values for most pods. Should I make it check against a user-defined label?

the two basic ones I saw that had values for most pods

to clarify, where does the observation come from? which are the "most pods"? like I mentioned in the previous comment, is "app.kubernetes.io/name" less common than those two per your observation?

My point is, I'm not sure about which particular labels should we look for, so I doubt whether it's a good way to design this feature. I would like to take one step back to the motivation and design. For example, I see there are some common labels listed in k8s documentation: https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/ , but I'm not convinced that we should look for these labels.

Could you also upload the screenshot of the diagram?

Does it mean user can only specify one label each time? Could we make use of the ad-hoc filter, showing all the available labels, and letting users to select one of more labels they would like to group by? I think a drop-down list is better than a text box, as
a. users may not know which labels they have right now in the cluster, if we don't give them the options
b. they can select multiple labels at once.

Although I'm not sure if it is doable. If you would like to do a design review/have a design doc, we can discuss it more in details.

I'll update with a screenshot of the diagram as well, but I don't think that an ad-hoc filter would be feasible for a couple reasons.

If pods are grouped by label, their pod label is used per square instead of the pod name itself. If multiple pod labels were used at once, I don't think there would be a good way of grouping them, especially if there are conflicts among labels. For example two pods could both be labelled by the same value for one label, but different values for another.

Ad-hoc filter can only check the database columns, anything more specific would require query variables, which cannot be included in the query if the query is to pass end to end testing. Dropdown menu is a great idea, I don't think it could be populated without a query variable. The options data structure that's passed to the Panel contains the editing options for the panel itself. That means that the options for the panel can't access the data the panel accesses in order to populate its members.

For the first one, I don't have a strong opinion on whether we should enable single label or multiple labels selection. If single label selection would make the graph representation easier, we can go with it.

For example two pods could both be labelled by the same value for one label, but different values for another.

Although I didn't get how does the above situation harm.

For the second one, we can remove this query from E2E test, if necessary. I think it worth exploring on query variables, ad-hoc filters, and maybe other Grafana configurations to see if it will make the dropdown menu work. I concern more on whether we can parse the Pod label string and split them into individual key value pairs.

Yeah, splitting the label is more challenging than getting the rest of the query variable to work, since distinct values of only the key needs to be selected. There isn't a query that can parse the JSON and format output, so it may not be possible to include a dropdown menu. I hope that users wishing to group by label will already have some idea for which labels they wish to group by.

heanlan

Could you please also update network-flow-visibility.md to add the introduction of the new options?

heanlan · 2023-04-28T17:02:38Z

plugins/grafana-custom-plugins/grafana-dependency-plugin/README.md

+### 3. Customize the Panel Options
+
+Users can customize the panel by choosing to group the diagram based a chosen
+label. It is also possible to change the color of the pod squares in the


label -> Pod labels
pod -> Pod

heanlan · 2023-04-28T17:06:56Z

plugins/grafana-custom-plugins/grafana-dependency-plugin/src/DependencyPanel.tsx

+      return sourcePodName;
+    }
+
+    // TODO add button or switch to set groupByLabelVariable


little confused on the TODO comment here. What is it for? I thought we already have a switch in the panel options?

I meant to delete that earlier

heanlan · 2023-04-28T17:09:53Z

plugins/grafana-custom-plugins/grafana-dependency-plugin/src/module.ts

-export const plugin = new PanelPlugin<DependencyOptions>(DependencyPanel).setPanelOptions((builder) => builder);
+export const plugin = new PanelPlugin<DependencyOptions>(DependencyPanel).setPanelOptions((builder) => builder.addBooleanSwitch({
+    path: 'groupByLabel',
+    name: 'Group By Label',


I think it would be better to change to Group by Pod Label

heanlan · 2023-04-28T17:11:12Z

plugins/grafana-custom-plugins/grafana-dependency-plugin/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog

+## 1.0.2 - 04-xx-2023
+
+Added label view toggle and color choice.


Added "Group by Pod Labels" toggle and Pod square color choices.

Dhruv-J · 2023-04-28T17:13:27Z

Could you please also update network-flow-visibility.md to add the introduction of the new options?

I left it off because it was part of the dependency plugin instead, so I added it to the README for that. I can also add it to flow-visibility.md if it would fit there.

heanlan · 2023-04-28T17:15:50Z

I left it off because it was part of the dependency plugin instead, so I added it to the README for that. I can also add it to flow-visibility.md if it would fit there.

Eventually it is a user option, right? Normally Theia users would only read network-flow-visibility.md but not plugin README. I think it would be good to let users know they have these options. Plugin README is more like a documentation for developers who would like to use or build on top of the plugin.

heanlan · 2023-05-01T21:14:54Z

docs/network-flow-visibility.md

@@ -764,6 +764,12 @@ the selected time range.

 <img src="https://downloads.antrea.io/static/04132023/flow-visibility-network-topology-0.png" width="400" alt="Network Topology Dashboard service dependency graph">

+Users can group Pods by label, allowing Pod squares in the diagram to represent
+a set of pods with the same label value. It is also possible to choose the


Suggested change

a set of pods with the same label value. It is also possible to choose the

a set of Pods with the same label value. It is also possible to choose the

heanlan · 2023-05-01T21:16:15Z

docs/network-flow-visibility.md

@@ -764,6 +764,12 @@ the selected time range.

 <img src="https://downloads.antrea.io/static/04132023/flow-visibility-network-topology-0.png" width="400" alt="Network Topology Dashboard service dependency graph">

+Users can group Pods by label, allowing Pod squares in the diagram to represent


maybe add some instructions to let users know where should they find the configurations, telling users it is in panel edit

heanlan · 2023-05-02T17:27:08Z

build/charts/theia/README.md

@@ -52,7 +52,7 @@ Kubernetes: `>= 1.16.0-0`
 | grafana.enable | bool | `true` | Determine whether to install Grafana. It is used as a data visualization and monitoring tool.   |
 | grafana.homeDashboard | string | `"homepage.json"` | Default home dashboard. |
 | grafana.image | object | `{"pullPolicy":"IfNotPresent","repository":"projects.registry.vmware.com/antrea/theia-grafana","tag":"8.3.3"}` | Container image used by Grafana. |
-| grafana.installPlugins | list | `["https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-sankey-plugin-1.0.2.zip;theia-grafana-sankey-plugin","https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-chord-plugin-1.0.1.zip;theia-grafana-chord-plugin","https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-dependency-plugin-1.0.1.zip;theia-grafana-dependency-plugin","grafana-clickhouse-datasource 1.0.1"]` | Grafana plugins to install. |
+| grafana.installPlugins | list | `["https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-sankey-plugin-1.0.2.zip;theia-grafana-sankey-plugin","https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-chord-plugin-1.0.1.zip;theia-grafana-chord-plugin","https://downloads.antrea.io/artifacts/grafana-custom-plugins/theia-grafana-chord-plugin-1.0.2.zip;theia-grafana-dependency-plugin","grafana-clickhouse-datasource 1.0.1"]` | Grafana plugins to install. |


chord -> dependency

heanlan · 2023-05-02T17:30:27Z

plugins/grafana-custom-plugins/grafana-dependency-plugin/README.md

- field 5: destinationServicePortName value with name or an alias of `destinationServicePortName`
- field 6: octetDeltaCount value with name or an alias of `octetDeltaCount`
+- field 2: sourcePodLabels value with name or alias of `sourcePodLabels`
+- field 3: sourcePodNamespace value with name or alias of `sourcePodNamespace`


why we add sourcePodNamespace here? Is it being used anywhere in the source code?

This PR allows for users to edit the panel and choose to group pods by labels or not. The user can also change the color of the boxes to red, yellow, green, or blue. Fixes issue antrea-io#164 Signed-off-by: Dhruv-J <dhruvj@vmware.com>

Dhruv-J · 2023-05-02T23:20:57Z

/theia-test-e2e

heanlan

LGTM

yuntanghsu reviewed Apr 6, 2023

View reviewed changes

Dhruv-J force-pushed the dep-plugin-toggle branch from 970a510 to 40bcf48 Compare April 17, 2023 17:25

Dhruv-J changed the title ~~Dependency Graph Label Toggle~~ Dependency Plugin Additional Options Apr 17, 2023

Dhruv-J requested a review from heanlan April 17, 2023 17:30

heanlan reviewed Apr 17, 2023

View reviewed changes

heanlan reviewed Apr 28, 2023

View reviewed changes

heanlan reviewed May 1, 2023

View reviewed changes

Dhruv-J force-pushed the dep-plugin-toggle branch from 9e05340 to a791d5c Compare May 2, 2023 16:49

heanlan reviewed May 2, 2023

View reviewed changes

heanlan requested a review from yuntanghsu May 2, 2023 17:32

Dependency Plugin Options

558967c

This PR allows for users to edit the panel and choose to group pods by labels or not. The user can also change the color of the boxes to red, yellow, green, or blue. Fixes issue antrea-io#164 Signed-off-by: Dhruv-J <dhruvj@vmware.com>

Dhruv-J force-pushed the dep-plugin-toggle branch from a791d5c to 558967c Compare May 2, 2023 18:48

heanlan approved these changes May 2, 2023

View reviewed changes

Dhruv-J merged commit e074ca0 into antrea-io:main May 3, 2023

elton-furtado added this to the Theia v0.6 release milestone May 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dependency Plugin Additional Options #200

Dependency Plugin Additional Options #200

Dhruv-J commented Mar 28, 2023 •

edited

Loading

codecov bot commented Mar 28, 2023 •

edited

Loading

yuntanghsu Apr 6, 2023

Dhruv-J Apr 13, 2023

heanlan left a comment •

edited

Loading

heanlan Apr 17, 2023

Dhruv-J Apr 17, 2023

heanlan Apr 17, 2023

Dhruv-J Apr 19, 2023

heanlan Apr 19, 2023 •

edited

Loading

Dhruv-J Apr 20, 2023 •

edited

Loading

heanlan Apr 25, 2023

Dhruv-J Apr 26, 2023 •

edited

Loading

heanlan left a comment

heanlan Apr 28, 2023

heanlan Apr 28, 2023

Dhruv-J Apr 28, 2023

heanlan Apr 28, 2023

heanlan Apr 28, 2023

Dhruv-J commented Apr 28, 2023

heanlan commented Apr 28, 2023 •

edited

Loading

heanlan May 1, 2023

heanlan May 1, 2023

heanlan May 2, 2023

heanlan May 2, 2023

Dhruv-J commented May 2, 2023

heanlan left a comment

	a set of pods with the same label value. It is also possible to choose the
	a set of Pods with the same label value. It is also possible to choose the

		@@ -764,6 +764,12 @@ the selected time range.

		<img src="https://downloads.antrea.io/static/04132023/flow-visibility-network-topology-0.png" width="400" alt="Network Topology Dashboard service dependency graph">

		Users can group Pods by label, allowing Pod squares in the diagram to represent

Dependency Plugin Additional Options #200

Dependency Plugin Additional Options #200

Conversation

Dhruv-J commented Mar 28, 2023 • edited Loading

codecov bot commented Mar 28, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heanlan left a comment • edited Loading

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

heanlan Apr 19, 2023 • edited Loading

Choose a reason for hiding this comment

Dhruv-J Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dhruv-J Apr 26, 2023 • edited Loading

Choose a reason for hiding this comment

heanlan 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

Choose a reason for hiding this comment

Dhruv-J commented Apr 28, 2023

heanlan commented Apr 28, 2023 • edited Loading

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

Dhruv-J commented May 2, 2023

heanlan left a comment

Choose a reason for hiding this comment

Dhruv-J commented Mar 28, 2023 •

edited

Loading

codecov bot commented Mar 28, 2023 •

edited

Loading

heanlan left a comment •

edited

Loading

heanlan Apr 19, 2023 •

edited

Loading

Dhruv-J Apr 20, 2023 •

edited

Loading

Dhruv-J Apr 26, 2023 •

edited

Loading

heanlan commented Apr 28, 2023 •

edited

Loading