-
-
Notifications
You must be signed in to change notification settings - Fork 201
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
Chronos npm microservices refactor #365
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the search bar in the dashboard component was removed from occupied component and given its own folder with associated styles
For easier readability and DX the developer icons sections was extracted into its own component and associated style sheet
Done to facilitate readablity and DX Styles also moved to ApplicationsCard component folder
Modularization
styles block at the top of occupied component was moved into a helper function. Tried to only switch the different styles (background-color, box shadow and another background color) Ran into trouble becuase those styles depend on the mode state which is defined in a context which cant be used outside a function component. Ended up just moving both styling functions whole
The modal component was being rendered multiple times which was unecessary. Additionally each modal component had a different state that was only being used to determine if that modal should be open or not. These states were condensed to one state to simplify the modal states
this commit represents the changes to all the individual modals that are now rendered from a single modal state. Done to clarify which modals are open and prevent unnecessary states
Modularization
ports originally set by default to 5000 on certain mac OS port 5000 reserved for air drop. Prevented app from starting on that server
The system information coming from si was strangley not being passed to through the function. The function was being called syncronously and because all the si function are async the return at the end of the function was always passing an empty array. This is because although the data from si was coming in, it was being pushed into an empty array defined at the top of the file inside of a .then and becuase the return was executing synchrounously the array was never being updated. Added awaits to each of the si functions and now the data is moving again
refactoring the mongo.health function The data from health metrics was not being passed out and there is now an error with duplicate mongo entries
Stephenfeature
The filter on the health data array now checks for null values. This resulted in an error since we were inputting multiple nulls into the same db.
Prior to this change new metrics were added based on the an arbitrary length variable that was changed after the awaiting a find call to the mongo db. Current metrics is now queried directly. Two layers of awaits are still necessary when calling the find on the metrics db due to the queueing of all the await calls by the different services. Am looking in to making it more streamm line
Moved the metrics model find function inside the health metrics function so that the data from the metrics model is populated before the length check between the current metrics and the health metrics
…ibed comments from prior groups
Current metrics is only needed within this function to compare the lengths between current metrics and health metrics. Instead of grabbging current metrics outside of the function it is now grabbing the current metrics data before the length check.
Upon load of the graphscontainer component an event call is made to electron (health request) which calls the mongo fetch function which grabs all the health data stored inside of the individual services databases. Inside of the mongoFetch function there is a call for the grafana api key database. In the microservices portion of the app grafana is not used so there is no key. This was causing an error and preventing data flow. This portion is being commented out temporarily so that more specific controls can be made to differentiate between apps that use docker (and thus grafana) and microservices
Stephenfeature
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Types of changes
Purpose
Approach
Dug into the local chronos npm package code and found that inside of the track function a number of helper function were being called asyncrounously
Inside of the the mongo.health function there was another function being called (collectHealthData) that was collecting system information health data.
The problem was that each of these functions (that were being called as methods on the systeminformation library) were all async. The thenables of these functions were pushing data inside an empty array declared at the top of the parent function.
At the end of the parent function this array was being filtered and then return synchrounsly.
Because the parent function was not async and the system information functions were not being awaited, the top level array meant to collect the health information was not being given a chance to actually populate.
I changed the parent function to an async function and awaited each of the systeminformation method calls which fixed the issue and successfully returned the data
Additonally I refactored code inside of the npm package and was able to remove a few layers of nested async calls which has significantly improved the performance of the app.
Lastly when the graphs container is rendered a 'health request' event is sent to electron that grabs the data stored from the npm package portion of the code.
A function called mongoFetch then gets called which is responsible for aggregating all the data from each of the services in the database into useable data for the frontend.
I found a side effect in this function that was causing the last blockage of data for microservices.
There is a part of the mongoFetch function that grabs an api key from grafana that is stored in env files in the DOCKER side of app. In microservices grafana is not used so that data comes back as undefined.
There is currently no check in that function to differentiate between docker and microservices so when the graphs container makes the call to electron an error would occur because that function expects that grafana key to populated as an object with properties.
I commented out the portion of this code relating to grafana temporarily and will add logic block to run code if the app is containerized or not.
Screenshot(s)
References issue #. Please review this @teammember1, @teamMember2.