forked from open-source-labs/Chronos
-
-
Notifications
You must be signed in to change notification settings - Fork 0
Chronos npm #25
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
Merged
Merged
Chronos npm #25
Conversation
This file contains hidden or 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
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
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
sarhiri
approved these changes
May 11, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mike, fantastic work removing bugs and using best practices to enhance the user and dev experience in the codebase.
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)