Skip to content

Add core number to web stats#89

Merged
koolzz merged 9 commits intosdnfv:developfrom
kevindweb:web_core
Apr 3, 2019
Merged

Add core number to web stats#89
koolzz merged 9 commits intosdnfv:developfrom
kevindweb:web_core

Conversation

@kevindweb
Copy link
Copy Markdown
Contributor

@kevindweb kevindweb commented Mar 24, 2019

Adds a core number label in the web stats view for each running network function.

Summary:

Since getting access to the nf_info->core, we wanted to add this information to our web statistics. In the future the core mapping page will be updated, showing all NFs and manager on their specific cores. Building the web app has to delete and recreate many files, which is why there are so many "changes". There are only 2 line changes from /onvm/onvm_mgr/onvm_stats.c

Usage:
See the web stats documentation for information about running the React website.

This PR includes
Resolves issues
Breaking API changes
Internal API changes
Usability improvements
Bug fixes
New functionality
New NF/onvm_mgr args
Changes to starting NFs
Dependency updates
Web stats updates 👍

Merging notes:

  • Dependencies: None

TODO before merging :

  • PR is ready for review

Test Plan:

Run the web stats with ./go.sh 0,1,2 1 0xF8 -s web and run speed tester nfs to see if the core number affects the website.

Review:

  • Sanity checks, assigned to @koolzz @Arjun-Vijay

  • Run linter

  • Check for memory leaks

    • Check that code is reusable
    • Code is clean, functions are concise
    • Verify that edge cases properly handled
  • Performance, assigned to @koolzz @Arjun-Vijay

    • Run Speed Tester NF, report performance.
  • Documentation, assigned to @koolzz @Arjun-Vijay

    • Check if the new changes are well documented, in both code and READMEs

@onvm
Copy link
Copy Markdown

onvm commented Mar 24, 2019

In response to PR creation

CI Message

Your results will arrive shortly

@onvm

This comment has been minimized.

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 24, 2019

Pull the latest Dev branch commits, there shouldn't be any linter issues

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 24, 2019

I see this rebuild the web stuff so all good

@AaronCoplan
Copy link
Copy Markdown
Contributor

Based on the fact this PR has updates to files within the onvm_web/web-build/ path, it's an indicator that @kevindweb has already run the build. But yes we will want to run it one more time before merging to ensure that the files are up to date.

@kevindweb
Copy link
Copy Markdown
Contributor Author

@onvm let's try again

@onvm
Copy link
Copy Markdown

onvm commented Mar 25, 2019

@onvm let's try again

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented Mar 25, 2019

@onvm let's try again

CI Message

Run successful see results:
[Results from nimbnode30]
Median TX pps for Speed Tester: 35161576

Linter Passed

@koolzz koolzz self-requested a review March 26, 2019 05:08
@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

I clicked on this first, brace yourselves for some CI emails

@onvm gooo

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm hello

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm bump

1 similar comment
@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm bump

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm boomp

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm bip bop

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm pls

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm go

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm gogogo

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm тык

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm тык

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm тык

CI Message

Error: ERROR: Script failed on nimbnode30

Linter Passed

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm good good again

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm good good again

CI Message

Your results will arrive shortly

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm тык

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm тык

CI Message

Another CI run in progress, please try again in 15 minutes

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm good good again

CI Message

Run successful see results:
[Results from nimbnode30]
Median TX pps for Speed Tester: 35158836

Linter Passed

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm pls work I wanna sleep

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm pls work I wanna sleep

CI Message

Your results will arrive shortly

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm last time

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm last time

CI Message

Your results will arrive shortly

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 28, 2019

@onvm well now last time for sure

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm well now last time for sure

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented Mar 28, 2019

@onvm well now last time for sure

CI Message

Error: ERROR: Script failed on nimbnode30

Linter Passed

Copy link
Copy Markdown
Member

@koolzz koolzz left a comment

Choose a reason for hiding this comment

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

Move the doc changes to a separate PR. Otherwise I tested it and it works.

Comment thread docs/Debug.md Outdated
@@ -1,57 +1,59 @@
Debugging OpenNetVM
==
# Debugging OpenNetVM
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few doc changes are fine to mix in with other PRs, but you have this + install doc which is about 400 line changes. Can you move it to another PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry I didn't see this, I'll fix it

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 29, 2019

@kevindweb Could you look into adding a minify step to the CSS and JS files to the build? This would fix our problem with the build being too inflated (this pr alone is 55k line changes right now)

@kevindweb
Copy link
Copy Markdown
Contributor Author

Finally got minified files to work! @onvm check it out one last time

@onvm
Copy link
Copy Markdown

onvm commented Apr 2, 2019

Finally got minified files to work! @onvm check it out one last time

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented Apr 2, 2019

Finally got minified files to work! @onvm check it out one last time

CI Message

Run successful see results:
[Results from nimbnode30]
Median TX pps for Speed Tester: 35211377

Linter Passed

Copy link
Copy Markdown
Member

@koolzz koolzz left a comment

Choose a reason for hiding this comment

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

Tested everything works as expected, great work on minifying everything. One tweak to this, can we automatically install uglifycss and uglify-js? Or even just creating a build README with all the dependencies would be useful, or do we have that somewhere?

@koolzz koolzz merged commit b8d7deb into sdnfv:develop Apr 3, 2019
@koolzz koolzz added this to the ONVM 19.05 Release milestone May 26, 2019
@kevindweb kevindweb deleted the web_core branch June 1, 2019 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants