ros-tooling / system_metrics_collector

[DEPRECATED] Lightweight, real-time system metrics collector for ROS2 systems
Apache License 2.0
19 stars 8 forks source link

Add statistic name and unit interfaces #115

Closed prajakta-gokhale closed 4 years ago

prajakta-gokhale commented 4 years ago

Changes to topic_statistics_collector interface. Needed for https://github.com/ros-tooling/system_metrics_collector/pull/112.

Also related to https://github.com/ros-tooling/aws-roadmap/issues/197

Signed-off-by: Prajakta Gokhale prajaktg@amazon.com

codecov[bot] commented 4 years ago

Codecov Report

Merging #115 into master will increase coverage by 0.07%. The diff coverage is 48.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage   38.70%   38.77%   +0.07%     
==========================================
  Files          38       39       +1     
  Lines        1465     1488      +23     
  Branches      892      908      +16     
==========================================
+ Hits          567      577      +10     
+ Misses         72       67       -5     
- Partials      826      844      +18     
Flag Coverage Δ
#unittests 38.77% <48.78%> (+0.07%) :arrow_up:
Impacted Files Coverage Δ
...llector/src/system_metrics_collector/collector.hpp 100.00% <ø> (ø)
...m_metrics_collector/linux_cpu_measurement_node.hpp 100.00% <ø> (ø)
...etrics_collector/linux_memory_measurement_node.hpp 100.00% <ø> (ø)
...s_collector/linux_process_cpu_measurement_node.hpp 100.00% <ø> (ø)
...ollector/linux_process_memory_measurement_node.hpp 100.00% <ø> (ø)
...em_metrics_collector/metrics_message_publisher.hpp 100.00% <ø> (ø)
...tatistics_collector/topic_statistics_collector.hpp 100.00% <ø> (ø)
...tistics_collector/test_received_message_period.cpp 18.75% <25.00%> (+0.89%) :arrow_up:
...r/test/system_metrics_collector/test_collector.cpp 46.15% <42.85%> (-0.52%) :arrow_down:
...trics_collector/test_periodic_measurement_node.cpp 27.38% <42.85%> (+1.58%) :arrow_up:
... and 11 more

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 6e9d44c...a4ec797. Read the comment docs.

thomas-moulard commented 4 years ago

The coverage seems to be decreasing with this PR, is there any easy fix for us to try not to reduce coverage on this?

prajakta-gokhale commented 4 years ago

The coverage seems to be decreasing with this PR, is there any easy fix for us to try not to reduce coverage on this?

After rebasing on origin/master it seems to be decreasing by 0.17%. The only new code added is an interface whose implementation is unit tested in concrete derived classes.

Increasing it would be a non-trivial change.

thomas-moulard commented 4 years ago

No worries, this is fine. On the report:

https://codecov.io/gh/ros-tooling/system_metrics_collector/pull/115/diff?src=pr&el=tree-more

test_collector.cpp GetMetricName/Unit methods are not hit currently, just FYI.