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.

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

New app monitor dashboard to display prometheus metrics of Codewind projects #977

Closed
rwalle61 opened this issue Nov 5, 2019 · 14 comments
Closed
Assignees

Comments

@rwalle61
Copy link
Contributor

@rwalle61 rwalle61 commented Nov 5, 2019

Codewind version: Latest
OS: Mac

Description of the enhancement:

Appmetrics-dash and javametrics-dash are cool, but users don't want to have to include that code in their projects. It'd be useful to be able to see these metrics dashboard without the user having to change anything about their Codewind projects.

Proposed solution:

It'd be good if the IDEs' Open App Monitor button would point to a metrics dashboard on an endpoint not hosted by the project itself, but by Codewind, such as http://127.0.0.1:34000/performance/codewind-metrics/dashboard/java?theme=dark&projectID=34e06980-fca0-11e9-b7e4-0558a951d35a.

@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Nov 5, 2019

@mattcolegate and myself are working on this, with others:

@jgwest jgwest added the area/portal label Nov 5, 2019
@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Nov 5, 2019

(FYI it will require code changes to the VSCode and Eclipse IDEs, but Matt and I will do the work/ lead the requirements)

@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Nov 7, 2019

with @tobespc we have decided to merge the appmetrics-prometheus and PFE changes for 0.6.0, then merge change to the plugins later (probably 0.7.0), when the new PFE dashboard supports Swift as well

@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Dec 6, 2019

So the PFE changes are in: we can inject metrics into nodejs, liberty and spring projects.

The remaining thing is to have the open app monitor button on the IDEs (VSCode, Eclipse and Theia) pointing to:

  • ${appOrigin}/appmetrics-dash if the user has not turned on auto-metrics-injection, and their app contains appmetrics, else
  • ${pfeOrigin}/performance/monitor/dashboard/${projectLanguage}?theme=dark&projectID=${projectID}. (If the user has not turned on auto-metrics-injection, and their app does not contain appmetrics, then they will see our dashboard unpopulated.

Whereas I have previously opened PRs in the IDE repos to manage this logic in the IDEs, I think it it makes more sense to have this logic in PFE instead. This allows the IDEs, as frontends, to simply consume information from PFE, where everything is worked out.

More specifically, PFE will calculate the app monitor dashboard url, and provide that information on the GET api/v1/projects/{id} endpoint. The IDEs will be able to simply ask PFE what the appMonitorUrl is for the project.

@mattcolegate and I are working on that now

@rwalle61 rwalle61 changed the title New performance metrics dashboard to show metrics of Codewind projects New app monitor dashboard to display prometheus metrics of Codewind projects Dec 6, 2019
@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Dec 9, 2019

/assign

@jgwest

This comment has been minimized.

Copy link
Member

@jgwest jgwest commented Dec 9, 2019

@rwalle61: Unable to locate pipeline with name: inProgress

@jgwest

This comment has been minimized.

Copy link
Member

@jgwest jgwest commented Dec 9, 2019

@rwalle61: Unable to locate pipeline with name: InProgress

@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Dec 9, 2019

/pipeline In Progress

@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Dec 9, 2019

@eharris369 @tetchel we have nearly finished the work mentioned above: to calculate the app monitor dashboard url in PFE and provide that information on the GET api/v1/projects/{id} endpoint.

So after we merge that PR, the IDEs will be able to simply ask PFE what the appMonitorUrl is for the project. I've raised a PR for the VSCode plugin to illustrate this - please review (but don't merge until that PR is merged and I have checked that it works for Swift, hopefully all happening tomorrow/wednesday).

@jopit

This comment has been minimized.

Copy link

@jopit jopit commented Dec 9, 2019

Is this going into 0.7.0?

@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Dec 10, 2019

@jopit yea. Should land Wednesday

@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Dec 11, 2019

Change of plan because it turns out that:

  1. PFE, when running locally, doesn't seem to know its origin (e.g. http://localhost:10000), but the IDE does. This is needed because the appMonitorUrl can point to the dashboard hosted at PFE.
  2. PFE sends the IDE project data via both GET projects and the onProjectStatusChange socket message. This makes it more complicated for PFE to keep the project.appMonitorUrl up to date between restarts of PFE (because the port may change).

Therefore we have decided to keep the logic in the IDEs. We will extend the existing getAppMonitorUrl function to provide the correct link for the circumstance.

@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Dec 12, 2019

done - manually tested on master and 0.7.0 branch

@rwalle61 rwalle61 closed this Dec 12, 2019
@rwalle61

This comment has been minimized.

Copy link
Contributor Author

@rwalle61 rwalle61 commented Dec 17, 2019

related issue #1509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.