[GitHub] [trafficcontrol] ezelkow1 commented on a change in pull request #4713: Add csv as an output format option for astats via header

[GitBox <gi...@apache.org>]

ezelkow1 commented on a change in pull request #4713:
URL: https://github.com/apache/trafficcontrol/pull/4713#discussion_r430499479



##########
File path: traffic_server/plugins/astats_over_http/astats_over_http.c
##########
@@ -326,7 +383,40 @@ static void appendSystemState(stats_state *my_state) {
 		end = strstr(str, "\n");
 		if (end)
 			*end = 0;
-		APPEND_STAT("proc.loadavg", "\"%s\"", str);
+		APPEND_STAT_JSON("proc.loadavg", "\"%s\"", str);
+	}
+}
+
+static void appendSystemStateCsv(stats_state *my_state) {
+	char *interface = my_state->interfaceName;
+	char buffer[16384];
+	char *str;
+	char *end;
+	int speed = 0;
+
+	APPEND_STAT_CSV("inf.name", "%s", interface);
+
+	speed = getSpeed(interface, buffer, sizeof(buffer));
+
+	APPEND_STAT_CSV("inf.speed", "%d", speed);
+
+	str = getFile("/proc/net/dev", buffer, sizeof(buffer));
+	if (str && interface) {
+		str = strstr(str, interface);
+		if (str) {
+			end = strstr(str, "\n");
+			if (end)
+				*end = 0;
+			APPEND_STAT_CSV("proc.net.dev", "%s", str);
+		}
+	}
+
+	str = getFile("/proc/loadavg", buffer, sizeof(buffer));
+	if (str) {
+		end = strstr(str, "\n");
+		if (end)
+			*end = 0;
+		APPEND_STAT_CSV("proc.loadavg", "%s", str);

Review comment:
       Agreed, just trying to leave as much of the original functionality alone as possible.  We can look at refactoring after this if we want

##########
File path: traffic_server/plugins/astats_over_http/astats_over_http.c
##########
@@ -193,14 +199,25 @@ stats_add_data_to_resp_buffer(const char *s, stats_state *my_state) {
 	return s_len;
 }
 
-static const char RESP_HEADER[] = "HTTP/1.0 200 Ok\r\nContent-Type: text/javascript\r\nCache-Control: no-cache\r\n\r\n";
+static const char RESP_HEADER_JSON[] = "HTTP/1.0 200 Ok\r\nContent-Type: text/json\r\nCache-Control: no-cache\r\n\r\n";
+static const char RESP_HEADER_CSV[] = "HTTP/1.0 200 Ok\r\nContent-Type: text/csv\r\nCache-Control: no-cache\r\n\r\n";
 
 static void
 stats_process_read(TSCont contp, TSEvent event, stats_state *my_state) {
 	TSDebug(PLUGIN_TAG, "stats_process_read(%d)", event);
 
 	if (event == TS_EVENT_VCONN_READ_READY) {
-		my_state->output_bytes = stats_add_data_to_resp_buffer(RESP_HEADER, my_state);
+		switch (my_state->output) {
+			case JSON_OUTPUT:
+				my_state->output_bytes = stats_add_data_to_resp_buffer(RESP_HEADER_JSON, my_state);
+				break;
+			case CSV_OUTPUT:
+				my_state->output_bytes = stats_add_data_to_resp_buffer(RESP_HEADER_CSV, my_state);
+				break;
+			default:
+				TSError("stats_process_read: Unknown output format\n");
+				break;

Review comment:
       It also shouldnt even change based on client requests since everywhere else it will default to json output, on startup, on request with anything other than `text/csv`, so if we manage to get here and its some unknown type some memory probably got stomped on or something very very bad happened

##########
File path: traffic_server/plugins/astats_over_http/astats_over_http.c
##########
@@ -343,25 +433,54 @@ static void json_out_stats(stats_state *my_state) {
 
 	if (my_state->recordTypes & SYSTEM_RECORD_TYPE) {
 		APPEND(",\n \"system\": {\n");
-		appendSystemState(my_state);
-		APPEND_STAT("configReloadRequests", "%d", configReloadRequests);
-		APPEND_STAT("lastReloadRequest", "%" PRIu64, lastReloadRequest);
-		APPEND_STAT("configReloads", "%d", configReloads);
-		APPEND_STAT("lastReload", "%" PRIu64, lastReload);
-		APPEND_STAT("astatsLoad", "%" PRIu64, astatsLoad);
+		appendSystemStateJson(my_state);
+		APPEND_STAT_JSON("configReloadRequests", "%d", configReloadRequests);
+		APPEND_STAT_JSON("lastReloadRequest", "%" PRIu64, lastReloadRequest);
+		APPEND_STAT_JSON("configReloads", "%d", configReloads);
+		APPEND_STAT_JSON("lastReload", "%" PRIu64, lastReload);
+		APPEND_STAT_JSON("astatsLoad", "%" PRIu64, astatsLoad);
 		APPEND("\"something\": \"here\"");
 		APPEND("\n  }");
 	}
 
 	APPEND("\n}\n");
 }
 
+static void csv_out_stats(stats_state *my_state) {
+	const char *version;
+	TSDebug(PLUGIN_TAG, "recordTypes: '0x%x'", my_state->recordTypes);

Review comment:
       It can, but leaving it as 0x%x preserves the existing syntax used in the rest of the file




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org