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

Add API status to the new dashboard #2551

Merged
merged 1 commit into from
Dec 7, 2021

Conversation

Kevan-Y
Copy link
Contributor

@Kevan-Y Kevan-Y commented Nov 30, 2021

Issue This PR Addresses

Fixes #2418

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

  • Add API route to get all health information
  • Update dashboard to display the status information
  • Add function to fetch and display to dashboard
rec.mp4

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@Kevan-Y Kevan-Y requested a review from humphd November 30, 2021 23:01
@gitpod-io
Copy link

gitpod-io bot commented Nov 30, 2021

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

On GitPod I get:

Screen Shot 2021-11-30 at 7 40 56 PM

It's doing:

Screen Shot 2021-11-30 at 7 42 49 PM

The problem is in:

export default function getAllServicesStatus() {
  fetch('/status')

This may be a problem specific to GitPod's URL/port layout, but I suspect that this isn't quite right yet.

@humphd humphd requested review from manekenpix and HyperTHD December 1, 2021 00:46
@Kevan-Y
Copy link
Contributor Author

Kevan-Y commented Dec 1, 2021

On GitPod I get:

Screen Shot 2021-11-30 at 7 40 56 PM

It's doing:

Screen Shot 2021-11-30 at 7 42 49 PM

The problem is in:

export default function getAllServicesStatus() {
  fetch('/status')

This may be a problem specific to GitPod's URL/port layout, but I suspect that this isn't quite right yet.

Notice in GitPods is not running the server which allows the API to fetch all status to work.
In src/api/status/src/server.js

service.router.get('/status', (req, res) => {
 check()
   .then((status) => {
     // This status response shouldn't be cached (we need current status info)
     res.set('Cache-Control', 'no-store, max-age=0');
     return res.json(status);
   })
   .catch((err) => res.status(500).json({ error: err.message }));
});

Could it also be this issue?

@humphd
Copy link
Contributor

humphd commented Dec 1, 2021

How is the status page showing up if it's not running? It gets hosted by that server.

@humphd
Copy link
Contributor

humphd commented Dec 1, 2021

cc @DukeManh for gitpod wisdom

Copy link
Contributor

@DukeManh DukeManh left a comment

Choose a reason for hiding this comment

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

It works on Gitpod when I run npm run dev from src/api/status.

I think the variable naming can be better

src/api/status/public/js/serviceStatus.js Outdated Show resolved Hide resolved
src/api/status/public/js/serviceStatus.js Outdated Show resolved Hide resolved
HyperTHD
HyperTHD previously approved these changes Dec 1, 2021
src/api/status/src/server.js Show resolved Hide resolved
@HyperTHD HyperTHD self-requested a review December 1, 2021 02:03
@HyperTHD
Copy link
Contributor

HyperTHD commented Dec 1, 2021

Ignore the approval, misclick. Need to fix what I mentioned above

Andrewnt219
Andrewnt219 previously approved these changes Dec 4, 2021
Copy link
Contributor

@Andrewnt219 Andrewnt219 left a comment

Choose a reason for hiding this comment

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

Nice enhancement, bonus for being responsive as well 🙌.

@humphd
Copy link
Contributor

humphd commented Dec 4, 2021

@Kevan-Y can you rebase to pick up the change to src/api/status/public/index.html?

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

I love this!

Screen Shot 2021-12-04 at 2 46 41 PM

Can we only show the services for the current deployment (i.e., staging or production, but not both at once)? I think if you are looking at the status service on staging, we should show you staging only; same for prod.

@Kevan-Y
Copy link
Contributor Author

Kevan-Y commented Dec 5, 2021

I love this!

Screen Shot 2021-12-04 at 2 46 41 PM

Can we only show the services for the current deployment (i.e., staging or production, but not both at once)? I think if you are looking at the status service on staging, we should show you staging only; same for prod.

That possible, I just looked at the env.local what API_HOST for?
Can we use this to check if it's production URL, or staging URL?

@humphd
Copy link
Contributor

humphd commented Dec 5, 2021

You should be able to use the URL the page is loaded on to figure it out, i.e., https://dev.api.telescope.cdot.systems/v1/status (staging) vs https://api.telescope.cdot.systems/v1/status (production). We can't access env vars in the dashboard.

@Kevan-Y
Copy link
Contributor Author

Kevan-Y commented Dec 6, 2021

You should be able to use the URL the page is loaded on to figure it out, i.e., https://dev.api.telescope.cdot.systems/v1/status (staging) vs https://api.telescope.cdot.systems/v1/status (production). We can't access env vars in the dashboard.

Using URL will work but not on Gitpot since they are using another URL.

@humphd
Copy link
Contributor

humphd commented Dec 6, 2021

When we fix #2553 we can do this server side.

@Kevan-Y
Copy link
Contributor Author

Kevan-Y commented Dec 6, 2021

Ok sure, I will leave it like that for now and when working on #2553 I will do it to show only staging/production

@humphd
Copy link
Contributor

humphd commented Dec 6, 2021

This is failing prettier

@HyperTHD
Copy link
Contributor

HyperTHD commented Dec 7, 2021

How can I test this PR to make sure it works? The services seem to not be able to appear despite running. Tried on both gitpod and locally by pulling this PR @Kevan-Y

@Kevan-Y
Copy link
Contributor Author

Kevan-Y commented Dec 7, 2021

@HyperTHD , to run it, you will have to go to src\api\status and do a npm run dev

@humphd humphd requested a review from Andrewnt219 December 7, 2021 16:56
Copy link
Contributor

@Andrewnt219 Andrewnt219 left a comment

Choose a reason for hiding this comment

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

Well done👍.

@humphd humphd merged commit 0d65fd0 into Seneca-CDOT:master Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite the Microservices API status check in the new status dashboard
6 participants