From 1ef06e6285364a16517cb9d7ac8ce6ab6c1d35c4 Mon Sep 17 00:00:00 2001 From: Hongyu Chen Date: Wed, 4 Feb 2015 10:36:04 -0800 Subject: [PATCH 01/10] V0 implementation of census_get_active_ops(). --- src/core/statistics/census_tracing.c | 64 +++++++++++++++++-------- src/core/statistics/census_tracing.h | 33 ++++++++++++- test/core/statistics/trace_test.c | 71 ++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 21 deletions(-) diff --git a/src/core/statistics/census_tracing.c b/src/core/statistics/census_tracing.c index 3c4ba66f5f4..ddf925cc47b 100644 --- a/src/core/statistics/census_tracing.c +++ b/src/core/statistics/census_tracing.c @@ -32,35 +32,19 @@ */ #include "src/core/statistics/census_interface.h" +#include "src/core/statistics/census_tracing.h" #include #include -#include "src/core/statistics/census_rpc_stats.h" #include "src/core/statistics/hash_table.h" #include "src/core/support/string.h" #include #include #include #include -#include - -/* Struct for a trace annotation. */ -typedef struct annotation { - gpr_timespec ts; /* timestamp of the annotation */ - char txt[CENSUS_MAX_ANNOTATION_LENGTH + 1]; /* actual txt annotation */ - struct annotation* next; -} annotation; - -typedef struct trace_obj { - census_op_id id; - gpr_timespec ts; - census_rpc_stats rpc_stats; - char* method; - annotation* annotations; -} trace_obj; - -static void trace_obj_destroy(trace_obj* obj) { + +void trace_obj_destroy(trace_obj* obj) { annotation* p = obj->annotations; while (p != NULL) { annotation* next = p->next; @@ -207,3 +191,45 @@ trace_obj* census_get_trace_obj_locked(census_op_id op_id) { const char* census_get_trace_method_name(const trace_obj* trace) { return (const char*)trace->method; } + +static annotation* dup_annotation_chain(annotation* from) { + annotation* to = NULL; + if (from != NULL) { + to = gpr_malloc(sizeof(annotation)); + memcpy(to, from, sizeof(annotation)); + to->next = dup_annotation_chain(from->next); + } + return to; +} + +static trace_obj* trace_obj_dup(trace_obj* from) { + trace_obj* to = NULL; + GPR_ASSERT(from != NULL); + to = gpr_malloc(sizeof(trace_obj)); + to->id = from->id; + to->ts = from->ts; + to->rpc_stats = from->rpc_stats; + to->method = gpr_strdup(from->method); + to->annotations = dup_annotation_chain(from->annotations); + return to; +} + +trace_obj** census_get_active_ops(int* num_active_ops) { + trace_obj** ret = NULL; + gpr_mu_lock(&g_mu); + if (g_trace_store != NULL) { + size_t n = 0; + census_ht_kv* all_kvs = census_ht_get_all_elements(g_trace_store, &n); + *num_active_ops = n; + if (n != 0 ) { + size_t i = 0; + ret = gpr_malloc(sizeof(trace_obj *) * n); + for (i = 0; i < n; i++) { + ret[i] = trace_obj_dup((trace_obj*)all_kvs[i].v); + } + } + gpr_free(all_kvs); + } + gpr_mu_unlock(&g_mu); + return ret; +} diff --git a/src/core/statistics/census_tracing.h b/src/core/statistics/census_tracing.h index f356c9424d5..58333f7f6b8 100644 --- a/src/core/statistics/census_tracing.h +++ b/src/core/statistics/census_tracing.h @@ -34,12 +34,35 @@ #ifndef __GRPC_INTERNAL_STATISTICS_CENSUS_TRACING_H_ #define __GRPC_INTERNAL_STATISTICS_CENSUS_TRACING_H_ +#include +#include "src/core/statistics/census_rpc_stats.h" + +/* WARNING: The data structures and APIs provided by this file are for GRPC + library's internal use ONLY. They might be changed in backward-incompatible + ways and are not subject to any deprecation policy. + They are not recommended for external use. + */ #ifdef __cplusplus extern "C" { #endif -/* Opaque structure for trace object */ -typedef struct trace_obj trace_obj; +/* Struct for a trace annotation. */ +typedef struct annotation { + gpr_timespec ts; /* timestamp of the annotation */ + char txt[CENSUS_MAX_ANNOTATION_LENGTH + 1]; /* actual txt annotation */ + struct annotation* next; +} annotation; + +typedef struct trace_obj { + census_op_id id; + gpr_timespec ts; + census_rpc_stats rpc_stats; + char* method; + annotation* annotations; +} trace_obj; + +/* Deletes trace object. */ +void trace_obj_destroy(trace_obj* obj); /* Initializes trace store. This function is thread safe. */ void census_tracing_init(void); @@ -60,6 +83,12 @@ void census_internal_unlock_trace_store(void); /* Gets method tag name associated with the input trace object. */ const char* census_get_trace_method_name(const trace_obj* trace); +/* Returns an array of pointers to trace objects of currently active operations + and fills in number of active operations. Returns NULL if there's no active + operations. + Caller owns the returned objects. */ +trace_obj** census_get_active_ops(int* num_active_ops); + #ifdef __cplusplus } #endif diff --git a/test/core/statistics/trace_test.c b/test/core/statistics/trace_test.c index 6eafcf14568..18edfb433aa 100644 --- a/test/core/statistics/trace_test.c +++ b/test/core/statistics/trace_test.c @@ -32,10 +32,12 @@ */ #include +#include #include "src/core/statistics/census_interface.h" #include "src/core/statistics/census_tracing.h" #include "src/core/statistics/census_tracing.h" +#include #include #include #include @@ -172,6 +174,74 @@ static void test_trace_print(void) { census_tracing_shutdown(); } +/* Returns 1 if two ids are equal, otherwise returns 0. */ +static int ids_equal(census_op_id id1, census_op_id id2) { + return (id1.upper == id2.upper) && (id1.lower == id2.lower); +} + +static void test_get_active_ops(void) { + census_op_id id_1, id_2, id_3; + trace_obj** active_ops; + const char* annotation_txt[] = {"annotation 1", "a2"}; + int i = 0; + int n = 0; + + gpr_log(GPR_INFO, "test_get_active_ops"); + census_tracing_init(); + /* No active ops before calling start_op(). */ + active_ops = census_get_active_ops(&n); + GPR_ASSERT(active_ops == NULL); + GPR_ASSERT(n == 0); + + /* Starts one op */ + id_1 = census_tracing_start_op(); + census_add_method_tag(id_1, "foo_1"); + active_ops = census_get_active_ops(&n); + GPR_ASSERT(active_ops != NULL); + GPR_ASSERT(n == 1); + GPR_ASSERT(ids_equal(active_ops[0]->id, id_1)); + trace_obj_destroy(active_ops[0]); + gpr_free(active_ops); + active_ops = NULL; + + /* Start the second and the third ops */ + id_2 = census_tracing_start_op(); + census_add_method_tag(id_2, "foo_2"); + id_3 = census_tracing_start_op(); + census_add_method_tag(id_3, "foo_3"); + + active_ops = census_get_active_ops(&n); + GPR_ASSERT(n == 3); + for (i = 0; i < 3; i++) { + trace_obj_destroy(active_ops[i]); + } + gpr_free(active_ops); + active_ops = NULL; + + /* End the second op and add annotations to the third ops*/ + census_tracing_end_op(id_2); + census_tracing_print(id_3, annotation_txt[0]); + census_tracing_print(id_3, annotation_txt[1]); + + active_ops = census_get_active_ops(&n); + GPR_ASSERT(active_ops != NULL); + GPR_ASSERT(n == 2); + for (i = 0; i < 2; i++) { + trace_obj_destroy(active_ops[i]); + } + gpr_free(active_ops); + active_ops = NULL; + + /* End all ops. */ + census_tracing_end_op(id_1); + census_tracing_end_op(id_3); + active_ops = census_get_active_ops(&n); + GPR_ASSERT(active_ops == NULL); + GPR_ASSERT(n == 0); + + census_tracing_shutdown(); +} + int main(int argc, char** argv) { grpc_test_init(argc, argv); test_init_shutdown(); @@ -180,5 +250,6 @@ int main(int argc, char** argv) { test_concurrency(); test_add_method_tag_to_unknown_op_id(); test_trace_print(); + test_get_active_ops(); return 0; } From 46c3be0156c60ed2767aeaf00cd0e46e27c8880c Mon Sep 17 00:00:00 2001 From: Alistair Veitch Date: Thu, 5 Feb 2015 11:41:01 -0800 Subject: [PATCH 02/10] Update census_tracing.h --- src/core/statistics/census_tracing.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/statistics/census_tracing.h b/src/core/statistics/census_tracing.h index 58333f7f6b8..ca1736630e8 100644 --- a/src/core/statistics/census_tracing.h +++ b/src/core/statistics/census_tracing.h @@ -80,11 +80,11 @@ trace_obj* census_get_trace_obj_locked(census_op_id op_id); void census_internal_lock_trace_store(void); void census_internal_unlock_trace_store(void); -/* Gets method tag name associated with the input trace object. */ +/* Gets method name associated with the input trace object. */ const char* census_get_trace_method_name(const trace_obj* trace); /* Returns an array of pointers to trace objects of currently active operations - and fills in number of active operations. Returns NULL if there's no active + and fills in number of active operations. Returns NULL if there are no active operations. Caller owns the returned objects. */ trace_obj** census_get_active_ops(int* num_active_ops); From c0ef1ded5522543e517144843dbd5869beeb2d48 Mon Sep 17 00:00:00 2001 From: Alistair Veitch Date: Thu, 5 Feb 2015 11:53:27 -0800 Subject: [PATCH 03/10] Update trace_test.c --- test/core/statistics/trace_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/statistics/trace_test.c b/test/core/statistics/trace_test.c index 18edfb433aa..220e0da0225 100644 --- a/test/core/statistics/trace_test.c +++ b/test/core/statistics/trace_test.c @@ -204,7 +204,7 @@ static void test_get_active_ops(void) { gpr_free(active_ops); active_ops = NULL; - /* Start the second and the third ops */ + /* Start the second and third ops */ id_2 = census_tracing_start_op(); census_add_method_tag(id_2, "foo_2"); id_3 = census_tracing_start_op(); From b5674540ed07fd0a2c731b6cc34915de3b6e74e5 Mon Sep 17 00:00:00 2001 From: Hongyu Chen Date: Thu, 5 Feb 2015 12:18:05 -0800 Subject: [PATCH 04/10] Update following a-vietch's comments --- src/core/statistics/census_tracing.c | 29 +++++++++++++++++----------- src/core/statistics/census_tracing.h | 8 ++++---- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/core/statistics/census_tracing.c b/src/core/statistics/census_tracing.c index ddf925cc47b..4c030152e6e 100644 --- a/src/core/statistics/census_tracing.c +++ b/src/core/statistics/census_tracing.c @@ -45,9 +45,9 @@ #include void trace_obj_destroy(trace_obj* obj) { - annotation* p = obj->annotations; + trace_annotation* p = obj->annotations; while (p != NULL) { - annotation* next = p->next; + trace_annotation* next = p->next; gpr_free(p); p = next; } @@ -119,7 +119,7 @@ void census_tracing_print(census_op_id op_id, const char* anno_txt) { gpr_mu_lock(&g_mu); trace = census_ht_find(g_trace_store, op_id_as_key(&op_id)); if (trace != NULL) { - annotation* anno = gpr_malloc(sizeof(annotation)); + trace_annotation* anno = gpr_malloc(sizeof(trace_annotation)); anno->ts = gpr_now(); { char* d = anno->txt; @@ -189,15 +189,22 @@ trace_obj* census_get_trace_obj_locked(census_op_id op_id) { } const char* census_get_trace_method_name(const trace_obj* trace) { - return (const char*)trace->method; + return trace->method; } -static annotation* dup_annotation_chain(annotation* from) { - annotation* to = NULL; - if (from != NULL) { - to = gpr_malloc(sizeof(annotation)); - memcpy(to, from, sizeof(annotation)); - to->next = dup_annotation_chain(from->next); +static trace_annotation* dup_annotation_chain(trace_annotation* from) { + trace_annotation *to = NULL, *prev = NULL; + for (; from != NULL; from = from->next) { + trace_annotation* tmp = gpr_malloc(sizeof(trace_annotation)); + memcpy(tmp, from, sizeof(trace_annotation)); + tmp->next = NULL; + if (to == NULL) { + to = tmp; + prev = to; + } else { + prev->next = tmp; + prev = tmp; + } } return to; } @@ -220,7 +227,7 @@ trace_obj** census_get_active_ops(int* num_active_ops) { if (g_trace_store != NULL) { size_t n = 0; census_ht_kv* all_kvs = census_ht_get_all_elements(g_trace_store, &n); - *num_active_ops = n; + *num_active_ops = (int)n; if (n != 0 ) { size_t i = 0; ret = gpr_malloc(sizeof(trace_obj *) * n); diff --git a/src/core/statistics/census_tracing.h b/src/core/statistics/census_tracing.h index 58333f7f6b8..fa0e087053f 100644 --- a/src/core/statistics/census_tracing.h +++ b/src/core/statistics/census_tracing.h @@ -47,18 +47,18 @@ extern "C" { #endif /* Struct for a trace annotation. */ -typedef struct annotation { +typedef struct trace_annotation { gpr_timespec ts; /* timestamp of the annotation */ char txt[CENSUS_MAX_ANNOTATION_LENGTH + 1]; /* actual txt annotation */ - struct annotation* next; -} annotation; + struct trace_annotation* next; +} trace_annotation; typedef struct trace_obj { census_op_id id; gpr_timespec ts; census_rpc_stats rpc_stats; char* method; - annotation* annotations; + trace_annotation* annotations; } trace_obj; /* Deletes trace object. */ From 1ed726c6f5ad5e08257986930bf732713aecdcf7 Mon Sep 17 00:00:00 2001 From: Hongyu Chen Date: Thu, 5 Feb 2015 15:49:09 -0800 Subject: [PATCH 05/10] More update on dup_annotation_list following review comments. --- src/core/statistics/census_tracing.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/core/statistics/census_tracing.c b/src/core/statistics/census_tracing.c index 4c030152e6e..bcd0ee4787b 100644 --- a/src/core/statistics/census_tracing.c +++ b/src/core/statistics/census_tracing.c @@ -193,20 +193,14 @@ const char* census_get_trace_method_name(const trace_obj* trace) { } static trace_annotation* dup_annotation_chain(trace_annotation* from) { - trace_annotation *to = NULL, *prev = NULL; + trace_annotation *ret = NULL; + trace_annotation **to = &ret; for (; from != NULL; from = from->next) { - trace_annotation* tmp = gpr_malloc(sizeof(trace_annotation)); - memcpy(tmp, from, sizeof(trace_annotation)); - tmp->next = NULL; - if (to == NULL) { - to = tmp; - prev = to; - } else { - prev->next = tmp; - prev = tmp; - } + *to = gpr_malloc(sizeof(trace_annotation)); + memcpy(*to, from, sizeof(trace_annotation)); + to = &(*to)->next; } - return to; + return ret; } static trace_obj* trace_obj_dup(trace_obj* from) { From 2bfbfe8bfad815f29f70fc939bd6fdb827c6295f Mon Sep 17 00:00:00 2001 From: Hongyu Chen Date: Thu, 5 Feb 2015 16:12:26 -0800 Subject: [PATCH 06/10] prefix struct & functions in census_tracing.h per ctiller's suggestion. --- src/core/statistics/census_rpc_stats.c | 2 +- src/core/statistics/census_tracing.c | 53 ++++++++++++++------------ src/core/statistics/census_tracing.h | 20 +++++----- test/core/statistics/trace_test.c | 10 ++--- 4 files changed, 44 insertions(+), 41 deletions(-) diff --git a/src/core/statistics/census_rpc_stats.c b/src/core/statistics/census_rpc_stats.c index 785c091debc..fc66cb951fe 100644 --- a/src/core/statistics/census_rpc_stats.c +++ b/src/core/statistics/census_rpc_stats.c @@ -141,7 +141,7 @@ static void record_stats(census_ht* store, census_op_id op_id, const census_rpc_stats* stats) { gpr_mu_lock(&g_mu); if (store != NULL) { - trace_obj* trace = NULL; + census_trace_obj* trace = NULL; census_internal_lock_trace_store(); trace = census_get_trace_obj_locked(op_id); if (trace != NULL) { diff --git a/src/core/statistics/census_tracing.c b/src/core/statistics/census_tracing.c index bcd0ee4787b..8b98323e64c 100644 --- a/src/core/statistics/census_tracing.c +++ b/src/core/statistics/census_tracing.c @@ -44,10 +44,10 @@ #include #include -void trace_obj_destroy(trace_obj* obj) { - trace_annotation* p = obj->annotations; +void census_trace_obj_destroy(census_trace_obj* obj) { + census_trace_annotation* p = obj->annotations; while (p != NULL) { - trace_annotation* next = p->next; + census_trace_annotation* next = p->next; gpr_free(p); p = next; } @@ -55,7 +55,9 @@ void trace_obj_destroy(trace_obj* obj) { gpr_free(obj); } -static void delete_trace_obj(void* obj) { trace_obj_destroy((trace_obj*)obj); } +static void delete_trace_obj(void* obj) { + census_trace_obj_destroy((census_trace_obj*)obj); +} static const census_ht_option ht_opt = { CENSUS_HT_UINT64 /* key type*/, 571 /* n_of_buckets */, NULL /* hash */, @@ -87,8 +89,8 @@ static void init_mutex_once(void) { census_op_id census_tracing_start_op(void) { gpr_mu_lock(&g_mu); { - trace_obj* ret = (trace_obj*)gpr_malloc(sizeof(trace_obj)); - memset(ret, 0, sizeof(trace_obj)); + census_trace_obj* ret = gpr_malloc(sizeof(census_trace_obj)); + memset(ret, 0, sizeof(census_trace_obj)); g_id++; memcpy(&ret->id, &g_id, sizeof(census_op_id)); ret->rpc_stats.cnt = 1; @@ -102,7 +104,7 @@ census_op_id census_tracing_start_op(void) { int census_add_method_tag(census_op_id op_id, const char* method) { int ret = 0; - trace_obj* trace = NULL; + census_trace_obj* trace = NULL; gpr_mu_lock(&g_mu); trace = census_ht_find(g_trace_store, op_id_as_key(&op_id)); if (trace == NULL) { @@ -115,11 +117,11 @@ int census_add_method_tag(census_op_id op_id, const char* method) { } void census_tracing_print(census_op_id op_id, const char* anno_txt) { - trace_obj* trace = NULL; + census_trace_obj* trace = NULL; gpr_mu_lock(&g_mu); trace = census_ht_find(g_trace_store, op_id_as_key(&op_id)); if (trace != NULL) { - trace_annotation* anno = gpr_malloc(sizeof(trace_annotation)); + census_trace_annotation* anno = gpr_malloc(sizeof(census_trace_annotation)); anno->ts = gpr_now(); { char* d = anno->txt; @@ -137,7 +139,7 @@ void census_tracing_print(census_op_id op_id, const char* anno_txt) { } void census_tracing_end_op(census_op_id op_id) { - trace_obj* trace = NULL; + census_trace_obj* trace = NULL; gpr_mu_lock(&g_mu); trace = census_ht_find(g_trace_store, op_id_as_key(&op_id)); if (trace != NULL) { @@ -180,33 +182,34 @@ void census_internal_lock_trace_store(void) { gpr_mu_lock(&g_mu); } void census_internal_unlock_trace_store(void) { gpr_mu_unlock(&g_mu); } -trace_obj* census_get_trace_obj_locked(census_op_id op_id) { +census_trace_obj* census_get_trace_obj_locked(census_op_id op_id) { if (g_trace_store == NULL) { gpr_log(GPR_ERROR, "Census trace store is not initialized."); return NULL; } - return (trace_obj*)census_ht_find(g_trace_store, op_id_as_key(&op_id)); + return (census_trace_obj*)census_ht_find(g_trace_store, op_id_as_key(&op_id)); } -const char* census_get_trace_method_name(const trace_obj* trace) { +const char* census_get_trace_method_name(const census_trace_obj* trace) { return trace->method; } -static trace_annotation* dup_annotation_chain(trace_annotation* from) { - trace_annotation *ret = NULL; - trace_annotation **to = &ret; +static census_trace_annotation* dup_annotation_chain( + census_trace_annotation* from) { + census_trace_annotation *ret = NULL; + census_trace_annotation **to = &ret; for (; from != NULL; from = from->next) { - *to = gpr_malloc(sizeof(trace_annotation)); - memcpy(*to, from, sizeof(trace_annotation)); + *to = gpr_malloc(sizeof(census_trace_annotation)); + memcpy(*to, from, sizeof(census_trace_annotation)); to = &(*to)->next; } return ret; } -static trace_obj* trace_obj_dup(trace_obj* from) { - trace_obj* to = NULL; +static census_trace_obj* trace_obj_dup(census_trace_obj* from) { + census_trace_obj* to = NULL; GPR_ASSERT(from != NULL); - to = gpr_malloc(sizeof(trace_obj)); + to = gpr_malloc(sizeof(census_trace_obj)); to->id = from->id; to->ts = from->ts; to->rpc_stats = from->rpc_stats; @@ -215,8 +218,8 @@ static trace_obj* trace_obj_dup(trace_obj* from) { return to; } -trace_obj** census_get_active_ops(int* num_active_ops) { - trace_obj** ret = NULL; +census_trace_obj** census_get_active_ops(int* num_active_ops) { + census_trace_obj** ret = NULL; gpr_mu_lock(&g_mu); if (g_trace_store != NULL) { size_t n = 0; @@ -224,9 +227,9 @@ trace_obj** census_get_active_ops(int* num_active_ops) { *num_active_ops = (int)n; if (n != 0 ) { size_t i = 0; - ret = gpr_malloc(sizeof(trace_obj *) * n); + ret = gpr_malloc(sizeof(census_trace_obj *) * n); for (i = 0; i < n; i++) { - ret[i] = trace_obj_dup((trace_obj*)all_kvs[i].v); + ret[i] = trace_obj_dup((census_trace_obj*)all_kvs[i].v); } } gpr_free(all_kvs); diff --git a/src/core/statistics/census_tracing.h b/src/core/statistics/census_tracing.h index f88316f5716..88a06a4a524 100644 --- a/src/core/statistics/census_tracing.h +++ b/src/core/statistics/census_tracing.h @@ -47,22 +47,22 @@ extern "C" { #endif /* Struct for a trace annotation. */ -typedef struct trace_annotation { +typedef struct census_trace_annotation { gpr_timespec ts; /* timestamp of the annotation */ char txt[CENSUS_MAX_ANNOTATION_LENGTH + 1]; /* actual txt annotation */ - struct trace_annotation* next; -} trace_annotation; + struct census_trace_annotation* next; +} census_trace_annotation; -typedef struct trace_obj { +typedef struct census_trace_obj { census_op_id id; gpr_timespec ts; census_rpc_stats rpc_stats; char* method; - trace_annotation* annotations; -} trace_obj; + census_trace_annotation* annotations; +} census_trace_obj; /* Deletes trace object. */ -void trace_obj_destroy(trace_obj* obj); +void census_trace_obj_destroy(census_trace_obj* obj); /* Initializes trace store. This function is thread safe. */ void census_tracing_init(void); @@ -73,7 +73,7 @@ void census_tracing_shutdown(void); /* Gets trace obj corresponding to the input op_id. Returns NULL if trace store is not initialized or trace obj is not found. Requires trace store being locked before calling this function. */ -trace_obj* census_get_trace_obj_locked(census_op_id op_id); +census_trace_obj* census_get_trace_obj_locked(census_op_id op_id); /* The following two functions acquire and release the trace store global lock. They are for census internal use only. */ @@ -81,13 +81,13 @@ void census_internal_lock_trace_store(void); void census_internal_unlock_trace_store(void); /* Gets method name associated with the input trace object. */ -const char* census_get_trace_method_name(const trace_obj* trace); +const char* census_get_trace_method_name(const census_trace_obj* trace); /* Returns an array of pointers to trace objects of currently active operations and fills in number of active operations. Returns NULL if there are no active operations. Caller owns the returned objects. */ -trace_obj** census_get_active_ops(int* num_active_ops); +census_trace_obj** census_get_active_ops(int* num_active_ops); #ifdef __cplusplus } diff --git a/test/core/statistics/trace_test.c b/test/core/statistics/trace_test.c index 220e0da0225..97e1463ae13 100644 --- a/test/core/statistics/trace_test.c +++ b/test/core/statistics/trace_test.c @@ -181,7 +181,7 @@ static int ids_equal(census_op_id id1, census_op_id id2) { static void test_get_active_ops(void) { census_op_id id_1, id_2, id_3; - trace_obj** active_ops; + census_trace_obj** active_ops; const char* annotation_txt[] = {"annotation 1", "a2"}; int i = 0; int n = 0; @@ -200,11 +200,11 @@ static void test_get_active_ops(void) { GPR_ASSERT(active_ops != NULL); GPR_ASSERT(n == 1); GPR_ASSERT(ids_equal(active_ops[0]->id, id_1)); - trace_obj_destroy(active_ops[0]); + census_trace_obj_destroy(active_ops[0]); gpr_free(active_ops); active_ops = NULL; - /* Start the second and third ops */ + /* Start the second and the third ops */ id_2 = census_tracing_start_op(); census_add_method_tag(id_2, "foo_2"); id_3 = census_tracing_start_op(); @@ -213,7 +213,7 @@ static void test_get_active_ops(void) { active_ops = census_get_active_ops(&n); GPR_ASSERT(n == 3); for (i = 0; i < 3; i++) { - trace_obj_destroy(active_ops[i]); + census_trace_obj_destroy(active_ops[i]); } gpr_free(active_ops); active_ops = NULL; @@ -227,7 +227,7 @@ static void test_get_active_ops(void) { GPR_ASSERT(active_ops != NULL); GPR_ASSERT(n == 2); for (i = 0; i < 2; i++) { - trace_obj_destroy(active_ops[i]); + census_trace_obj_destroy(active_ops[i]); } gpr_free(active_ops); active_ops = NULL; From 1988221d15a4c092837b3cabf17f595c0d01e5c7 Mon Sep 17 00:00:00 2001 From: Yang Gao Date: Thu, 5 Feb 2015 23:06:54 -0800 Subject: [PATCH 07/10] Use environment variable to set root certs in c++ interop test. --- test/cpp/util/create_test_channel.cc | 4 +++- tools/dockerfile/grpc_cxx/Dockerfile | 2 ++ tools/gce_setup/shared_startup_funcs.sh | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/cpp/util/create_test_channel.cc b/test/cpp/util/create_test_channel.cc index a521162bea2..2f95e3aa74f 100644 --- a/test/cpp/util/create_test_channel.cc +++ b/test/cpp/util/create_test_channel.cc @@ -45,6 +45,8 @@ namespace grpc { // override_hostname is provided. // When ssl is not enabled, override_hostname is ignored. // Set use_prod_root to true to use the SSL root for connecting to google. +// In this case, The path to the root file must be set via environment variable +// GRPC_DEFAULT_SSL_ROOTS_FILE_PATH. // Otherwise, root for test SSL cert will be used. // creds will be used to create a channel when enable_ssl is true. // Use examples: @@ -60,7 +62,7 @@ std::shared_ptr CreateTestChannel( ChannelArguments channel_args; if (enable_ssl) { const char* roots_certs = - use_prod_roots ? prod_roots_certs : test_root_cert; + use_prod_roots ? "" : test_root_cert; SslCredentialsOptions ssl_opts = {roots_certs, "", ""}; std::unique_ptr channel_creds = diff --git a/tools/dockerfile/grpc_cxx/Dockerfile b/tools/dockerfile/grpc_cxx/Dockerfile index 43da9fefc37..9b20e7a58e4 100644 --- a/tools/dockerfile/grpc_cxx/Dockerfile +++ b/tools/dockerfile/grpc_cxx/Dockerfile @@ -22,5 +22,7 @@ RUN cd /var/local/git/grpc && ls \ && make interop_server ADD service_account service_account +ADD cacerts cacerts +ENV GRPC_DEFAULT_SSL_ROOTS_FILE_PATH /cacerts/roots.pem CMD ["/var/local/git/grpc/bins/opt/interop_server", "--enable_ssl", "--port=8010"] diff --git a/tools/gce_setup/shared_startup_funcs.sh b/tools/gce_setup/shared_startup_funcs.sh index eea940864da..a6f73d16367 100755 --- a/tools/gce_setup/shared_startup_funcs.sh +++ b/tools/gce_setup/shared_startup_funcs.sh @@ -389,6 +389,7 @@ grpc_dockerfile_install() { grpc_docker_sync_service_account $dockerfile_dir/service_account || return 1; } [[ $image_label == "grpc/cxx" ]] && { + grpc_docker_sync_roots_pem $dockerfile_dir/cacerts || return 1; grpc_docker_sync_service_account $dockerfile_dir/service_account || return 1; } From 48d80e57698fe3d2a8b850c02782724e37a25418 Mon Sep 17 00:00:00 2001 From: Yang Gao Date: Thu, 5 Feb 2015 23:11:02 -0800 Subject: [PATCH 08/10] update comments --- test/cpp/util/create_test_channel.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/util/create_test_channel.cc b/test/cpp/util/create_test_channel.cc index 2f95e3aa74f..301e9a3c3a3 100644 --- a/test/cpp/util/create_test_channel.cc +++ b/test/cpp/util/create_test_channel.cc @@ -45,7 +45,7 @@ namespace grpc { // override_hostname is provided. // When ssl is not enabled, override_hostname is ignored. // Set use_prod_root to true to use the SSL root for connecting to google. -// In this case, The path to the root file must be set via environment variable +// In this case, path to the roots pem file must be set via environment variable // GRPC_DEFAULT_SSL_ROOTS_FILE_PATH. // Otherwise, root for test SSL cert will be used. // creds will be used to create a channel when enable_ssl is true. From d6dcec526db6ac58027be54be977dc50d5b559b3 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Thu, 5 Feb 2015 23:28:17 -0800 Subject: [PATCH 09/10] Making the gpr_tmpfile's win32 version a bit more Windows-y. Also adding windows helpers to convert to and from TCHAR strings. --- build.json | 1 + src/core/support/env_win32.c | 1 + src/core/support/file_win32.c | 51 +++++++++++++++------------ src/core/support/string_win32.c | 29 +++++++++++++++ src/core/support/string_win32.h | 49 +++++++++++++++++++++++++ vsprojects/vs2013/gpr.vcxproj | 1 + vsprojects/vs2013/gpr.vcxproj.filters | 3 ++ 7 files changed, 112 insertions(+), 23 deletions(-) create mode 100644 src/core/support/string_win32.h diff --git a/build.json b/build.json index 170b1480671..579ede329f6 100644 --- a/build.json +++ b/build.json @@ -231,6 +231,7 @@ "src/core/support/file.h", "src/core/support/murmur_hash.h", "src/core/support/string.h", + "src/core/support/string_win32.h", "src/core/support/thd_internal.h" ], "src": [ diff --git a/src/core/support/env_win32.c b/src/core/support/env_win32.c index a31fa79d68e..3159c20f7d7 100644 --- a/src/core/support/env_win32.c +++ b/src/core/support/env_win32.c @@ -39,6 +39,7 @@ #include +#include #include char *gpr_getenv(const char *name) { diff --git a/src/core/support/file_win32.c b/src/core/support/file_win32.c index d415281e0d3..af7eebe3de8 100644 --- a/src/core/support/file_win32.c +++ b/src/core/support/file_win32.c @@ -35,43 +35,48 @@ #ifdef GPR_WIN32 -#include "src/core/support/file.h" - #include #include #include +#include #include #include -FILE *gpr_tmpfile(const char *prefix, char **tmp_filename) { +#include "src/core/support/file.h" +#include "src/core/support/string_win32.h" + +FILE *gpr_tmpfile(const char *prefix, char **tmp_filename_out) { FILE *result = NULL; - char *template; + LPTSTR template_string = NULL; + TCHAR tmp_path[MAX_PATH]; + TCHAR tmp_filename[MAX_PATH]; + DWORD status; + UINT success; - if (tmp_filename != NULL) *tmp_filename = NULL; + if (tmp_filename_out != NULL) *tmp_filename_out = NULL; - gpr_asprintf(&template, "%s_XXXXXX", prefix); - GPR_ASSERT(template != NULL); + /* Convert our prefix to TCHAR. */ + template_string = gpr_char_to_tchar(prefix); + GPR_ASSERT(template_string); - /* _mktemp_s can only create a maximum of 26 file names for any combination of - base and template values which is kind of sad... We may revisit this - function later to have something better... */ - if (_mktemp_s(template, strlen(template) + 1) != 0) { - gpr_log(LOG_ERROR, "Could not create tmp file."); - goto end; - } - if (fopen_s(&result, template, "wb+") != 0) { - gpr_log(GPR_ERROR, "Could not open file %s", template); - result = NULL; - goto end; - } + /* Get the path to the best temporary folder available. */ + status = GetTempPath(MAX_PATH, tmp_path); + if (status == 0 || status > MAX_PATH) goto end; + + /* Generate a unique filename with our template + temporary path. */ + success = GetTempFileName(tmp_path, template_string, 0, tmp_filename); + if (!success) goto end; + + /* Open a file there. */ + if (_tfopen_s(&result, tmp_filename, TEXT("wb+")) != 0) goto end; end: - if (result != NULL && tmp_filename != NULL) { - *tmp_filename = template; - } else { - gpr_free(template); + if (result && tmp_filename) { + *tmp_filename_out = gpr_tchar_to_char(tmp_filename); } + + gpr_free(tmp_filename); return result; } diff --git a/src/core/support/string_win32.c b/src/core/support/string_win32.c index 1c4c8547dc9..02e1c74d9ce 100644 --- a/src/core/support/string_win32.c +++ b/src/core/support/string_win32.c @@ -37,6 +37,7 @@ #ifdef GPR_WIN32 +#include #include #include #include @@ -78,4 +79,32 @@ int gpr_asprintf(char **strp, const char *format, ...) { return -1; } +#if defined UNICODE || defined _UNICODE +LPTSTR gpr_char_to_tchar(LPCSTR input) { + LPTSTR ret; + int needed = MultiByteToWideChar(CP_UTF8, 0, input, -1, NULL, 0); + if (needed == 0) return NULL; + ret = gpr_malloc(needed * sizeof(TCHAR)); + MultiByteToWideChar(CP_UTF8, 0, input, -1, ret, needed); + return ret; +} + +LPSTR gpr_tchar_to_char(LPCTSTR input) { + LPSTR ret; + int needed = WideCharToMultiByte(CP_UTF8, 0, input, -1, NULL, 0, NULL, NULL); + if (needed == 0) return NULL; + ret = gpr_malloc(needed); + WideCharToMultiByte(CP_UTF8, 0, input, -1, ret, needed, NULL, NULL); + return ret; +} +#else +char *gpr_tchar_to_char(LPTSTR input) { + return gpr_strdup(input); +} + +char *gpr_char_to_tchar(LPTSTR input) { + return gpr_strdup(input); +} +#endif + #endif /* GPR_WIN32 */ diff --git a/src/core/support/string_win32.h b/src/core/support/string_win32.h new file mode 100644 index 00000000000..9102d982569 --- /dev/null +++ b/src/core/support/string_win32.h @@ -0,0 +1,49 @@ +/* + * + * Copyright 2014, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#ifndef __GRPC_SUPPORT_STRING_WIN32_H__ +#define __GRPC_SUPPORT_STRING_WIN32_H__ + +#include + +#ifdef GPR_WIN32 + +#include + +/* These allocate new strings using gpr_malloc to convert from and to utf-8. */ +LPTSTR gpr_char_to_tchar(LPCSTR input); +LPSTR gpr_tchar_to_char(LPCTSTR input); + +#endif /* GPR_WIN32 */ + +#endif /* __GRPC_SUPPORT_STRING_WIN32_H__ */ diff --git a/vsprojects/vs2013/gpr.vcxproj b/vsprojects/vs2013/gpr.vcxproj index d1f70853839..8276082cfda 100644 --- a/vsprojects/vs2013/gpr.vcxproj +++ b/vsprojects/vs2013/gpr.vcxproj @@ -100,6 +100,7 @@ + diff --git a/vsprojects/vs2013/gpr.vcxproj.filters b/vsprojects/vs2013/gpr.vcxproj.filters index 13add834a88..2329acc9dae 100644 --- a/vsprojects/vs2013/gpr.vcxproj.filters +++ b/vsprojects/vs2013/gpr.vcxproj.filters @@ -176,6 +176,9 @@ src\core\support + + src\core\support + src\core\support From a0ec4fc8ea4ab3e75a28b8de76a6e2316559ed5d Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Fri, 6 Feb 2015 00:01:56 -0800 Subject: [PATCH 10/10] Adding /tmp to prefix when creating posix temporary files. - Addresses #416. --- src/core/support/file_posix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/support/file_posix.c b/src/core/support/file_posix.c index 21a985012eb..cb48b3d52f4 100644 --- a/src/core/support/file_posix.c +++ b/src/core/support/file_posix.c @@ -67,7 +67,7 @@ FILE *gpr_tmpfile(const char *prefix, char **tmp_filename) { if (tmp_filename != NULL) *tmp_filename = NULL; - gpr_asprintf(&template, "%s_XXXXXX", prefix); + gpr_asprintf(&template, "/tmp/%s_XXXXXX", prefix); GPR_ASSERT(template != NULL); fd = mkstemp(template);