-
Notifications
You must be signed in to change notification settings - Fork 3
Fixed the Timeseries Chart/Interface Utilization Graph #228
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
base: master
Are you sure you want to change the base?
Conversation
|
@viniarck @rmotitsuki |
|
You need the related PRs to run it |
|
Also, I blew up the unit tests with a change. I know what it was; I just need to rewrite them. |
|
@rmotitsuki I just found what I believe to be a very clean solution for the dynamic resizing of the graph. I'll maybe have the changes by today. |
|
I don't think it's a good idea for the chart in its current state to have a dynamic width. It should have a base width, and then the SVG can scale up and down while maintaining its aspect ratio. |
|
I just made those changes and fixed the unit tests. |
|
But it seems to be working well now. It even scales. The x-axis ticks sometimes make no sense and squish together, but the D3 functions I am using are supposed to dynamically take care of the ticks as they see fit while it updates and receives new data. |
|
The data does not save when you exit the info panel, I may need to add local storage. |
|
I think Vue 3 has a new feature called |
viniarck
left a comment
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.
@HeriLFIU, fantastic to have the interface utilization graphs again. They look great.
First, I don't have answers to all questions/points you raised, so we'll need to keep researching and reassessing. What I found though regarding the graphs being displayed:
- At some point it was displaying 2 Tbps on a 10 Gbps interface:
- At some point it was not displaying the lines of the chart although it had numeric values:
I'm not sure if all of these details are due to axis scaling, but seem like they need to be adjusted.
How to reproduce
I was using this EPL with a ring topology (but a linear,3 should suffice too):
{
"name": "epl",
"service_level": 6,
"dynamic_backup_path": true,
"uni_a": {
"interface_id": "00:00:00:00:00:00:00:01:1"
},
"uni_z": {
"interface_id": "00:00:00:00:00:00:00:03:1"
}
}
And then I was generating traffic with iperf on mininet:
mininet> iperf h11 h3
*** Iperf: testing TCP bandwidth between h11 and h3
*** Results: ['45.7 Gbits/sec', '45.7 Gbits/sec']
mininet>
@HeriLFIU could you explore this scenario and check out the charts again? Also let me know regarding the timestamp value, let's see if Italo, me or someone else can help out to unblock the kytos_stats PR that you need.
|
@viniarck That other issue with the interfaces, it could be the scaling, but the most likely issue which I think could be the culprit is an old conversion function that I left in and didn't check. |
Right. Let's look into it. Also, btw, I observed the same behavior both when the charts panel were maximized too. |
|
@viniarck The numeric values not displaying is a strange one. Did you leave it on all the time? Currently, when you open and close the info panel, it resets the table, and you need to wait for it to re-update. I'm working on a fix for that with local storage. That's strange because if the table receives bad data, it should just ignore it by default, and it shouldn't stop displaying the values since the table itself isn't updated or changed while it's active; it only collects data from the backend and displays it. |
I observed that behavior when opening a new panel or when switching to another switch (opening the panel again). It stays for several seconds without a line chart sometimes.
Also @HeriLFIU, even when the tx_bytes and rx_bytes aren't incrementing much anymore it's displaying the incorrect interface utilization. For instance in the prior graphs in addition to 229 Gbps being incoherent (way more than the interface speed), it was no longer sending traffic in the data plane, so the counters weren't incrementing much anymore. The UI needs to plot the delta between the samples, I haven't reviewed that part yet, but that should give some clue. Try to look into it, in addition to the part that you shared in the prior reply. |
|
@viniarck Oh, then yes, that's the updating issue; it only runs when its menu/info panel is currently open and resets its data if it switches menus. |
|
Ok, I'll look into the other issue and see what may be causing it. |
|
@viniarck |
|
@viniarck @rmotitsuki It should all be working. I did some testing and the new calculations for bps were working great and the local storage as well. |
|
@viniarck @rmotitsuki There are actually only 2 issues; the charts only run while open. A fix for this is just to have a sort of manager or store that handles all the data and constantly fetches it so that when the timeseries chart is opened, it can fetch the data from the store. Another issue is that when no topology is present, the time series chart throws an error. |
|
Also currently the local storage just keeps pushing new data till infinity; I don't know if that could be a future issue. Maybe a "clear data" button would be nice and a limit to the amount of data stored within the chart. What do you all think? |
|
@HeriLFIU, great to see the latest commits and fixes. I think we're overusing and overstretching what local store is typically for, especially considering the volume of data and the other concerns you raised too (QuotaExceededError and managing all of that data clean up). Can we get rid of local storage and then only display the data when the chart is option and regarding the prior issues/requirements mentioned on #228 (comment), when it still doesn't have enough data then we could simply show that's loading data? These charts here are expected to be a quick glance summarized over a reasonable (up to 15 minutes tops?) time window, that would also simplify what we'd need to maintain in the front-end. What do you think @HeriLFIU and @rmotitsuki? Any other suggestions? |
|
@viniarck That sounds good. Should I also implement a store so that the chart updates while closed, or should it only update while open? Also, if it should only update while open, should I also shorten the time to fetch new data? |
|
@viniarck It's a bit tricky because the component gets removed whenever the tab is not open. When you close the tab/accordion, the data gets deleted as well. It's currently loading in all of the data because of local storage. I need to somehow keep the data even when the tab/accordion is closed, and I think I can do that with a store, but moving a lot of the code and data from the component, especially since it receives a lot of it from other components, is a bit tricky. Maybe I can think of another simpler solution. |
|
@HeriLFIU, since the stats are part of switch details and since just one request to @HeriLFIU, yes, please think about and let's see which one we can go, looks like pre-computing/pre-fetching and keep fetching periodically wouldn't be too far away from what you currently have here right? Except no longer using localStorage but using the browser heap memory |
|
dear god, |
|
@viniarck I was able to apply all of the latest changes, and they all seem to be functioning pretty well. There is only one sad caveat. The store I made for the interfaces—currently I have seen no issues with it (although it gave more than enough issues while writing it)—is excellent and can be reused for many other components. I had this idea in the past of having a store that fetches and constantly refreshes the data for all of the components since it's all very similar, and the code from this store can be recycled to do just that. I believe it just needs a ton of getters to safely retrieve the data and clone it if it's an object or array, but I just discovered something very sad, but it could have a clean solution; it's very tricky to know, especially since it will require a lot of testing. Currently, I had set up the store to also fetch the data from |
|
@viniarck I checked the code for the port stats endpoint, and it seems like it should work. Every time it's called, it should get a new |
|
The reason I wasn't getting this issue before is because I was using a linear topo, and everything seemed to run very smoothly, but now that I run a ring topo, it's very laggy and takes a few minutes to load. |
|
I see a bunch of overlapping stats requests, I don't know much about the backend but that seems related. |
@HeriLFIU that's typically a symptom when either the controller or mininet is overload, usually, it ends up being mininet especially when running in a VM, can you double check if you've given sufficient CPUs/vCPUs and RAM for it?
You should be able to rely on We can also consider ignoring if the front-end ended up seeing the same timestamp per switch, but let's debug here and see if it's also not being truncated, even when things get super overloaded it's still expected to have unique timestamps per switch |
@viniarck I've got 4 CPUs and 8 Gb of ram. Oh, it could also be because I haven't pulled the latest changes from kytos in the longest time. Let me get to it real quick.
|
|
and it could be caching the first request |
Right. Yes, that's a pretty good config/resource, |
That might be it @HeriLFIU, at least the part that was contributing to a same timestamp, now regarding overlapping requests and overall slowness that one we'd have to see, but usually as mentioned before it's when the server doesn't have too much CPU/RAM available, but in your case seems sufficient. Try rebooting too the VM, see if with just ta ring topo if you're able to run it without any log warnings, if you still see warnings we can try to try debug this together |
|
@viniarck I deleted my virtual environment, pulled the latest changes from all repositories, and redid a I also did a lot of testing and I couldn't find any signs of caching. |
|
@viniarck It took me some time, but I believe I have finally identified the root cause of the issue; it was related to references and JS objects. I was pushing an object into an array; I forgot it would be a reference and not a copy. I pushed the response data into an array, and whenever I modified the array, it would in turn modify the response data, which meant that the response data would always be the same. At least that's what I got from the code; let me test it out. |
|
@viniarck I actually don't think the reference issue affected the response. I also checked for caching and saw none of the headers or signs in Chrome DevTools or Postman that show that the data was retrieved from a cache. Could it be being cached in the backend, or is something else going on here? I'm stumped. |
|
@viniarck Also, this seems to be out of whack, because I sometimes restart Kytos and the VM, and I do get different timestamps or updated_at values, but then I reload and it stays the same throughout. Which led me to a bit of confusion. |
|
Maybe it's an issue on my environment. |
|
@viniarck David helped me locate the issue, but it's still a bit strange. I created an issue on |
|
@viniarck The date does not update on request because the function it is within updates when |
|
Ill add an additional check to make sure the data is fresh before putting it into the graph. |
|
@viniarck Actually raising the time to 20 seconds fixed it for a bit, but the overlapping requests started to take hold again, and now it will update no more. |
|
Even if I get a timestamp from the frontend or the backend sends a new timestamp on every request it receives instead of when it receives new data from |
|
@HeriLFIU thanks for the updates.
|
viniarck
left a comment
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.
I've just re-reviewed the code too, looks good @HeriLFIU, the only final thing remaining is dealing with potential stale data points, and think can land. Once again, thanks for all the effort fixing these charts and enabling this feature to work again.
Also, reminder not to forget to collect a good looking screenshot for us to add on the upcoming release notes that will be announced with the 2025.2 release
|
@viniarck I think adding the little stale data option and some color would be the best way to go, because it is the most direct. I think just showing a color to represent stale could be interpreted in different ways and could lead to some confusion, so I'll maybe choose an orange or brown color for stale and add the name. I'll also send you some nice screenshots in a bit. I was installing WSL2 and Docker on my PC to see if I could do some projects directly on Windows without booting up a VM, and I had some drivers, from some lab equipment I worked with for an engineering lab, installed that triggered something and crashed my PC, since all of those lab programs are old anyways and I'll probably never use them again. I'm uninstalling them all to then try and reinstall WSL. |
Sounds good @HeriLFIU, I agree, just a color might not be obvious for whoever is reading the chart in this case, yes, if you can add a name/label that'd be great.
Thanks, you can upload them here for the record, and then whoever is writing the release notes (me or someone else) can use it. Damn, it sucks to hear that you had this crash with WSL2, yes, not using a VM is nice, with Linux I get not to use a VM and things run pretty smoothly. Good luck uninstalling them all and try WSL again, if you succeed with WSL that's very helpful and good to know since that can be helpful for other team mates or new contributors in the future |

















Closes #222
Summary
The Interface Utilization Graph is fixed.
Local Tests
The Graphs displayed properly and no errors were thrown.
Related PR
kytos-ng/kytos_stats#25
kytos-ng/topology#275