make statistics available in metrics API no matter what

Previously, if `--server.statistics` was set to `false`, the metrics API did not return any values provided by the StatisticsFeature, as the statistics were turned off completely. This prevented the metrics API from returning some useful process statistics (e.g. memory usage, number of threads etc.). This change now enables these process statistics as well as a few others in metrics regardless of the setting of `--server.statistics`. This was already the case in 3.8 and devel, but 3.7 still behaved slightly differently
arangodb · KVS85 · Aug 27, 2021 · Aug 26, 2021 · Aug 26, 2021 · Aug 26, 2021
commit 393e5b44f47d2eaa9a15ab90b1e8edbc56e670d8
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,10 @@
 v3.7.15 (XXXX-XX-XX)
 --------------------
 
+* Make statistics available in metrics API even if `--server.statistics` is set
+  to `false`. This allows clients to fetch some process statistics metrics from
+  instances that have statistics aggregation and storage turned off.
+
 * When creating Pregel memory-mapped files, create them with O_TMPFILE attribute
   on Linux so that files are guaranteed to vanish even if a process dies.
 

diff --git a/arangod/RestServer/MetricsFeature.cpp b/arangod/RestServer/MetricsFeature.cpp
@@ -85,9 +85,7 @@ void MetricsFeature::validateOptions(std::shared_ptr<ProgramOptions>) {
   }
 }
 
-
 void MetricsFeature::toPrometheus(std::string& result) const {
-
   // minimize reallocs
   result.reserve(32768);
 
@@ -100,10 +98,8 @@ void MetricsFeature::toPrometheus(std::string& result) const {
 
   // StatisticsFeature
   auto& sf = server().getFeature<StatisticsFeature>();
-  if (sf.enabled()) {
-    sf.toPrometheus(result, std::chrono::duration<double, std::milli>(
-                      std::chrono::system_clock::now().time_since_epoch()).count());
-  }
+  sf.toPrometheus(result, std::chrono::duration<double, std::milli>(
+                    std::chrono::system_clock::now().time_since_epoch()).count());
 
   // RocksDBEngine
   auto es = EngineSelectorFeature::ENGINE;

diff --git a/arangod/Statistics/ConnectionStatistics.cpp b/arangod/Statistics/ConnectionStatistics.cpp
@@ -72,11 +72,6 @@ ConnectionStatistics::Item ConnectionStatistics::acquire() {
 }
 
 void ConnectionStatistics::getSnapshot(Snapshot& snapshot) {
-  if (!StatisticsFeature::enabled()) {
-    // all the below objects may be deleted if we don't have statistics enabled
-    return;
-  }
-
   snapshot.httpConnections = statistics::HttpConnections;
   snapshot.totalRequests = statistics::TotalRequests;
   snapshot.totalRequestsSuperuser = statistics::TotalRequestsSuperuser;

diff --git a/arangod/Statistics/RequestStatistics.cpp b/arangod/Statistics/RequestStatistics.cpp
@@ -195,11 +195,6 @@ void RequestStatistics::release() {
 }
 
 void RequestStatistics::getSnapshot(Snapshot& snapshot, stats::RequestStatisticsSource source) {
-  if (!StatisticsFeature::enabled()) {
-    // all the below objects may be deleted if we don't have statistics enabled
-    return;
-  }
-
   statistics::RequestFigures& figures = source == stats::RequestStatisticsSource::USER
     ? statistics::UserRequestFigures
     : statistics::SuperuserRequestFigures;

diff --git a/arangod/Statistics/StatisticsFeature.cpp b/arangod/Statistics/StatisticsFeature.cpp
diff --git a/arangod/Statistics/StatisticsFeature.h b/arangod/Statistics/StatisticsFeature.h
@@ -24,6 +24,7 @@
 #define APPLICATION_FEATURES_STATISTICS_FEATURE_H 1
 
 #include <array>
+#include <initializer_list>
 
 #include "ApplicationFeatures/ApplicationFeature.h"
 #include "Basics/Result.h"
@@ -115,7 +116,15 @@ class StatisticsFeature final : public application_features::ApplicationFeature
 
   bool allDatabases() const { return _statisticsAllDatabases; }
 
+  static arangodb::velocypack::Builder fillDistribution(statistics::Distribution const& dist);
+
  private:
+  void appendHistogram(
+    std::string& result, statistics::Distribution const& dist,
+    std::string const& label, std::initializer_list<std::string> const& les) const;
+  void appendMetric(
+    std::string& result, std::string const& val, std::string const& label) const;
+
   bool _statistics;
   bool _statisticsHistory;
   bool _statisticsHistoryTouched;

diff --git a/arangod/Statistics/StatisticsWorker.cpp b/arangod/Statistics/StatisticsWorker.cpp
diff --git a/arangod/Statistics/StatisticsWorker.h b/arangod/Statistics/StatisticsWorker.h
@@ -73,12 +73,6 @@ class StatisticsWorker final : public Thread {
   // save one statistics object
   void saveSlice(velocypack::Slice const&, std::string const&) const;
 
-  void appendHistogram(
-    std::string& result, statistics::Distribution const& dist,
-    std::string const& label, std::initializer_list<std::string> const& les) const;
-  void appendMetric(
-    std::string& result, std::string const& val, std::string const& label) const;
-
   static constexpr uint64_t STATISTICS_INTERVAL = 10;    // 10 secs
   static constexpr uint64_t GC_INTERVAL = 8 * 60;        //  8 mins
   static constexpr uint64_t HISTORY_INTERVAL = 15 * 60;  // 15 mins

diff --git a/tests/js/client/server_parameters/test-statistics-enabled-false.js b/tests/js/client/server_parameters/test-statistics-enabled-false.js
@@ -0,0 +1,88 @@
+/*jshint globalstrict:false, strict:false */
+/* global getOptions, assertEqual, assertTrue, arango */
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test for security-related server options
+///
+/// @file
+///
+/// DISCLAIMER
+///
+/// Copyright 2010-2012 triagens GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB Inc, Cologne, Germany
+///
+/// @author Jan Steemann
+/// @author Copyright 2019, ArangoDB Inc, Cologne, Germany
+////////////////////////////////////////////////////////////////////////////////
+
+if (getOptions === true) {
+  return {
+    'server.statistics': "false",
+    'server.export-metrics-api': "true",
+  };
+}
+
+const jsunity = require('jsunity');
+
+function testSuite() {
+  return {
+    testGetAdminStatistics : function() {
+      let res = arango.GET("/_admin/statistics");
+      assertTrue(res.error);
+      assertEqual(404, res.code);
+    },
+
+    testGetAdminStatisticsDescription : function() {
+      let res = arango.GET("/_admin/statistics-description");
+      assertTrue(res.error);
+      assertEqual(404, res.code);
+    },
+
+    testGetMetrics : function() {
+      let metrics = {};
+      String(arango.GET("/_admin/metrics"))
+        .split("\n")
+        .filter((line) => line.match(/^[^#]/))
+        .filter((line) => line.match(/^arangodb_/))
+        .forEach((line) => {
+          let name = line.replace(/[ \{].*$/g, '');
+          let value = Number(line.replace(/^.+ (\d+)$/, '$1'));
+          metrics[name] = value;
+        });
+
+      const expected = [
+        "arangodb_process_statistics_minor_page_faults",
+        "arangodb_process_statistics_major_page_faults",
+        "arangodb_process_statistics_user_time",
+        "arangodb_process_statistics_system_time",
+        "arangodb_process_statistics_number_of_threads",
+        "arangodb_process_statistics_resident_set_size",
+        "arangodb_process_statistics_resident_set_size_percent",
+        "arangodb_process_statistics_virtual_memory_size",
+        "arangodb_server_statistics_physical_memory",
+        "arangodb_server_statistics_server_uptime",
+      ];
+
+      expected.forEach((name) => {
+        assertTrue(metrics.hasOwnProperty(name));
+        assertEqual("number", typeof metrics[name]);
+      });
+    },
+  };
+}
+
+jsunity.run(testSuite);
+return jsunity.done();