Combined and renamed `csi_*_plugin_terminations` metrics. To avoid double counting when the operator aggregates the `csi_controller_plugin_terminations` and `csi_node_plugin_terminations`, these two are now merged into `csi_plugin/container_terminations`.
Review: https://reviews.apache.org/r/67224 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/43143343 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/43143343 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/43143343 Branch: refs/heads/master Commit: 43143343905f5e83d79b9a8d7b27664d224d2176 Parents: 5bdea19 Author: Chun-Hung Hsiao <chhs...@mesosphere.io> Authored: Fri May 18 15:54:53 2018 -0700 Committer: Chun-Hung Hsiao <chhs...@mesosphere.io> Committed: Thu May 31 18:29:56 2018 -0700 ---------------------------------------------------------------------- src/resource_provider/storage/provider.cpp | 23 +++++--------------- .../storage_local_resource_provider_tests.cpp | 20 +++++------------ 2 files changed, 12 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/43143343/src/resource_provider/storage/provider.cpp ---------------------------------------------------------------------- diff --git a/src/resource_provider/storage/provider.cpp b/src/resource_provider/storage/provider.cpp index 542d3ae..8a4b037 100644 --- a/src/resource_provider/storage/provider.cpp +++ b/src/resource_provider/storage/provider.cpp @@ -497,8 +497,7 @@ private: ~Metrics(); // CSI plugin metrics. - Counter csi_controller_plugin_terminations; - Counter csi_node_plugin_terminations; + Counter csi_plugin_container_terminations; // Operation state metrics. hashmap<Offer::Operation::Type, PushGauge> operations_pending; @@ -1898,13 +1897,7 @@ Future<csi::v0::Client> StorageLocalResourceProviderProcess::getService( })); })), std::function<Future<Nothing>()>(defer(self(), [=]() -> Future<Nothing> { - if (containerId == controllerContainerId.get()) { - metrics.csi_controller_plugin_terminations++; - } - - if (containerId == nodeContainerId.get()) { - metrics.csi_node_plugin_terminations++; - } + ++metrics.csi_plugin_container_terminations; services.at(containerId)->discard(); services.at(containerId).reset(new Promise<csi::v0::Client>()); @@ -3435,13 +3428,10 @@ void StorageLocalResourceProviderProcess::sendOperationStatusUpdate( StorageLocalResourceProviderProcess::Metrics::Metrics(const string& prefix) - : csi_controller_plugin_terminations( - prefix + "csi_controller_plugin_terminations"), - csi_node_plugin_terminations( - prefix + "csi_node_plugin_terminations") + : csi_plugin_container_terminations( + prefix + "csi_plugin/container_terminations") { - process::metrics::add(csi_controller_plugin_terminations); - process::metrics::add(csi_node_plugin_terminations); + process::metrics::add(csi_plugin_container_terminations); vector<Offer::Operation::Type> operationTypes; @@ -3507,8 +3497,7 @@ StorageLocalResourceProviderProcess::Metrics::Metrics(const string& prefix) StorageLocalResourceProviderProcess::Metrics::~Metrics() { - process::metrics::remove(csi_controller_plugin_terminations); - process::metrics::remove(csi_node_plugin_terminations); + process::metrics::remove(csi_plugin_container_terminations); foreachvalue (const PushGauge& gauge, operations_pending) { process::metrics::remove(gauge); http://git-wip-us.apache.org/repos/asf/mesos/blob/43143343/src/tests/storage_local_resource_provider_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/storage_local_resource_provider_tests.cpp b/src/tests/storage_local_resource_provider_tests.cpp index 3a2eec3..9bd8558 100644 --- a/src/tests/storage_local_resource_provider_tests.cpp +++ b/src/tests/storage_local_resource_provider_tests.cpp @@ -2790,8 +2790,8 @@ TEST_F( // This test verifies that storage local resource provider properly -// reports metrics related to CSI plugin terminations. -TEST_F(StorageLocalResourceProviderTest, ROOT_PluginTerminationMetrics) +// reports the metric related to CSI plugin container terminations. +TEST_F(StorageLocalResourceProviderTest, ROOT_ContainerTerminationMetric) { setupResourceProviderConfig(Gigabytes(4)); @@ -2829,13 +2829,9 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_PluginTerminationMetrics) JSON::Object snapshot = Metrics(); ASSERT_NE(0u, snapshot.values.count(metricName( - "csi_controller_plugin_terminations"))); + "csi_plugin/container_terminations"))); EXPECT_EQ(0, snapshot.values.at(metricName( - "csi_controller_plugin_terminations"))); - ASSERT_NE(0u, snapshot.values.count(metricName( - "csi_node_plugin_terminations"))); - EXPECT_EQ(0, snapshot.values.at(metricName( - "csi_node_plugin_terminations"))); + "csi_plugin/container_terminations"))); // Get the ID of the CSI plugin container. Future<hashset<ContainerID>> pluginContainers = containerizer->containers(); @@ -2863,13 +2859,9 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_PluginTerminationMetrics) snapshot = Metrics(); ASSERT_NE(0u, snapshot.values.count(metricName( - "csi_controller_plugin_terminations"))); - EXPECT_EQ(1, snapshot.values.at(metricName( - "csi_controller_plugin_terminations"))); - ASSERT_NE(0u, snapshot.values.count(metricName( - "csi_node_plugin_terminations"))); + "csi_plugin/container_terminations"))); EXPECT_EQ(1, snapshot.values.at(metricName( - "csi_node_plugin_terminations"))); + "csi_plugin/container_terminations"))); }
