Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: linking dashboard part-1 #2931

Merged
merged 11 commits into from
Jun 4, 2024
Merged

feat: linking dashboard part-1 #2931

merged 11 commits into from
Jun 4, 2024

Conversation

Hardikl
Copy link
Contributor

@Hardikl Hardikl commented May 28, 2024

No description provided.

@cla-bot cla-bot bot added the cla-signed label May 28, 2024
@Hardikl Hardikl linked an issue May 28, 2024 that may be closed by this pull request
Harvest should support linking between dashboards #2647
Closed
4 tasks
@Hardikl Hardikl marked this pull request as draft May 29, 2024 11:23
@Hardikl
Copy link
Contributor Author

Hardikl commented May 29, 2024

Volume dashboard changes are done.
Testing import/export workflow and other uid related cases.

@Hardikl Hardikl changed the title feat: linking dashboard volume, aggregate, svm part-1 feat: linking dashboard part-1 May 30, 2024
@Hardikl Hardikl marked this pull request as ready for review May 30, 2024 10:14
@Hardikl
Copy link
Contributor Author

Hardikl commented May 30, 2024

Changes covered so far:

  1. UID would be non-empty and human readable form for all dashboards.
  2. Folder name would be generic and don't have release/date in it, same behaviour as NABox.
  3. No need to have overwrite flag as we always overwrite with same uid (will remove this arg later)
  4. Volume dashboard all tables have links
  5. Grafan Import, then export to local folder and then importing this local folder works fine.
  6. Docker workflow testing is in-progress.

@Hardikl
Copy link
Contributor Author

Hardikl commented May 30, 2024

Tested in .127 system for docker workflow and it's working fine with new changes.

cgrinds
cgrinds reviewed May 31, 2024
View reviewed changes
Copy link
Collaborator

@cgrinds cgrinds left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Grafana has a flat namespace and folders don't really exist, perhaps we should simplify the uids like so:

harvest-cdot-aggregate             => cdot-aggregate
harvest-cdot-details-volume-by-svm => cdot-volume-by-svm
harvest-storagegrid-overview       => storagegrid-overview

cmd/tools/grafana/dashboard_test.go Outdated Show resolved Hide resolved
cmd/tools/grafana/grafana.go Outdated
m[filepath.Join(opts.dir, "cmode", "details")] = &Folder{name: "Harvest-" + harvestRelease + "-cDOT Details"}
m[filepath.Join(opts.dir, "7mode")] = &Folder{name: "Harvest-" + harvestRelease + "-7mode"}
m[filepath.Join(opts.dir, "storagegrid")] = &Folder{name: "Harvest-" + harvestRelease + "-StorageGrid"}
m[filepath.Join(opts.dir, "cmode")] = &Folder{name: "Harvest-main-cDOT"}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're ditching versioned folders maybe Harvest-cDOT, Harvest-StorageGrid, etc. would be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I was just matching the behaviour with what our docker workflow currently doing.

cmd/tools/grafana/grafana.go Outdated Show resolved Hide resolved
cmd/tools/grafana/grafana.go Outdated Show resolved Hide resolved
@Hardikl
Copy link
Contributor Author

Hardikl commented Jun 3, 2024

Since Grafana has a flat namespace and folders don't really exist, perhaps we should simplify the uids like so:

harvest-cdot-aggregate             => cdot-aggregate
harvest-cdot-details-volume-by-svm => cdot-volume-by-svm
harvest-storagegrid-overview       => storagegrid-overview

Added the naming convention this way.

Hardikl and others added 3 commits June 3, 2024 15:41
rahulguptajss
rahulguptajss previously approved these changes Jun 3, 2024
View reviewed changes
cgrinds
cgrinds previously approved these changes Jun 4, 2024
View reviewed changes
@Hardikl Hardikl dismissed stale reviews from cgrinds and rahulguptajss via ddb658d June 4, 2024 12:13
@rahulguptajss rahulguptajss merged commit 3b920f6 into main Jun 4, 2024
6 checks passed
@rahulguptajss rahulguptajss deleted the hl_link branch June 4, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgrinds cgrinds cgrinds approved these changes

@rahulguptajss rahulguptajss rahulguptajss left review comments

Assignees
No one assigned
Labels
cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Harvest should support linking between dashboards
3 participants
@Hardikl @cgrinds @rahulguptajss