From 57859e3c9d5e8351d0ceef2c4b4287e51cc2c8fe Mon Sep 17 00:00:00 2001 From: easy Date: Thu, 8 Apr 2021 19:42:56 +1000 Subject: [PATCH] Remove duplicate code for grpc encoding, and extra class. (#22334) * Remove duplicate code for grpc encoding, and extra class. Originally from #18359 by @bogdandrutu. * Autoformat. --- BUILD | 1 + bazel/grpc_deps.bzl | 5 + src/cpp/ext/filters/census/context.cc | 24 +-- src/cpp/ext/filters/census/context.h | 4 +- src/cpp/ext/filters/census/rpc_encoding.cc | 7 - src/cpp/ext/filters/census/rpc_encoding.h | 169 --------------------- 6 files changed, 21 insertions(+), 189 deletions(-) diff --git a/BUILD b/BUILD index 39296e23b45..cba753fcc35 100644 --- a/BUILD +++ b/BUILD @@ -2748,6 +2748,7 @@ grpc_cc_library( "absl-time", "opencensus-trace", "opencensus-trace-context_util", + "opencensus-trace-propagation", "opencensus-stats", "opencensus-context", ], diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl index bb774077a5c..6a4d68071bc 100644 --- a/bazel/grpc_deps.bzl +++ b/bazel/grpc_deps.bzl @@ -121,6 +121,11 @@ def grpc_deps(): actual = "@io_opencensus_cpp//opencensus/trace:context_util", ) + native.bind( + name = "opencensus-trace-propagation", + actual = "@io_opencensus_cpp//opencensus/trace:grpc_trace_bin", + ) + native.bind( name = "opencensus-stats", actual = "@io_opencensus_cpp//opencensus/stats:stats", diff --git a/src/cpp/ext/filters/census/context.cc b/src/cpp/ext/filters/census/context.cc index cffa68d6f56..0650bd1b01a 100644 --- a/src/cpp/ext/filters/census/context.cc +++ b/src/cpp/ext/filters/census/context.cc @@ -20,6 +20,7 @@ #include "opencensus/tags/context_util.h" #include "opencensus/trace/context_util.h" +#include "opencensus/trace/propagation/grpc_trace_bin.h" #include "src/cpp/ext/filters/census/context.h" namespace grpc { @@ -33,14 +34,11 @@ void GenerateServerContext(absl::string_view tracing, absl::string_view method, // Destruct the current CensusContext to free the Span memory before // overwriting it below. context->~CensusContext(); - GrpcTraceContext trace_ctxt; - if (TraceContextEncoding::Decode(tracing, &trace_ctxt) != - TraceContextEncoding::kEncodeDecodeFailure) { - SpanContext parent_ctx = trace_ctxt.ToSpanContext(); - if (parent_ctx.IsValid()) { - new (context) CensusContext(method, parent_ctx); - return; - } + SpanContext parent_ctx = + opencensus::trace::propagation::FromGrpcTraceBinHeader(tracing); + if (parent_ctx.IsValid()) { + new (context) CensusContext(method, parent_ctx); + return; } new (context) CensusContext(method, TagMap{}); } @@ -71,9 +69,13 @@ void GenerateClientContext(absl::string_view method, CensusContext* ctxt, size_t TraceContextSerialize(const ::opencensus::trace::SpanContext& context, char* tracing_buf, size_t tracing_buf_size) { - GrpcTraceContext trace_ctxt(context); - return TraceContextEncoding::Encode(trace_ctxt, tracing_buf, - tracing_buf_size); + if (tracing_buf_size < + opencensus::trace::propagation::kGrpcTraceBinHeaderLen) { + return 0; + } + opencensus::trace::propagation::ToGrpcTraceBinHeader( + context, reinterpret_cast(tracing_buf)); + return opencensus::trace::propagation::kGrpcTraceBinHeaderLen; } size_t StatsContextSerialize(size_t /*max_tags_len*/, grpc_slice* /*tags*/) { diff --git a/src/cpp/ext/filters/census/context.h b/src/cpp/ext/filters/census/context.h index 55709963522..67f5704a624 100644 --- a/src/cpp/ext/filters/census/context.h +++ b/src/cpp/ext/filters/census/context.h @@ -75,8 +75,8 @@ class CensusContext { ::opencensus::tags::TagMap tags_; }; -// Serializes the outgoing trace context. Field IDs are 1 byte followed by -// field data. A 1 byte version ID is always encoded first. +// Serializes the outgoing trace context. tracing_buf must be +// opencensus::trace::propagation::kGrpcTraceBinHeaderLen bytes long. size_t TraceContextSerialize(const ::opencensus::trace::SpanContext& context, char* tracing_buf, size_t tracing_buf_size); diff --git a/src/cpp/ext/filters/census/rpc_encoding.cc b/src/cpp/ext/filters/census/rpc_encoding.cc index 45a66d9dc82..7ce3e940df7 100644 --- a/src/cpp/ext/filters/census/rpc_encoding.cc +++ b/src/cpp/ext/filters/census/rpc_encoding.cc @@ -22,13 +22,6 @@ namespace grpc { -constexpr size_t TraceContextEncoding::kGrpcTraceContextSize; -constexpr size_t TraceContextEncoding::kEncodeDecodeFailure; -constexpr size_t TraceContextEncoding::kVersionIdSize; -constexpr size_t TraceContextEncoding::kFieldIdSize; -constexpr size_t TraceContextEncoding::kVersionIdOffset; -constexpr size_t TraceContextEncoding::kVersionId; - constexpr size_t RpcServerStatsEncoding::kRpcServerStatsSize; constexpr size_t RpcServerStatsEncoding::kEncodeDecodeFailure; constexpr size_t RpcServerStatsEncoding::kVersionIdSize; diff --git a/src/cpp/ext/filters/census/rpc_encoding.h b/src/cpp/ext/filters/census/rpc_encoding.h index b897dfcb638..821c7158dd9 100644 --- a/src/cpp/ext/filters/census/rpc_encoding.h +++ b/src/cpp/ext/filters/census/rpc_encoding.h @@ -31,175 +31,6 @@ namespace grpc { -// TODO(unknown): Rename to GrpcTraceContextV0. -struct GrpcTraceContext { - GrpcTraceContext() {} - - explicit GrpcTraceContext(const ::opencensus::trace::SpanContext& ctx) { - ctx.trace_id().CopyTo(trace_id); - ctx.span_id().CopyTo(span_id); - ctx.trace_options().CopyTo(trace_options); - } - - ::opencensus::trace::SpanContext ToSpanContext() const { - return ::opencensus::trace::SpanContext( - ::opencensus::trace::TraceId(trace_id), - ::opencensus::trace::SpanId(span_id), - ::opencensus::trace::TraceOptions(trace_options)); - } - - // TODO(unknown): For performance: - // uint8_t version; - // uint8_t trace_id_field_id; - uint8_t trace_id[::opencensus::trace::TraceId::kSize]; - // uint8_t span_id_field_id; - uint8_t span_id[::opencensus::trace::SpanId::kSize]; - // uint8_t trace_options_field_id; - uint8_t trace_options[::opencensus::trace::TraceOptions::kSize]; -}; - -// TraceContextEncoding encapsulates the logic for encoding and decoding of -// trace contexts. -class TraceContextEncoding { - public: - // Size of encoded GrpcTraceContext. (16 + 8 + 1 + 4) - static constexpr size_t kGrpcTraceContextSize = 29; - // Error value. - static constexpr size_t kEncodeDecodeFailure = 0; - - // Deserializes a GrpcTraceContext from the incoming buffer. Returns the - // number of bytes deserialized from the buffer. If the incoming buffer is - // empty or the encoding version is not supported it will return 0 bytes, - // currently only version 0 is supported. If an unknown field ID is - // encountered it will return immediately without parsing the rest of the - // buffer. Inlined for performance reasons. - static size_t Decode(absl::string_view buf, GrpcTraceContext* tc) { - if (buf.empty()) { - return kEncodeDecodeFailure; - } - uint8_t version = buf[kVersionIdOffset]; - // TODO(unknown): Support other versions later. Only support version 0 for - // now. - if (version != kVersionId) { - return kEncodeDecodeFailure; - } - - size_t pos = kVersionIdSize; - while (pos < buf.size()) { - size_t bytes_read = - ParseField(absl::string_view(&buf[pos], buf.size() - pos), tc); - if (bytes_read == 0) { - break; - } else { - pos += bytes_read; - } - } - return pos; - } - - // Serializes a GrpcTraceContext into the provided buffer. Returns the number - // of bytes serialized into the buffer. If the buffer is not of sufficient - // size (it must be at least kGrpcTraceContextSize bytes) it will drop - // everything and return 0 bytes serialized. Inlined for performance reasons. - static size_t Encode(const GrpcTraceContext& tc, char* buf, size_t buf_size) { - if (buf_size < kGrpcTraceContextSize) { - return kEncodeDecodeFailure; - } - buf[kVersionIdOffset] = kVersionId; - buf[kTraceIdOffset] = kTraceIdField; - memcpy(&buf[kTraceIdOffset + 1], tc.trace_id, - opencensus::trace::TraceId::kSize); - buf[kSpanIdOffset] = kSpanIdField; - memcpy(&buf[kSpanIdOffset + 1], tc.span_id, - opencensus::trace::SpanId::kSize); - buf[kTraceOptionsOffset] = kTraceOptionsField; - memcpy(&buf[kTraceOptionsOffset + 1], tc.trace_options, - opencensus::trace::TraceOptions::kSize); - return kGrpcTraceContextSize; - } - - private: - // Parses the next field from the incoming buffer and stores the parsed value - // in a GrpcTraceContext struct. If it does not recognize the field ID it - // will return 0, otherwise it returns the number of bytes read. - static size_t ParseField(absl::string_view buf, GrpcTraceContext* tc) { - // TODO(unknown): Add support for multi-byte field IDs. - if (buf.empty()) { - return 0; - } - // Field ID is always the first byte in a field. - uint32_t field_id = buf[0]; - size_t bytes_read = kFieldIdSize; - switch (field_id) { - case kTraceIdField: - bytes_read += kTraceIdSize; - if (bytes_read > buf.size()) { - return 0; - } - memcpy(tc->trace_id, &buf[kFieldIdSize], - opencensus::trace::TraceId::kSize); - break; - case kSpanIdField: - bytes_read += kSpanIdSize; - if (bytes_read > buf.size()) { - return 0; - } - memcpy(tc->span_id, &buf[kFieldIdSize], - opencensus::trace::SpanId::kSize); - break; - case kTraceOptionsField: - bytes_read += kTraceOptionsSize; - if (bytes_read > buf.size()) { - return 0; - } - memcpy(tc->trace_options, &buf[kFieldIdSize], - opencensus::trace::TraceOptions::kSize); - break; - default: // Invalid field ID - return 0; - } - - return bytes_read; - } - - // Size of Version ID. - static constexpr size_t kVersionIdSize = 1; - // Size of Field ID. - static constexpr size_t kFieldIdSize = 1; - - // Offset and value for currently supported version ID. - static constexpr size_t kVersionIdOffset = 0; - static constexpr size_t kVersionId = 0; - - // Fixed Field ID values: - enum FieldIdValue { - kTraceIdField = 0, - kSpanIdField = 1, - kTraceOptionsField = 2, - }; - - // Field data sizes in bytes - enum FieldSize { - kTraceIdSize = 16, - kSpanIdSize = 8, - kTraceOptionsSize = 1, - }; - - // Fixed size offsets for field ID start positions during encoding. Field - // data immediately follows. - enum FieldIdOffset { - kTraceIdOffset = kVersionIdSize, - kSpanIdOffset = kTraceIdOffset + kFieldIdSize + kTraceIdSize, - kTraceOptionsOffset = kSpanIdOffset + kFieldIdSize + kSpanIdSize, - }; - - TraceContextEncoding() = delete; - TraceContextEncoding(const TraceContextEncoding&) = delete; - TraceContextEncoding(TraceContextEncoding&&) = delete; - TraceContextEncoding operator=(const TraceContextEncoding&) = delete; - TraceContextEncoding operator=(TraceContextEncoding&&) = delete; -}; - // TODO(unknown): This may not be needed. Check to see if opencensus requires // a trailing server response. // RpcServerStatsEncoding encapsulates the logic for encoding and decoding of