Improved Error Handling (#161)

* added aborting mechanism and enhanced the output in example 01

* defined NOGDI for windows to remove silly clashes from a polluted global namespae -- thanks ms

* added calls to tirex::abort()

* added error reporting to python

* remove outdated comment from measure cmd

* oops :D

Signed-off-by: Sheldon <[email protected]>

* apply patch for cjson

* fixed minor issues

* Fixed an issue with the error handling test

---------

Signed-off-by: Sheldon <[email protected]>

Author: TheMrSheldon

c/examples/01_tracking/CMakeLists.txt

@@ -1,5 +1,19 @@
 add_executable(01_tracking
     main.c
+    plotting.cpp
 )
 
-target_link_libraries(01_tracking tirex::tracker)
+target_link_libraries(01_tracking tirex::tracker)
+
+
+##########################################################################################
+# Libraries
+##########################################################################################
+# asciichart for simple plotting
+CPMAddPackage(URI "gh:Civitasv/asciichart#7464831085adcc51f7389fd6a825cc648a99e82f" EXCLUDE_FROM_ALL YES)
+target_include_directories(01_tracking PRIVATE ${asciichart_SOURCE_DIR}/include)
+
+# cJSON for lightweight parsing of the results
+CPMAddPackage(URI "gh:DaveGamble/[email protected]" EXCLUDE_FROM_ALL YES PATCHES "${CMAKE_CURRENT_LIST_DIR}/cjson.patch")
+target_link_libraries(01_tracking cjson)
+target_include_directories(01_tracking PRIVATE ${cJSON_SOURCE_DIR}) # cJSON does not seem to set this properly so we have to

c/examples/01_tracking/cjson.patch

@@ -0,0 +1,9 @@
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -1,5 +1,5 @@
+ set(CMAKE_LEGACY_CYGWIN_WIN32 0)
+-cmake_minimum_required(VERSION 3.0)
++cmake_minimum_required(VERSION 3.20)
+ 
+ project(cJSON
+     VERSION 1.7.19

c/examples/01_tracking/main.c

@@ -110,6 +110,8 @@ int fib(int n) {
 	return fib(n - 1) + fib(n - 2);
 }
 
+extern void plot(tirexResult* result);
+
 int main(int argc, char* argv[]) {
 	tirexMeasureHandle* measure;
 	tirexResult* result;
@@ -177,17 +179,35 @@ int main(int argc, char* argv[]) {
 	if (tirexStartTracking(providers, 100, &measure) != TIREX_SUCCESS)
 		abort();
 	{
+		// 1) Sleep
 		thrd_sleep(&(struct timespec){.tv_sec = 1}, NULL);
-		char* data = calloc(24 * 1000 * 1000, 1);	  // allocate 24 MB
-		for (size_t i = 0; i < 24 * 1000 * 1000; ++i) // Access the data so it is not optimized away
-			data[i] = 1;
-		thrd_sleep(&(struct timespec){.tv_sec = 1}, NULL);
-		free(data);
+
+		// 2) Allocate Memory then sleep
+		{
+			char* data = calloc(24 * 1000 * 1000, 1);	  // allocate 24 MB
+			for (size_t i = 0; i < 24 * 1000 * 1000; ++i) // Access the data so it is not optimized away
+				data[i] = 1;
+			thrd_sleep(&(struct timespec){.tv_sec = 1}, NULL);
+			free(data);
+		}
+
+		// 3) Calculate Fibonacci
 		fib(45);
+
+		// 4) Allocate some more memory then sleep
+		{
+			char* data = calloc(48 * 1000 * 1000, 1);	  // allocate 24 MB
+			for (size_t i = 0; i < 48 * 1000 * 1000; ++i) // Access the data so it is not optimized away
+				data[i] = 1;
+			thrd_sleep(&(struct timespec){.tv_sec = 1}, NULL);
+			free(data);
+		}
 	}
 	if (tirexStopTracking(measure, &result) != TIREX_SUCCESS)
 		abort();
 	printResult(result, NULL);
+	plot(result);
 	tirexResultFree(result);
+
 	return 0;
 }

c/examples/01_tracking/plotting.cpp

@@ -0,0 +1,59 @@
+#include <tirex_tracker.h>
+
+#include <ascii/ascii.h>
+#include <cJSON.h>
+
+extern "C" {
+void plot(tirexResult* result);
+}
+
+bool tirexResultEntryByMeasure(tirexResult* result, tirexMeasure measure, tirexResultEntry& entry) {
+	size_t num;
+	if (tirexResultEntryNum(result, &num) != tirexError::TIREX_SUCCESS)
+		std::abort();
+	for (size_t i = 0; i < num; ++i) {
+		if (tirexResultEntryGetByIndex(result, i, &entry) == tirexError::TIREX_SUCCESS && entry.source == measure)
+			return true;
+	}
+	return false;
+}
+
+std::vector<double> parseTimeseries(const char* json) {
+	cJSON* parsed = cJSON_Parse(json);
+	auto timeseries = cJSON_GetObjectItem(parsed, "timeseries");
+	// We ignore the timestamps here. This will result in a somewhat skewed graph since the timesteps may not be equidistant
+	auto values = cJSON_GetObjectItem(timeseries, "values");
+	size_t size = cJSON_GetArraySize(values);
+	std::vector<double> data;
+	data.reserve(size);
+	for (size_t i = 0; i < size; ++i) {
+		auto item = cJSON_GetArrayItem(values, i);
+		data.push_back(cJSON_GetNumberValue(item));
+	}
+	cJSON_Delete(parsed);
+	return data;
+}
+
+std::vector<double> readTimeseriesMeasure(tirexResult* result, tirexMeasure measure) {
+	tirexResultEntry entry;
+	if (!tirexResultEntryByMeasure(result, measure, entry))
+		std::abort();
+
+	return parseTimeseries(static_cast<const char*>(entry.value));
+}
+
+void plot(tirexResult* result) {
+	// Note: these two timeseries may not necessarily align in their time-axis
+	{
+		auto cpu = readTimeseriesMeasure(result, tirexMeasure::TIREX_CPU_USED_PROCESS_PERCENT);
+		std::cout << "CPU usage over time:" << std::endl;
+		ascii::Asciichart asciichart(std::vector<std::vector<double>>{cpu});
+		std::cout << '\n' << asciichart.height(10).Plot() << '\n';
+	}
+	{
+		auto ram = readTimeseriesMeasure(result, tirexMeasure::TIREX_RAM_USED_PROCESS_KB);
+		std::cout << "RAM usage over time:" << std::endl;
+		ascii::Asciichart asciichart(std::vector<std::vector<double>>{ram});
+		std::cout << '\n' << asciichart.height(10).Plot() << '\n';
+	}
+}

c/examples/03_measure_command/main.cpp

@@ -95,6 +95,11 @@ static void runMeasureCmd(const MeasureCmdArgs& args) {
 	auto logger = tirex::getLogger("measure");
 	logger->info("Measuring command: {}", args.command);
 
+	if (args.pedantic) {
+		logger->info("Switching to pedantic mode");
+		tirexSetAbortLevel(tirexLogLevel::WARN);
+	}
+
 	// Start measuring
 	std::vector<tirexMeasureConf> measures;
 	for (const auto& provider : args.statproviders) {
@@ -149,7 +154,7 @@ int main(int argc, char* argv[]) {
 			)
 			->default_val(100);
 	app.add_flag("--pedantic", measureArgs.pedantic, "If set, measure will stop execution on errors")
-			->default_val(false); /** \todo support pedantic **/
+			->default_val(false);
 	app.add_option("-o", measureArgs.outfile)->description("Sets the file to write the result measurements into.");
 
 	app.callback([&measureArgs]() { runMeasureCmd(measureArgs); });

c/include/tirex_tracker.h

@@ -17,6 +17,35 @@
 extern "C" {
 #endif
 
+/**
+ * @defgroup logging Logging
+ * @details Definitions and functions relevant for telling measure where to log information to.
+ * @{
+ */
+/**
+ * @brief Represents different levels of verbosity.
+ * @details The enum values are sorted by verbosity. That is, for example, if logs should only be logged \c level
+ * \c INFO or higher, then `level >= INFO` can be used to check what should be logged.
+ */
+typedef enum tirexLogLevel_enum { TRACE, DEBUG, INFO, WARN, ERROR, CRITICAL } tirexLogLevel;
+
+/**
+ * @brief Points to a function that takes in a log level, component, and message which should be logged.
+ * @details The log callback can handle the call as it likes (including ignoring it for example because logging is set
+ * to a lower verbosity or turned of). The \p component may be used to identify, where the log is comming from.
+ */
+typedef void (*tirexLogCallback)(tirexLogLevel level, const char* component, const char* message);
+
+/**
+ * @brief Set a callback that should be used for all future logs.
+ * @details Per default, nothing is logged and passing NULL for \p callback disables logging.
+ * 
+ * @param callback Points to a function that takes in a log level, component, and message which should be logged. Pass
+ * NULL to disable logging.
+ */
+TIREX_TRACKER_EXPORT void tirexSetLogCallback(tirexLogCallback callback);
+/** @} */ // end of logging
+
 /**
  * @defgroup error Error handling
  * @details
@@ -26,6 +55,18 @@ extern "C" {
  * @brief Categorizes different classes of error.
  */
 typedef enum tirexError_enum { TIREX_SUCCESS = 0, TIREX_INVALID_ARGUMENT = 1 } tirexError;
+
+typedef void (*tirexAbortCallback)(const char* message);
+
+TIREX_TRACKER_EXPORT void tirexSetAbortLevel(tirexLogLevel level);
+
+/**
+ * @brief Set a callback that should be used in case the program needs to abort.
+ * @details Per default, [`abort()`](https://en.cppreference.com/w/c/program/abort.html) is used.
+ * 
+ * @param callback
+ */
+TIREX_TRACKER_EXPORT void tirexSetAbortCallback(tirexAbortCallback callback);
 /** @} */ // end of error
 
 /**
@@ -281,35 +322,6 @@ tirexStartTracking(const tirexMeasureConf* measures, size_t pollIntervalMs, tire
 TIREX_TRACKER_EXPORT tirexError tirexStopTracking(tirexMeasureHandle* handle, tirexResult** result);
 /** @} */ // end of measure
 
-/**
- * @defgroup logging Logging
- * @details Definitions and functions relevant for telling measure where to log information to.
- * @{
- */
-/**
- * @brief Represents different levels of verbosity.
- * @details The enum values are sorted by verbosity. That is, for example, if logs should only be logged \c level
- * \c INFO or higher, then `level >= INFO` can be used to check what should be logged.
- */
-typedef enum tirexLogLevel_enum { TRACE, DEBUG, INFO, WARN, ERROR, CRITICAL } tirexLogLevel;
-
-/**
- * @brief Points to a function that takes in a log level, component, and message which should be logged.
- * @details The log callback can handle the call as it likes (including ignoring it for example because logging is set
- * to a lower verbosity or turned of). The \p component may be used to identify, where the log is comming from.
- */
-typedef void (*tirexLogCallback)(tirexLogLevel level, const char* component, const char* message);
-
-/**
- * @brief Set a callback that should be used for all future logs.
- * @details Per default, nothing is logged and passing NULL for \p callback disables logging.
- * 
- * @param callback Points to a function that takes in a log level, component, and message which should be logged. Pass
- * NULL to disable logging.
- */
-TIREX_TRACKER_EXPORT void tirexSetLogCallback(tirexLogCallback callback);
-/** @} */ // end of logging
-
 /**
  * @defgroup dataprovider Data Providers
  * @details Methods and definitions necessary to fetch information about the underlying data sources.

c/src/CMakeLists.txt

@@ -22,10 +22,11 @@ target_sources(tirex_tracker
 			${CMAKE_CURRENT_BINARY_DIR}/tirex_tracker_export.h
 			${TIREX_EXT_INCLUDE_FILES}
 	PRIVATE
+		abort.cpp
+		logging.cpp
 		measureapi.cpp
 		measureinfo.cpp
 		measureresult.cpp
-		logging.cpp
 		measure/stats/provider.cpp
 
 		measure/stats/energystats.cpp

c/src/abort.cpp

@@ -0,0 +1,20 @@
+#include "abort.hpp"
+#include "logging.hpp"
+
+#include <cstdlib>
+
+static void defaultabort(const char* msg) {
+	tirex::log::critical("tracker", "Aborting with Error: {}", msg);
+	std::abort();
+}
+
+static tirexLogLevel abortlevel = tirexLogLevel::CRITICAL;
+static tirexAbortCallback abortfn = defaultabort;
+
+void tirexSetAbortCallback(tirexAbortCallback callback) { abortfn = (callback == nullptr) ? defaultabort : callback; }
+void tirexSetAbortLevel(tirexLogLevel level) { abortlevel = level; }
+
+void tirex::abort(tirexLogLevel level, const char* message) {
+	if (level >= abortlevel)
+		abortfn(message);
+}

c/src/abort.hpp

@@ -0,0 +1,10 @@
+#ifndef ABORT_HPP
+#define ABORT_HPP
+
+#include <tirex_tracker.h>
+
+namespace tirex {
+	void abort(tirexLogLevel level, const char* message);
+} // namespace tirex
+
+#endif

c/src/measure/stats/gitstats.cpp

@@ -1,5 +1,6 @@
 #include "gitstats.hpp"
 
+#include "../../abort.hpp"
 #include "../../logging.hpp"
 #include "../utils/rangeutils.hpp"
 
@@ -164,9 +165,8 @@ static std::string hashAllFiles(git_repository* repo) {
 					"gitstats", "I will not include it in the hash. Please add it to the .gitignore if is not part of "
 								"your codebase or check it into the repository if it should be."
 			);
+			tirex::abort(tirexLogLevel::WARN, "Folders that are not checked into the repository are ignored.");
 			continue;
-			/** \todo if pedantic abort here **/
-			// abort();
 		}
 		std::ifstream is(root / entry->index_to_workdir->new_file.path, std::ios::binary);
 		if (!is) {
@@ -218,12 +218,17 @@ repoToArchive(git_repository* repo, const std::filesystem::path& archive) noexce
 		}
 		auto source = zip_source_file(handle, path.string().c_str(), 0, 0);
 		if (source == nullptr) {
-			tirex::log::error("gitstats", "Error reading file: {}", entry->index_to_workdir->new_file.path);
-			continue; /** \todo handle pedantic? **/
+			tirex::log::error(
+					"gitstats", "Error reading file: {}; I will not add it to the archive",
+					entry->index_to_workdir->new_file.path
+			);
+			tirex::abort(tirexLogLevel::ERROR, "Failed to read file to add it to the archive");
+			continue;
 		}
 		if (zip_file_add(handle, entry->index_to_workdir->new_file.path, source, ZIP_FL_OVERWRITE) < 0) {
 			tirex::log::error("gitstats", "Error adding file to archive: {}", entry->index_to_workdir->new_file.path);
-			continue; /** \todo handle pedantic? **/
+			tirex::abort(tirexLogLevel::ERROR, "Failed to add file to the archive");
+			continue;
 		}
 	}
 	git_status_list_free(list);