Refactor metric print (#134) (#141)

* update twemcache admin * update slimcache * update pingserver
twitter · Feb 26, 2017 · 23ba44e · 23ba44e
1 parent eb1d4b6
commit 23ba44e
Show file tree

Hide file tree

Showing 19 changed files with 76 additions and 199 deletions.
diff --git a/src/protocol/admin/CMakeLists.txt b/src/protocol/admin/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(SOURCE
+    format.c
     compose.c
     parse.c
     request.c

diff --git a/src/protocol/admin/admin_include.h b/src/protocol/admin/admin_include.h
@@ -1,4 +1,5 @@
 #include "compose.h"
+#include "format.h"
 #include "parse.h"
 #include "process.h"
 #include "request.h"

diff --git a/src/protocol/admin/format.c b/src/protocol/admin/format.c
@@ -0,0 +1,18 @@
+#include "format.h"
+
+#include <cc_print.h>
+
+size_t
+print_stats(char *buf, size_t cap, struct metric *metrics, unsigned int nmetric)
+{
+    size_t offset = 0;
+
+    /* TODO: report error if running out of space in buf */
+    for (int i = 0; i < nmetric; ++i) {
+        offset += metric_print(buf + offset, cap - offset, METRIC_PRINT_FMT,
+                &metrics[i]);
+    }
+    offset += cc_scnprintf(buf + offset, cap - offset, METRIC_END);
+
+    return offset;
+}
diff --git a/src/protocol/admin/format.h b/src/protocol/admin/format.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include <cc_metric.h>
+
+#define METRIC_PRINT_FMT "STAT %s %s\r\n"
+#define METRIC_PRINT_LEN 64 /* > 5("STAT ") + 32 (name) + 20 (value) + CRLF */
+#define METRIC_DESCRIBE_FMT "%33s %15s %s\r\n"
+#define METRIC_DESCRIBE_LEN 120 /* 34 (name) + 16 (type) + 68 (description) + CRLF */
+#define METRIC_END "END\r\n"
+#define METRIC_END_LEN (sizeof(METRIC_END) - 1)
+
+#define VERSION_PRINTED "VERSION " VERSION_STRING "\r\n"
+
+size_t print_stats(char *buf, size_t cap, struct metric *metrics, unsigned int nmetric);
diff --git a/src/server/pingserver/admin/process.c b/src/server/pingserver/admin/process.c
@@ -4,43 +4,27 @@
 #include "util/procinfo.h"
 
 #include <cc_mm.h>
-#include <cc_print.h>
 
 #define PINGSERVER_ADMIN_MODULE_NAME "pingserver::admin"
 
-#define METRIC_PRINT_FMT "STAT %s %s\r\n"
-#define METRIC_PRINT_LEN 64 /* > 5("STAT ") + 32 (name) + 20 (value) + CRLF */
-#define METRIC_DESCRIBE_FMT "%33s %15s %s\r\n"
-#define METRIC_DESCRIBE_LEN 120 /* 34 (name) + 16 (type) + 68 (description) + CRLF */
-#define METRIC_FOOTER CRLF
-#define METRIC_END "END\r\n"
-#define METRIC_END_LEN (sizeof(METRIC_END) - 1)
-
-#define VERSION_PRINT_FMT "VERSION %s\r\n"
-#define VERSION_PRINT_LEN 30
-
 extern struct stats stats;
 extern unsigned int nmetric;
 
 static bool admin_init = false;
-static admin_process_metrics_st *admin_metrics = NULL;
-static char *stats_buf = NULL;
-static char version_buf[VERSION_PRINT_LEN];
-static size_t stats_len;
+static char *buf = NULL;
+static size_t cap;
 
 void
-admin_process_setup(admin_process_metrics_st *metrics)
+admin_process_setup(void)
 {
     log_info("set up the %s module", PINGSERVER_ADMIN_MODULE_NAME);
     if (admin_init) {
         log_warn("%s has already been setup, overwrite",
                  PINGSERVER_ADMIN_MODULE_NAME);
     }
 
-    admin_metrics = metrics;
-
-    stats_len = METRIC_PRINT_LEN * nmetric;
-    stats_buf = cc_alloc(stats_len + METRIC_END_LEN);
+    cap = METRIC_PRINT_LEN * nmetric + METRIC_END_LEN;
+    buf = cc_alloc(cap);
     /* TODO: check return status of cc_alloc */
 
     admin_init = true;
@@ -54,49 +38,28 @@ admin_process_teardown(void)
         log_warn("%s has never been setup", PINGSERVER_ADMIN_MODULE_NAME);
     }
 
-    admin_metrics = NULL;
     admin_init = false;
 }
 
 static void
 _admin_stats(struct response *rsp, struct request *req)
 {
-    size_t offset = 0;
-    struct metric *metrics = (struct metric *)&stats;
-
-    INCR(admin_metrics, stats);
-
     procinfo_update();
-    for (int i = 0; i < nmetric; ++i) {
-        offset += metric_print(stats_buf + offset, stats_len - offset,
-                METRIC_PRINT_FMT, &metrics[i]);
-    }
-    strcpy(stats_buf + offset, METRIC_END);
-
-    rsp->type = RSP_GENERIC;
-    rsp->data.data = stats_buf;
-    rsp->data.len = offset + METRIC_END_LEN;
-}
-
-static void
-_admin_version(struct response *rsp, struct request *req)
-{
-    INCR(admin_metrics, version);
-
-    rsp->type = RSP_GENERIC;
-    cc_snprintf(version_buf, VERSION_PRINT_LEN, VERSION_PRINT_FMT, VERSION_STRING);
-    rsp->data = str2bstr(version_buf);
+    rsp->data.data = buf;
+    rsp->data.len = print_stats(buf, cap, (struct metric *)&stats, nmetric);
 }
 
 void
 admin_process_request(struct response *rsp, struct request *req)
 {
+    rsp->type = RSP_GENERIC;
+
     switch (req->type) {
     case REQ_STATS:
         _admin_stats(rsp, req);
         break;
     case REQ_VERSION:
-        _admin_version(rsp, req);
+        rsp->data = str2bstr(VERSION_PRINTED);
         break;
     default:
         rsp->type = RSP_INVALID;

diff --git a/src/server/pingserver/admin/process.h b/src/server/pingserver/admin/process.h
@@ -1,16 +1,4 @@
 #pragma once
 
-#include <cc_metric.h>
-
-/*          name                        type            description */
-#define ADMIN_PROCESS_METRIC(ACTION)                                    \
-    ACTION( stats,             METRIC_COUNTER, "# stats requests"      )\
-    ACTION( stats_ex,          METRIC_COUNTER, "# stats errors"        )\
-    ACTION( version,           METRIC_COUNTER, "# version requests"    )
-
-typedef struct {
-    ADMIN_PROCESS_METRIC(METRIC_DECLARE)
-} admin_process_metrics_st;
-
-void admin_process_setup(admin_process_metrics_st *metrics);
+void admin_process_setup(void);
 void admin_process_teardown(void);
diff --git a/src/server/pingserver/main.c b/src/server/pingserver/main.c
@@ -112,7 +112,7 @@ setup(void)
     procinfo_setup(&stats.procinfo);
     parse_setup(&stats.parse_req, NULL);
     compose_setup(NULL, &stats.compose_rsp);
-    admin_process_setup(&stats.admin_process);
+    admin_process_setup();
     core_setup(&setting.admin, &setting.server, &setting.worker,
             &stats.server, &stats.worker);
 

diff --git a/src/server/pingserver/stats.c b/src/server/pingserver/stats.c
@@ -2,7 +2,6 @@
 
 struct stats stats = {
     { PROCINFO_METRIC(METRIC_INIT)      },
-    { ADMIN_PROCESS_METRIC(METRIC_INIT) },
     { PARSE_REQ_METRIC(METRIC_INIT)     },
     { COMPOSE_RSP_METRIC(METRIC_INIT)   },
     { CORE_SERVER_METRIC(METRIC_INIT)   },

diff --git a/src/server/pingserver/stats.h b/src/server/pingserver/stats.h
@@ -1,6 +1,5 @@
 #pragma once
 
-#include "admin/process.h"
 #include "data/process.h"
 
 #include "core/core.h"
@@ -18,7 +17,6 @@ struct stats {
     /* perf info */
     procinfo_metrics_st         procinfo;
     /* application modules */
-    admin_process_metrics_st    admin_process;
     parse_req_metrics_st        parse_req;
     compose_rsp_metrics_st      compose_rsp;
     server_metrics_st           server;

diff --git a/src/server/slimcache/admin/process.c b/src/server/slimcache/admin/process.c
@@ -4,43 +4,27 @@
 #include "util/procinfo.h"
 
 #include <cc_mm.h>
-#include <cc_print.h>
 
 #define SLIMCACHE_ADMIN_MODULE_NAME "slimcache::admin"
 
-#define METRIC_PRINT_FMT "STAT %s %s\r\n"
-#define METRIC_PRINT_LEN 64 /* > 5("STAT ") + 32 (name) + 20 (value) + CRLF */
-#define METRIC_DESCRIBE_FMT "%33s %15s %s\r\n"
-#define METRIC_DESCRIBE_LEN 120 /* 34 (name) + 16 (type) + 68 (description) + CRLF */
-#define METRIC_FOOTER CRLF
-#define METRIC_END "END\r\n"
-#define METRIC_END_LEN (sizeof(METRIC_END) - 1)
-
-#define VERSION_PRINT_FMT "VERSION %s\r\n"
-#define VERSION_PRINT_LEN 30
-
 extern struct stats stats;
 extern unsigned int nmetric;
 
 static bool admin_init = false;
-static admin_process_metrics_st *admin_metrics = NULL;
-static char *stats_buf = NULL;
-static char version_buf[VERSION_PRINT_LEN];
-static size_t stats_len;
+static char *buf = NULL;
+static size_t cap;
 
 void
-admin_process_setup(admin_process_metrics_st *metrics)
+admin_process_setup(void)
 {
     log_info("set up the %s module", SLIMCACHE_ADMIN_MODULE_NAME);
     if (admin_init) {
         log_warn("%s has already been setup, overwrite",
                  SLIMCACHE_ADMIN_MODULE_NAME);
     }
 
-    admin_metrics = metrics;
-
-    stats_len = METRIC_PRINT_LEN * nmetric;
-    stats_buf = cc_alloc(stats_len + METRIC_END_LEN);
+    cap = METRIC_PRINT_LEN * nmetric + METRIC_END_LEN;
+    buf = cc_alloc(cap);
     /* TODO: check return status of cc_alloc */
 
     admin_init = true;
@@ -54,49 +38,28 @@ admin_process_teardown(void)
         log_warn("%s has never been setup", SLIMCACHE_ADMIN_MODULE_NAME);
     }
 
-    admin_metrics = NULL;
     admin_init = false;
 }
 
 static void
 _admin_stats(struct response *rsp, struct request *req)
 {
-    size_t offset = 0;
-    struct metric *metrics = (struct metric *)&stats;
-
-    INCR(admin_metrics, stats);
-
     procinfo_update();
-    for (int i = 0; i < nmetric; ++i) {
-        offset += metric_print(stats_buf + offset, stats_len - offset,
-                METRIC_PRINT_FMT, &metrics[i]);
-    }
-    strcpy(stats_buf + offset, METRIC_END);
-
-    rsp->type = RSP_GENERIC;
-    rsp->data.data = stats_buf;
-    rsp->data.len = offset + METRIC_END_LEN;
-}
-
-static void
-_admin_version(struct response *rsp, struct request *req)
-{
-    INCR(admin_metrics, version);
-
-    rsp->type = RSP_GENERIC;
-    cc_snprintf(version_buf, VERSION_PRINT_LEN, VERSION_PRINT_FMT, VERSION_STRING);
-    rsp->data = str2bstr(version_buf);
+    rsp->data.data = buf;
+    rsp->data.len = print_stats(buf, cap, (struct metric *)&stats, nmetric);
 }
 
 void
 admin_process_request(struct response *rsp, struct request *req)
 {
+    rsp->type = RSP_GENERIC;
+
     switch (req->type) {
     case REQ_STATS:
         _admin_stats(rsp, req);
         break;
     case REQ_VERSION:
-        _admin_version(rsp, req);
+        rsp->data = str2bstr(VERSION_PRINTED);
         break;
     default:
         rsp->type = RSP_INVALID;

diff --git a/src/server/slimcache/admin/process.h b/src/server/slimcache/admin/process.h
@@ -1,16 +1,4 @@
 #pragma once
 
-#include <cc_metric.h>
-
-/*          name                        type            description */
-#define ADMIN_PROCESS_METRIC(ACTION)                                    \
-    ACTION( stats,             METRIC_COUNTER, "# stats requests"      )\
-    ACTION( stats_ex,          METRIC_COUNTER, "# stats errors"        )\
-    ACTION( version,           METRIC_COUNTER, "# version requests"    )
-
-typedef struct {
-    ADMIN_PROCESS_METRIC(METRIC_DECLARE)
-} admin_process_metrics_st;
-
-void admin_process_setup(admin_process_metrics_st *metrics);
+void admin_process_setup(void);
 void admin_process_teardown(void);
diff --git a/src/server/slimcache/main.c b/src/server/slimcache/main.c
@@ -122,7 +122,7 @@ setup(void)
     klog_setup(&setting.klog, &stats.klog);
     cuckoo_setup(&setting.cuckoo, &stats.cuckoo);
     process_setup(&setting.process, &stats.process);
-    admin_process_setup(&stats.admin_process);
+    admin_process_setup();
     core_setup(&setting.admin, &setting.server, &setting.worker,
             &stats.server, &stats.worker);
 

diff --git a/src/server/slimcache/stats.c b/src/server/slimcache/stats.c
@@ -3,7 +3,6 @@
 struct stats stats = {
     { PROCINFO_METRIC(METRIC_INIT)      },
     { PROCESS_METRIC(METRIC_INIT)       },
-    { ADMIN_PROCESS_METRIC(METRIC_INIT) },
     { PARSE_REQ_METRIC(METRIC_INIT)     },
     { COMPOSE_RSP_METRIC(METRIC_INIT)   },
     { KLOG_METRIC(METRIC_INIT)          },

diff --git a/src/server/slimcache/stats.h b/src/server/slimcache/stats.h
@@ -1,6 +1,5 @@
 #pragma once
 
-#include "admin/process.h"
 #include "data/process.h"
 
 #include "core/core.h"
@@ -19,7 +18,6 @@ struct stats {
     procinfo_metrics_st         procinfo;
     /* application modules */
     process_metrics_st          process;
-    admin_process_metrics_st    admin_process;
     parse_req_metrics_st        parse_req;
     compose_rsp_metrics_st      compose_rsp;
     klog_metrics_st             klog;