Merge pull request grpc#10391 from dgquintas/lr_back_to_md

Load Reporting back to using metadata
daniel-j-born · Apr 17, 2017 · a6f64f4 · a6f64f4
2 parents c5fc2b7 + a818f72
commit a6f64f4
Show file tree

Hide file tree

Showing 14 changed files with 201 additions and 199 deletions.
diff --git a/grpc.def b/grpc.def
@@ -72,7 +72,6 @@ EXPORTS
     grpc_channel_create_registered_call
     grpc_call_start_batch
     grpc_call_get_peer
-    grpc_call_set_load_reporting_cost_context
     grpc_census_call_set_context
     grpc_census_call_get_context
     grpc_channel_get_target

diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h
@@ -296,12 +296,6 @@ GRPCAPI grpc_call_error grpc_call_start_batch(grpc_call *call,
     functionality. Instead, use grpc_auth_context. */
 GRPCAPI char *grpc_call_get_peer(grpc_call *call);
 
-struct grpc_load_reporting_cost_context;
-
-/* Associate costs contained in \a cost_context to \a call. */
-GRPCAPI void grpc_call_set_load_reporting_cost_context(
-    grpc_call *call, struct grpc_load_reporting_cost_context *context);
-
 struct census_context;
 
 /** Set census context for a call; Must be called before first call to

diff --git a/include/grpc/load_reporting.h b/include/grpc/load_reporting.h
@@ -35,7 +35,6 @@
 #define GRPC_LOAD_REPORTING_H
 
 #include <grpc/impl/codegen/port_platform.h>
-#include <grpc/slice.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -50,11 +49,12 @@ extern "C" {
  * gRPC LB system. */
 #define GRPC_LB_TOKEN_MD_KEY "lb-token"
 
-/** A sequence of values for load reporting purposes */
-typedef struct grpc_load_reporting_cost_context {
-  grpc_slice *values;
-  size_t values_count;
-} grpc_load_reporting_cost_context;
+/** Metadata key for gRPC LB cost reporting.
+ *
+ * The value corresponding to this key is an opaque binary blob reported by the
+ * backend as part of its trailing metadata containing cost information for the
+ * call. */
+#define GRPC_LB_COST_MD_KEY "lb-cost-bin"
 
 #ifdef __cplusplus
 }

diff --git a/src/core/ext/filters/load_reporting/load_reporting.c b/src/core/ext/filters/load_reporting/load_reporting.c
@@ -47,23 +47,6 @@
 #include "src/core/lib/surface/call.h"
 #include "src/core/lib/surface/channel_init.h"
 
-static void destroy_lr_cost_context(void *c) {
-  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
-  grpc_load_reporting_cost_context *cost_ctx = c;
-  for (size_t i = 0; i < cost_ctx->values_count; ++i) {
-    grpc_slice_unref_internal(&exec_ctx, cost_ctx->values[i]);
-  }
-  grpc_exec_ctx_finish(&exec_ctx);
-  gpr_free(cost_ctx->values);
-  gpr_free(cost_ctx);
-}
-
-void grpc_call_set_load_reporting_cost_context(
-    grpc_call *call, grpc_load_reporting_cost_context *ctx) {
-  grpc_call_context_set(call, GRPC_CONTEXT_LR_COST, ctx,
-                        destroy_lr_cost_context);
-}
-
 static bool is_load_reporting_enabled(const grpc_channel_args *a) {
   return grpc_channel_arg_get_bool(
       grpc_channel_args_find(a, GRPC_ARG_ENABLE_LOAD_REPORTING), false);

diff --git a/src/core/ext/filters/load_reporting/load_reporting_filter.c b/src/core/ext/filters/load_reporting/load_reporting_filter.c
@@ -48,6 +48,8 @@
 
 typedef struct call_data {
   intptr_t id; /**< an id unique to the call */
+  bool have_trailing_md_string;
+  grpc_slice trailing_md_string;
   bool have_initial_md_string;
   grpc_slice initial_md_string;
   bool have_service_method;
@@ -140,6 +142,9 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
   if (calld->have_initial_md_string) {
     grpc_slice_unref_internal(exec_ctx, calld->initial_md_string);
   }
+  if (calld->have_trailing_md_string) {
+    grpc_slice_unref_internal(exec_ctx, calld->trailing_md_string);
+  }
   if (calld->have_service_method) {
     grpc_slice_unref_internal(exec_ctx, calld->service_method);
   }
@@ -183,20 +188,40 @@ static void destroy_channel_elem(grpc_exec_ctx *exec_ctx,
   */
 }
 
+static grpc_filtered_mdelem lr_trailing_md_filter(grpc_exec_ctx *exec_ctx,
+                                                  void *user_data,
+                                                  grpc_mdelem md) {
+  grpc_call_element *elem = user_data;
+  call_data *calld = elem->call_data;
+  if (grpc_slice_eq(GRPC_MDKEY(md), GRPC_MDSTR_LB_COST_BIN)) {
+    calld->trailing_md_string = GRPC_MDVALUE(md);
+    return GRPC_FILTERED_REMOVE();
+  }
+  return GRPC_FILTERED_MDELEM(md);
+}
+
 static void lr_start_transport_stream_op_batch(
     grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
     grpc_transport_stream_op_batch *op) {
   GPR_TIMER_BEGIN("lr_start_transport_stream_op_batch", 0);
   call_data *calld = elem->call_data;
 
   if (op->recv_initial_metadata) {
+    /* substitute our callback for the higher callback */
     calld->recv_initial_metadata =
         op->payload->recv_initial_metadata.recv_initial_metadata;
-    /* substitute our callback for the higher callback */
     calld->ops_recv_initial_metadata_ready =
         op->payload->recv_initial_metadata.recv_initial_metadata_ready;
     op->payload->recv_initial_metadata.recv_initial_metadata_ready =
         &calld->on_initial_md_ready;
+  } else if (op->send_trailing_metadata) {
+    GRPC_LOG_IF_ERROR(
+        "grpc_metadata_batch_filter",
+        grpc_metadata_batch_filter(
+            exec_ctx,
+            op->payload->send_trailing_metadata.send_trailing_metadata,
+            lr_trailing_md_filter, elem,
+            "LR trailing metadata filtering error"));
   }
   grpc_call_next_op(exec_ctx, elem, op);
 

diff --git a/src/core/lib/channel/context.h b/src/core/lib/channel/context.h
@@ -50,9 +50,6 @@ typedef enum {
   /// Reserved for traffic_class_context.
   GRPC_CONTEXT_TRAFFIC,
 
-  /// Costs for Load Reporting.
-  GRPC_CONTEXT_LR_COST,
-
   GRPC_CONTEXT_COUNT
 } grpc_context_index;