szaszm commented on a change in pull request #1152:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1152#discussion_r732604183
##########
File path: extensions/procfs/CPPLINT.cfg
##########
@@ -0,0 +1 @@
+filter=-runtime/int
Review comment:
Is this still relevant? I didn't see any `long` in the code and this
checks for `short`, `long`, and `long long` AFAIK.
##########
File path: extensions/procfs/CpuStat.cpp
##########
@@ -0,0 +1,116 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#include "CpuStat.h"
+
+namespace org::apache::nifi::minifi::procfs {
+
+std::optional<CpuStatData> CpuStatData::parseCpuStatLine(std::istringstream&
iss) {
Review comment:
This only depends on the `std::istream` interface, consider relaxing the
requirements by accepting `std::istream&`.
##########
File path: extensions/procfs/CpuStat.h
##########
@@ -0,0 +1,140 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+
+#include <sstream>
+#include <string>
+#include <utility>
+#include <optional>
+#include "rapidjson/document.h"
+
+namespace org::apache::nifi::minifi::procfs {
+
+class CpuStatData {
+ CpuStatData() = default;
+
+ public:
+ CpuStatData(const CpuStatData& src) = default;
+ CpuStatData(CpuStatData&& src) noexcept = default;
+
+ static std::optional<CpuStatData> parseCpuStatLine(std::istringstream& iss);
+
+ bool operator>=(const CpuStatData &rhs) const;
+ bool operator==(const CpuStatData &rhs) const;
+ CpuStatData operator-(const CpuStatData &rhs) const;
+
+ uint64_t getUser() const { return user_; }
+ uint64_t getNice() const { return nice_; }
+ uint64_t getSystem() const { return system_; }
+ uint64_t getIdle() const { return idle_; }
+ uint64_t getIoWait() const { return io_wait_; }
+ uint64_t getIrq() const { return irq_; }
+ uint64_t getSoftIrq() const { return soft_irq_; }
+ uint64_t getSteal() const { return steal_; }
+ uint64_t getGuest() const { return guest_; }
+ uint64_t getGuestNice() const { return guest_nice_; }
+
+ uint64_t getIdleAll() const { return idle_ + io_wait_; }
+ uint64_t getSystemAll() const { return system_ + irq_ + soft_irq_; }
+ uint64_t getVirtAll() const { return guest_ + guest_nice_; }
+ uint64_t getTotal() const { return user_ + nice_ + getSystemAll() +
getIdleAll() + steal_ + getVirtAll(); }
Review comment:
Cosider using `[[nodiscard]]` and `noexcept` on getters
##########
File path: extensions/procfs/CpuStat.cpp
##########
@@ -0,0 +1,116 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#include "CpuStat.h"
+
+namespace org::apache::nifi::minifi::procfs {
+
+std::optional<CpuStatData> CpuStatData::parseCpuStatLine(std::istringstream&
iss) {
+ CpuStatData data;
+ iss >> data.user_ >> data.nice_ >> data.system_ >> data.idle_ >>
data.io_wait_ >> data.irq_ >> data.soft_irq_ >> data.steal_ >> data.guest_ >>
data.guest_nice_;
+ if (iss.fail())
+ return std::nullopt;
+ data.user_ -= data.guest_; // Guest time is already accounted in usertime
+ data.nice_ -= data.guest_nice_;
+ return data;
+}
+
+bool CpuStatData::operator>=(const CpuStatData &rhs) const {
+ return user_ >= rhs.user_
+ && nice_ >= rhs.nice_
+ && system_ >= rhs.system_
+ && idle_ >= rhs.idle_
+ && io_wait_ >= rhs.io_wait_
+ && irq_ >= rhs.irq_
+ && soft_irq_ >= rhs.soft_irq_
+ && steal_ >= rhs.steal_
+ && guest_ >= rhs.guest_
+ && guest_nice_ >= rhs.guest_nice_;
+}
Review comment:
Does this implementation makes sense? It can happen that both `a < b &&
b < a` are true. If you want lexicographical comparison instead, you can use an
explicitly defaulted `operator<=>` instead of both implemented operators.
##########
File path: CMakeLists.txt
##########
@@ -587,6 +587,14 @@ if (ENABLE_SYSTEMD)
createExtension(SYSTEMD-EXTENSIONS "SYSTEMD EXTENSIONS" "Enabled log
collection from journald" "extensions/systemd" "extensions/systemd/tests")
endif()
+## Add the procfs extension
+if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
Review comment:
This has no extra dependencies other than procfs, which is already
present on all systems. I would consider adding it to standard-processors on
linux.
##########
File path: extensions/procfs/CpuStat.h
##########
@@ -0,0 +1,140 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+
+#include <sstream>
+#include <string>
+#include <utility>
+#include <optional>
+#include "rapidjson/document.h"
+
+namespace org::apache::nifi::minifi::procfs {
+
+class CpuStatData {
+ CpuStatData() = default;
+
+ public:
+ CpuStatData(const CpuStatData& src) = default;
+ CpuStatData(CpuStatData&& src) noexcept = default;
+
+ static std::optional<CpuStatData> parseCpuStatLine(std::istringstream& iss);
+
+ bool operator>=(const CpuStatData &rhs) const;
+ bool operator==(const CpuStatData &rhs) const;
+ CpuStatData operator-(const CpuStatData &rhs) const;
+
+ uint64_t getUser() const { return user_; }
+ uint64_t getNice() const { return nice_; }
+ uint64_t getSystem() const { return system_; }
+ uint64_t getIdle() const { return idle_; }
+ uint64_t getIoWait() const { return io_wait_; }
+ uint64_t getIrq() const { return irq_; }
+ uint64_t getSoftIrq() const { return soft_irq_; }
+ uint64_t getSteal() const { return steal_; }
+ uint64_t getGuest() const { return guest_; }
+ uint64_t getGuestNice() const { return guest_nice_; }
+
+ uint64_t getIdleAll() const { return idle_ + io_wait_; }
+ uint64_t getSystemAll() const { return system_ + irq_ + soft_irq_; }
+ uint64_t getVirtAll() const { return guest_ + guest_nice_; }
+ uint64_t getTotal() const { return user_ + nice_ + getSystemAll() +
getIdleAll() + steal_ + getVirtAll(); }
+
+ private:
+ uint64_t user_;
+ uint64_t nice_;
+ uint64_t system_;
+ uint64_t idle_;
+ uint64_t io_wait_;
+ uint64_t irq_;
+ uint64_t soft_irq_;
+ uint64_t steal_;
+ uint64_t guest_;
+ uint64_t guest_nice_;
+};
+
+class CpuStat {
+ public:
+ static constexpr const char USER_STR[] = "user time";
+ static constexpr const char NICE_STR[] = "nice time";
+ static constexpr const char SYSTEM_STR[] = "system time";
+ static constexpr const char IDLE_STR[] = "idle time";
+ static constexpr const char IO_WAIT_STR[] = "io wait time";
+ static constexpr const char IRQ_STR[] = "irq time";
+ static constexpr const char SOFT_IRQ_STR[] = "soft irq time";
+ static constexpr const char STEAL_STR[] = "steal time";
+ static constexpr const char GUEST_STR[] = "guest time";
+ static constexpr const char GUEST_NICE_STR[] = "guest nice time";
+
+ CpuStat(const CpuStat& src) = default;
+ CpuStat(CpuStat&& src) noexcept = default;
+
Review comment:
The class would be simpler without these lines, since all of the [rule
of
five](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c21-if-you-define-or-delete-any-copy-move-or-destructor-function-define-or-delete-them-all)
operations are defaulted in one way or another.
##########
File path: extensions/procfs/CpuStat.h
##########
@@ -0,0 +1,140 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+
+#include <sstream>
+#include <string>
+#include <utility>
+#include <optional>
+#include "rapidjson/document.h"
+
+namespace org::apache::nifi::minifi::procfs {
+
+class CpuStatData {
+ CpuStatData() = default;
+
+ public:
+ CpuStatData(const CpuStatData& src) = default;
+ CpuStatData(CpuStatData&& src) noexcept = default;
+
+ static std::optional<CpuStatData> parseCpuStatLine(std::istringstream& iss);
+
+ bool operator>=(const CpuStatData &rhs) const;
+ bool operator==(const CpuStatData &rhs) const;
+ CpuStatData operator-(const CpuStatData &rhs) const;
+
+ uint64_t getUser() const { return user_; }
+ uint64_t getNice() const { return nice_; }
+ uint64_t getSystem() const { return system_; }
+ uint64_t getIdle() const { return idle_; }
+ uint64_t getIoWait() const { return io_wait_; }
+ uint64_t getIrq() const { return irq_; }
+ uint64_t getSoftIrq() const { return soft_irq_; }
+ uint64_t getSteal() const { return steal_; }
+ uint64_t getGuest() const { return guest_; }
+ uint64_t getGuestNice() const { return guest_nice_; }
+
+ uint64_t getIdleAll() const { return idle_ + io_wait_; }
+ uint64_t getSystemAll() const { return system_ + irq_ + soft_irq_; }
+ uint64_t getVirtAll() const { return guest_ + guest_nice_; }
+ uint64_t getTotal() const { return user_ + nice_ + getSystemAll() +
getIdleAll() + steal_ + getVirtAll(); }
+
+ private:
+ uint64_t user_;
+ uint64_t nice_;
+ uint64_t system_;
+ uint64_t idle_;
+ uint64_t io_wait_;
+ uint64_t irq_;
+ uint64_t soft_irq_;
+ uint64_t steal_;
+ uint64_t guest_;
+ uint64_t guest_nice_;
+};
+
+class CpuStat {
+ public:
+ static constexpr const char USER_STR[] = "user time";
+ static constexpr const char NICE_STR[] = "nice time";
+ static constexpr const char SYSTEM_STR[] = "system time";
+ static constexpr const char IDLE_STR[] = "idle time";
+ static constexpr const char IO_WAIT_STR[] = "io wait time";
+ static constexpr const char IRQ_STR[] = "irq time";
+ static constexpr const char SOFT_IRQ_STR[] = "soft irq time";
+ static constexpr const char STEAL_STR[] = "steal time";
+ static constexpr const char GUEST_STR[] = "guest time";
+ static constexpr const char GUEST_NICE_STR[] = "guest nice time";
+
+ CpuStat(const CpuStat& src) = default;
+ CpuStat(CpuStat&& src) noexcept = default;
+
+ CpuStat(std::string cpu_name, CpuStatData data) :
cpu_name_(std::move(cpu_name)), data_(data) {}
+
+ void addToJson(rapidjson::Value& body, rapidjson::Document::AllocatorType&
alloc) const;
+ const CpuStatData& getData() const { return data_; }
+ const std::string& getName() const { return cpu_name_; }
+
+ protected:
+ std::string cpu_name_;
+ CpuStatData data_;
+};
+
+class CpuStatPeriod {
+ CpuStatPeriod(std::string cpu_name, double period, CpuStatData data_diff) :
cpu_name_(std::move(cpu_name)), period_(period), data_diff_(data_diff) {}
+
+ public:
+ static constexpr const char USER_PERCENT_STR[] = "user time %";
+ static constexpr const char NICE_PERCENT_STR[] = "nice time %";
+ static constexpr const char SYSTEM_PERCENT_STR[] = "system time %";
+ static constexpr const char IDLE_PERCENT_STR[] = "idle time %";
+ static constexpr const char IO_WAIT_PERCENT_STR[] = "io wait time %";
+ static constexpr const char IRQ_PERCENT_STR[] = "irq time %";
+ static constexpr const char SOFT_IRQ_PERCENT_STR[] = "soft irq %";
+ static constexpr const char STEAL_PERCENT_STR[] = "steal time %";
+ static constexpr const char GUEST_PERCENT_STR[] = "guest time %";
+ static constexpr const char GUEST_NICE_PERCENT_STR[] = "guest nice time %";
+
+ CpuStatPeriod(const CpuStatPeriod& src) = default;
+ CpuStatPeriod(CpuStatPeriod&& src) noexcept = default;
Review comment:
same here
##########
File path: extensions/procfs/CpuStat.h
##########
@@ -0,0 +1,140 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+
+#include <sstream>
+#include <string>
+#include <utility>
+#include <optional>
+#include "rapidjson/document.h"
+
+namespace org::apache::nifi::minifi::procfs {
+
+class CpuStatData {
+ CpuStatData() = default;
+
+ public:
+ CpuStatData(const CpuStatData& src) = default;
+ CpuStatData(CpuStatData&& src) noexcept = default;
Review comment:
Follow the [rule of
zero](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c20-if-you-can-avoid-defining-default-operations-do)
##########
File path: extensions/procfs/MemInfo.h
##########
@@ -0,0 +1,51 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+
+#include <string>
+#include <sstream>
+#include <fstream>
+#include <optional>
+#include "rapidjson/document.h"
+
+namespace org::apache::nifi::minifi::procfs {
+
+class MemInfo {
+ MemInfo() = default;
+
+ public:
+ MemInfo(const MemInfo& src) = default;
+ MemInfo(MemInfo&& src) noexcept = default;
+ static std::optional<MemInfo> parseMemInfoFile(std::ifstream& mem_info_file);
+
+ void addToJson(rapidjson::Value& body, rapidjson::Document::AllocatorType&
alloc) const;
+
+ uint64_t getTotalMemory() const { return memory_total_; }
+ uint64_t getFreeMemory() const { return memory_free_; }
+ uint64_t getAvailableMemory() const { return memory_available_; }
+ uint64_t getTotalSwap() const { return swap_total_; }
+ uint64_t getFreeSwap() const { return swap_free_; }
Review comment:
Please consider adding `[[nodiscard]]` and `noexcept` to all getters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
