Create libstatistics_collector package

[dabonnie]

Create new libstatistics_collector library and copy relevant classes to new library

This is part of #224 to done.

Signed-off-by: Devin Bonnie dbbonnie@amazon.com

[codecov]

Codecov Report

Merging #117 into master will decrease coverage by 5.23%.
The diff coverage is 16.66%.

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
- Coverage   38.77%   33.54%   -5.24%     
==========================================
  Files          39       26      -13     
  Lines        1488     1106     -382     
  Branches      908      724     -184     
==========================================
- Hits          577      371     -206     
+ Misses         67       63       -4     
+ Partials      844      672     -172     

Flag	Coverage Δ
#unittests	33.54% <16.66%> (-5.24%)	⬇️

Impacted Files	Coverage Δ
...em_metrics_collector/metrics_message_publisher.cpp	62.50% <ø> (ø)
...em_metrics_collector/metrics_message_publisher.hpp	100.00% <ø> (ø)
...em_metrics_collector/periodic_measurement_node.hpp	100.00% <ø> (ø)
...r/test/system_metrics_collector/test_functions.hpp	50.87% <ø> (ø)
...m_metrics_collector/test_linux_cpu_measurement.cpp	18.42% <ø> (ø)
...etrics_collector/test_linux_memory_measurement.cpp	20.86% <0.00%> (ø)
...tor/test_linux_process_memory_measurement_node.cpp	42.85% <0.00%> (ø)
...trics_collector/test_metrics_message_publisher.cpp	34.14% <ø> (ø)
...trics_collector/test_periodic_measurement_node.cpp	27.38% <0.00%> (ø)
...lector/test_linux_process_cpu_measurement_node.cpp	25.42% <50.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e672ed0...20f7160. Read the comment docs.

[dabonnie]

Ah sorry, I marked this as ready for review, but neglected to move the topic statistics classes to the libcollector package.

[dabonnie]

Will look into the decreased code coverage, waiting for the latest report.

[zmichaels11]

Only concern is that libcollector already exists and does something similar:

libcollector - API for the Performance Tools collector library

The shared object, libcollector.so, is used to collect performance data and is normally loaded using LD_PRELOAD by the collect command or by dbx when the Collector is enabled. 

[piraka9011]

Ditto Zach's comment, I'm assuming this was part of the design discussion but any chance we can make the library name a bit less generic to avoid potential name clashes?

[dabonnie]

It appears that we are generating reports for the system_metrics_collector package only via this report, so that would make sense why coverage decreased given the only changes are moving code.

[thomas-moulard]

isn'it the default?

[thomas-moulard]

What about this tested on OS X and Windows? If ROS2 is depending on it, I guess we'll need to make sure it works too there?

[mm318]

Removed ARCHIVE and INCLUDES. Are you saying we should remove the whole thing?

[thomas-moulard]

SGTM with the structure cleanup, consider using a Pimpl to avoid leaking the mutex to your public header.

[dabonnie]

Updated name and made sure tests are passing. Still needs the following:

• update namespace (current libstatistics_collector namespaces should be nested under namespace libstatistics_collector
• update libstatistics_collector and system_metrics_collector READMEs
• cleanup libstatistics_collector CMakeLists.txt

[thomas-moulard]

you'll need this, my comment was about removing setting ARCHIVE, etc. explicitly because I'm not sure whether it'd work on Windows.

[zmichaels11]

I think this is also the wrong syntax for a todo comment

[mm318]

Removed ARCHIVE and INCLUDES (seeing as how there's a separate install() command that follows for the include/ directory).

[mm318]

I've addressed #117 (comment). Can you please re-review, @dabonnie, especially the changes to the README.md?

[dabonnie]

LGTM, but I can't approve my own PR.

[thomas-moulard]

LGTM w/ ns cleaned-up

[zmichaels11]

LGTM.
You can skip the suggestions I left if you want.
I'd weigh the "move using into unnamed namespace" suggestions higher than using auto.