Skip to content
This repository was archived by the owner on Jul 31, 2023. It is now read-only.

Commit d28a1bb

Browse files
authored
Use different names for client Spans and server Spans. (#88)
Trimming leading '/' if present and appending "Sent." and "Recv." to method names.
1 parent af429f6 commit d28a1bb

File tree

6 files changed

+53
-45
lines changed

6 files changed

+53
-45
lines changed

opencensus/plugins/grpc/internal/client_filter.cc

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ void CensusClientCallData::StartTransportStreamOpBatch(
8686
// grpc has been changed to populate census context at call initialization.
8787
census_context *ctxt = op->get_census_context();
8888
GenerateClientContext(
89-
method_, &context_,
89+
qualified_method_, &context_,
9090
(ctxt == nullptr) ? nullptr : reinterpret_cast<CensusContext *>(ctxt));
9191
char tracing_buf[kMaxTracingLen];
9292
size_t tracing_len =
@@ -139,18 +139,14 @@ grpc_error *CensusClientCallData::Init(grpc_call_element *elem,
139139
const grpc_call_element_args *args) {
140140
path_ = grpc_slice_ref_internal(args->path);
141141
start_time_ = absl::Now();
142-
const char *method_str =
143-
GPR_SLICE_IS_EMPTY(path_)
144-
? ""
145-
: reinterpret_cast<const char *>(GRPC_SLICE_START_PTR(path_));
146-
method_ = absl::string_view(
147-
method_str, GRPC_SLICE_IS_EMPTY(path_) ? 0 : GRPC_SLICE_LENGTH(path_));
142+
qualified_method_ = StrCat("Sent.", GetMethod(&path_));
148143
GRPC_CLOSURE_INIT(&on_done_recv_message_, OnDoneRecvMessageCb, elem,
149144
grpc_schedule_on_exec_ctx);
150145
GRPC_CLOSURE_INIT(&on_done_recv_trailing_metadata_,
151146
OnDoneRecvTrailingMetadataCb, elem,
152147
grpc_schedule_on_exec_ctx);
153-
stats::Record({{RpcClientStartedCount(), 1}}, {{kMethodTagKey, method_}});
148+
stats::Record({{RpcClientStartedCount(), 1}},
149+
{{kMethodTagKey, qualified_method_}});
154150
return GRPC_ERROR_NONE;
155151
}
156152

@@ -171,7 +167,7 @@ void CensusClientCallData::Destroy(grpc_call_element *elem,
171167
{RpcClientFinishedCount(), 1},
172168
{RpcClientRequestCount(), sent_message_count_},
173169
{RpcClientResponseCount(), recv_message_count_}},
174-
{{kMethodTagKey, method_},
170+
{{kMethodTagKey, qualified_method_},
175171
{kStatusTagKey, StatusCodeToString(final_info->final_status)}});
176172
grpc_slice_unref_internal(path_);
177173
context_.EndSpan();

opencensus/plugins/grpc/internal/client_filter.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#ifndef OPENCENSUS_PLUGINS_INTERNAL_CLIENT_FILTER_H_
1616
#define OPENCENSUS_PLUGINS_INTERNAL_CLIENT_FILTER_H_
1717

18+
#include <string>
19+
1820
#include "absl/strings/string_view.h"
1921
#include "absl/time/time.h"
2022
#include "opencensus/plugins/grpc/internal/channel_filter.h"
@@ -64,7 +66,7 @@ class CensusClientCallData : public grpc::CallData {
6466
grpc_linked_mdelem stats_bin_;
6567
grpc_linked_mdelem tracing_bin_;
6668
// Client method.
67-
absl::string_view method_;
69+
std::string qualified_method_;
6870
grpc_slice path_;
6971
// The recv trailing metadata callbacks.
7072
grpc_metadata_batch *recv_trailing_metadata_;

opencensus/plugins/grpc/internal/filter.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#define OPENCENSUS_PLUGINS_INTERNAL_FILTER_H_
1717

1818
#include "absl/strings/string_view.h"
19+
#include "absl/strings/strip.h"
1920
#include "include/grpc/impl/codegen/status.h"
2021
#include "opencensus/plugins/grpc/internal/rpc_encoding.h"
2122
#include "opencensus/trace/span.h"
@@ -104,6 +105,17 @@ trace::Span SpanFromCensusContext(const census_context *ctxt);
104105
// Returns a string representation of the StatusCode enum.
105106
absl::string_view StatusCodeToString(grpc_status_code code);
106107

108+
inline absl::string_view GetMethod(const grpc_slice *path) {
109+
if (GRPC_SLICE_IS_EMPTY(*path)) {
110+
return "";
111+
}
112+
// Check for leading '/' and trim it if present.
113+
return absl::StripPrefix(absl::string_view(reinterpret_cast<const char *>(
114+
GRPC_SLICE_START_PTR(*path)),
115+
GRPC_SLICE_LENGTH(*path)),
116+
"/");
117+
}
118+
107119
} // namespace opencensus
108120

109121
#endif // OPENCENSUS_PLUGINS_INTERNAL_FILTER_H_

opencensus/plugins/grpc/internal/server_filter.cc

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,7 @@ void CensusServerCallData::OnDoneRecvInitialMetadataCb(void *user_data,
9494
sml.census_proto = grpc_empty_slice();
9595
FilterInitialMetadata(initial_metadata, &sml);
9696
calld->path_ = grpc_slice_ref_internal(sml.path);
97-
const char *method_str = GRPC_SLICE_IS_EMPTY(calld->path_)
98-
? ""
99-
: reinterpret_cast<const char *>(
100-
GRPC_SLICE_START_PTR(calld->path_));
101-
calld->method_ = absl::string_view(
102-
method_str,
103-
GRPC_SLICE_IS_EMPTY(sml.path) ? 0 : GRPC_SLICE_LENGTH(sml.path));
97+
calld->qualified_method_ = StrCat("Recv.", GetMethod(&calld->path_));
10498
const char *tracing_str =
10599
GRPC_SLICE_IS_EMPTY(sml.tracing_slice)
106100
? ""
@@ -119,10 +113,10 @@ void CensusServerCallData::OnDoneRecvInitialMetadataCb(void *user_data,
119113

120114
GenerateServerContext(absl::string_view(tracing_str, tracing_str_len),
121115
absl::string_view(census_str, census_str_len),
122-
/*primary_role*/ "", calld->method_,
116+
/*primary_role*/ "", calld->qualified_method_,
123117
&calld->context_);
124118
stats::Record({{RpcServerStartedCount(), 1}},
125-
{{kMethodTagKey, calld->method_}});
119+
{{kMethodTagKey, calld->qualified_method_}});
126120

127121
grpc_slice_unref_internal(sml.tracing_slice);
128122
grpc_slice_unref_internal(sml.census_proto);
@@ -206,7 +200,7 @@ void CensusServerCallData::Destroy(grpc_call_element *elem,
206200
{RpcServerRequestCount(), sent_message_count_},
207201
{RpcServerFinishedCount(), 1},
208202
{RpcServerResponseCount(), recv_message_count_}},
209-
{{kMethodTagKey, method_},
203+
{{kMethodTagKey, qualified_method_},
210204
{kStatusTagKey, StatusCodeToString(final_info->final_status)}});
211205
grpc_slice_unref_internal(path_);
212206
context_.EndSpan();

opencensus/plugins/grpc/internal/server_filter.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#ifndef OPENCENSUS_PLUGINS_INTERNAL_SERVER_FILTER_H_
1616
#define OPENCENSUS_PLUGINS_INTERNAL_SERVER_FILTER_H_
1717

18+
#include <string>
19+
1820
#include "absl/strings/string_view.h"
1921
#include "absl/time/clock.h"
2022
#include "absl/time/time.h"
@@ -61,7 +63,7 @@ class CensusServerCallData : public grpc::CallData {
6163
private:
6264
CensusContext context_;
6365
// server method
64-
absl::string_view method_;
66+
std::string qualified_method_;
6567
grpc_slice path_;
6668
// Pointer to the grpc_call element
6769
grpc_call *gc_;

opencensus/plugins/grpc/internal/stats_plugin_end2end_test.cc

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ class StatsPluginEnd2EndTest : public ::testing::Test {
7575

7676
void RunServerLoop() { server_->Wait(); }
7777

78-
const std::string method_name_ = "/opencensus.testing.EchoService/Echo";
78+
const std::string client_method_name_ =
79+
"Sent.opencensus.testing.EchoService/Echo";
80+
const std::string server_method_name_ =
81+
"Recv.opencensus.testing.EchoService/Echo";
7982

8083
std::string server_address_;
8184
EchoServer service_;
@@ -91,7 +94,6 @@ TEST_F(StatsPluginEnd2EndTest, ErrorCount) {
9194
.set_measure(kRpcClientErrorCountMeasureName)
9295
.set_name("client_method")
9396
.set_aggregation(stats::Aggregation::Sum())
94-
9597
.add_column(kMethodTagKey);
9698
stats::View client_method_view(client_method_descriptor);
9799
const auto server_method_descriptor =
@@ -100,7 +102,7 @@ TEST_F(StatsPluginEnd2EndTest, ErrorCount) {
100102
.set_name("server_method")
101103
.set_aggregation(stats::Aggregation::Sum())
102104
.add_column(kMethodTagKey);
103-
stats::View server_method_view(client_method_descriptor);
105+
stats::View server_method_view(server_method_descriptor);
104106

105107
const auto client_status_descriptor =
106108
stats::ViewDescriptor()
@@ -129,11 +131,11 @@ TEST_F(StatsPluginEnd2EndTest, ErrorCount) {
129131
absl::SleepFor(absl::Milliseconds(500));
130132

131133
EXPECT_THAT(client_method_view.GetData().double_data(),
132-
::testing::UnorderedElementsAre(
133-
::testing::Pair(::testing::ElementsAre(method_name_), 16.0)));
134+
::testing::UnorderedElementsAre(::testing::Pair(
135+
::testing::ElementsAre(client_method_name_), 16.0)));
134136
EXPECT_THAT(server_method_view.GetData().double_data(),
135-
::testing::UnorderedElementsAre(
136-
::testing::Pair(::testing::ElementsAre(method_name_), 16.0)));
137+
::testing::UnorderedElementsAre(::testing::Pair(
138+
::testing::ElementsAre(server_method_name_), 16.0)));
137139

138140
auto codes = {
139141
::testing::Pair(::testing::ElementsAre("OK"), 0.0),
@@ -181,28 +183,28 @@ TEST_F(StatsPluginEnd2EndTest, RequestResponseBytes) {
181183
EXPECT_THAT(
182184
client_request_bytes_view.GetData().distribution_data(),
183185
::testing::UnorderedElementsAre(::testing::Pair(
184-
::testing::ElementsAre(method_name_),
186+
::testing::ElementsAre(client_method_name_),
185187
::testing::AllOf(::testing::Property(&stats::Distribution::count, 1),
186188
::testing::Property(&stats::Distribution::mean,
187189
::testing::Gt(0.0))))));
188190
EXPECT_THAT(
189191
client_response_bytes_view.GetData().distribution_data(),
190192
::testing::UnorderedElementsAre(::testing::Pair(
191-
::testing::ElementsAre(method_name_),
193+
::testing::ElementsAre(client_method_name_),
192194
::testing::AllOf(::testing::Property(&stats::Distribution::count, 1),
193195
::testing::Property(&stats::Distribution::mean,
194196
::testing::Gt(0.0))))));
195197
EXPECT_THAT(
196198
server_request_bytes_view.GetData().distribution_data(),
197199
::testing::UnorderedElementsAre(::testing::Pair(
198-
::testing::ElementsAre(method_name_),
200+
::testing::ElementsAre(server_method_name_),
199201
::testing::AllOf(::testing::Property(&stats::Distribution::count, 1),
200202
::testing::Property(&stats::Distribution::mean,
201203
::testing::Gt(0.0))))));
202204
EXPECT_THAT(
203205
server_response_bytes_view.GetData().distribution_data(),
204206
::testing::UnorderedElementsAre(::testing::Pair(
205-
::testing::ElementsAre(method_name_),
207+
::testing::ElementsAre(server_method_name_),
206208
::testing::AllOf(::testing::Property(&stats::Distribution::count, 1),
207209
::testing::Property(&stats::Distribution::mean,
208210
::testing::Gt(0.0))))));
@@ -234,7 +236,7 @@ TEST_F(StatsPluginEnd2EndTest, Latency) {
234236
EXPECT_THAT(
235237
client_latency_view.GetData().distribution_data(),
236238
::testing::UnorderedElementsAre(::testing::Pair(
237-
::testing::ElementsAre(method_name_),
239+
::testing::ElementsAre(client_method_name_),
238240
::testing::AllOf(::testing::Property(&stats::Distribution::count, 1),
239241
::testing::Property(&stats::Distribution::mean,
240242
::testing::Gt(0.0)),
@@ -244,11 +246,11 @@ TEST_F(StatsPluginEnd2EndTest, Latency) {
244246
// Elapsed time is a subinterval of total latency.
245247
const auto client_latency = client_latency_view.GetData()
246248
.distribution_data()
247-
.find({method_name_})
249+
.find({client_method_name_})
248250
->second.mean();
249251
EXPECT_THAT(client_server_elapsed_time_view.GetData().distribution_data(),
250252
::testing::UnorderedElementsAre(::testing::Pair(
251-
::testing::ElementsAre(method_name_),
253+
::testing::ElementsAre(client_method_name_),
252254
::testing::AllOf(
253255
::testing::Property(&stats::Distribution::count, 1),
254256
::testing::Property(&stats::Distribution::mean,
@@ -260,12 +262,12 @@ TEST_F(StatsPluginEnd2EndTest, Latency) {
260262
// client.
261263
const auto client_elapsed_time = client_server_elapsed_time_view.GetData()
262264
.distribution_data()
263-
.find({method_name_})
265+
.find({client_method_name_})
264266
->second.mean();
265267
EXPECT_THAT(
266268
server_server_elapsed_time_view.GetData().distribution_data(),
267269
::testing::UnorderedElementsAre(::testing::Pair(
268-
::testing::ElementsAre(method_name_),
270+
::testing::ElementsAre(server_method_name_),
269271
::testing::AllOf(
270272
::testing::Property(&stats::Distribution::count, 1),
271273
::testing::Property(&stats::Distribution::mean,
@@ -293,16 +295,16 @@ TEST_F(StatsPluginEnd2EndTest, StartFinishCount) {
293295

294296
EXPECT_THAT(client_started_count_view.GetData().double_data(),
295297
::testing::UnorderedElementsAre(::testing::Pair(
296-
::testing::ElementsAre(method_name_), i + 1)));
298+
::testing::ElementsAre(client_method_name_), i + 1)));
297299
EXPECT_THAT(client_finished_count_view.GetData().double_data(),
298300
::testing::UnorderedElementsAre(::testing::Pair(
299-
::testing::ElementsAre(method_name_), i + 1)));
301+
::testing::ElementsAre(client_method_name_), i + 1)));
300302
EXPECT_THAT(server_started_count_view.GetData().double_data(),
301303
::testing::UnorderedElementsAre(::testing::Pair(
302-
::testing::ElementsAre(method_name_), i + 1)));
304+
::testing::ElementsAre(server_method_name_), i + 1)));
303305
EXPECT_THAT(server_finished_count_view.GetData().double_data(),
304306
::testing::UnorderedElementsAre(::testing::Pair(
305-
::testing::ElementsAre(method_name_), i + 1)));
307+
::testing::ElementsAre(server_method_name_), i + 1)));
306308
}
307309
}
308310

@@ -328,28 +330,28 @@ TEST_F(StatsPluginEnd2EndTest, RequestResponseCount) {
328330

329331
EXPECT_THAT(client_request_count_view.GetData().distribution_data(),
330332
::testing::UnorderedElementsAre(::testing::Pair(
331-
::testing::ElementsAre(method_name_),
333+
::testing::ElementsAre(client_method_name_),
332334
::testing::AllOf(
333335
::testing::Property(&stats::Distribution::count, i + 1),
334336
::testing::Property(&stats::Distribution::mean,
335337
::testing::DoubleEq(1.0))))));
336338
EXPECT_THAT(client_response_count_view.GetData().distribution_data(),
337339
::testing::UnorderedElementsAre(::testing::Pair(
338-
::testing::ElementsAre(method_name_),
340+
::testing::ElementsAre(client_method_name_),
339341
::testing::AllOf(
340342
::testing::Property(&stats::Distribution::count, i + 1),
341343
::testing::Property(&stats::Distribution::mean,
342344
::testing::DoubleEq(1.0))))));
343345
EXPECT_THAT(server_request_count_view.GetData().distribution_data(),
344346
::testing::UnorderedElementsAre(::testing::Pair(
345-
::testing::ElementsAre(method_name_),
347+
::testing::ElementsAre(server_method_name_),
346348
::testing::AllOf(
347349
::testing::Property(&stats::Distribution::count, i + 1),
348350
::testing::Property(&stats::Distribution::mean,
349351
::testing::DoubleEq(1.0))))));
350352
EXPECT_THAT(server_response_count_view.GetData().distribution_data(),
351353
::testing::UnorderedElementsAre(::testing::Pair(
352-
::testing::ElementsAre(method_name_),
354+
::testing::ElementsAre(server_method_name_),
353355
::testing::AllOf(
354356
::testing::Property(&stats::Distribution::count, i + 1),
355357
::testing::Property(&stats::Distribution::mean,

0 commit comments

Comments
 (0)