Skip to content

Commit

Permalink
always use latest exit span to inject propagation header (#89)
Browse files Browse the repository at this point in the history
  • Loading branch information
Shikugawa authored Mar 11, 2022
1 parent 34053a0 commit 838aa58
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 31 deletions.
5 changes: 0 additions & 5 deletions cpp2sky/tracing_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,14 +234,9 @@ class TracingContext {

/**
* Generate sw8 value to send SegmentRef.
* @param parent Parent span that belongs to current segment.
* @param target_address Target address to send request. For more detail:
* https://github.com/apache/skywalking-data-collect-protocol/blob/master/language-agent/Tracing.proto#L97-L101
*/
virtual std::optional<std::string> createSW8HeaderValue(
TracingSpanPtr parent, const std::string_view target_address) = 0;
// If you don't specify parent span, stored to current segment, it will be
// selected newest span as parent span.
virtual std::optional<std::string> createSW8HeaderValue(
const std::string_view target_address) = 0;

Expand Down
6 changes: 3 additions & 3 deletions example/sample_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ int main() {
* httplib::Client cli("remote", 8082);
* httplib::Headers headers = {
* {kPropagationHeader.data(),
* tracing_context->createSW8HeaderValue(current_span, "remote:8082")}};
* tracing_context->createSW8HeaderValue("remote:8082")}};
*
* auto res = cli.Get("/ping", headers);
*
Expand All @@ -74,8 +74,8 @@ int main() {

httplib::Client cli("127.0.0.1", 8081);
httplib::Headers headers = {
{kPropagationHeader.data(), *tracing_context->createSW8HeaderValue(
exit_span.get(), target_address)}};
{kPropagationHeader.data(),
*tracing_context->createSW8HeaderValue(target_address)}};

auto res = cli.Get("/ping", headers);
}
Expand Down
12 changes: 2 additions & 10 deletions source/tracing_context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,8 @@ TracingSpanPtr TracingContextImpl::createEntrySpan() {
}

std::optional<std::string> TracingContextImpl::createSW8HeaderValue(
TracingSpanPtr parent_span, const std::string_view target_address) {
TracingSpanPtr target_span = parent_span;
if (target_span == nullptr) {
if (spans_.empty()) {
throw TracerException(
"Can't create propagation header because current segment has no "
"valid span.");
}
target_span = spans_.back();
}
const std::string_view target_address) {
auto target_span = spans_.back();
if (target_span->spanType() != skywalking::v3::SpanType::Exit) {
return std::nullopt;
}
Expand Down
5 changes: 0 additions & 5 deletions source/tracing_context_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,6 @@ class TracingContextImpl : public TracingContext {

TracingSpanPtr createEntrySpan() override;
std::optional<std::string> createSW8HeaderValue(
const std::string_view target_address) override {
return createSW8HeaderValue(nullptr, target_address);
}
std::optional<std::string> createSW8HeaderValue(
TracingSpanPtr parent_span,
const std::string_view target_address) override;
skywalking::v3::SegmentObject createSegmentObject() override;
void setSkipAnalysis() override { should_skip_analysis_ = true; }
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ int main() {

httplib::Client cli("consumer", 8080);
httplib::Headers headers = {
{kPropagationHeader.data(), *tracing_context->createSW8HeaderValue(
exit_span.get(), target_address)}};
{kPropagationHeader.data(),
*tracing_context->createSW8HeaderValue(target_address)}};
auto res = cli.Get("/pong", headers);
}
}
Expand All @@ -75,8 +75,8 @@ int main() {

httplib::Client cli("interm", 8082);
httplib::Headers headers = {
{kPropagationHeader.data(), *tracing_context->createSW8HeaderValue(
exit_span.get(), target_address)}};
{kPropagationHeader.data(),
*tracing_context->createSW8HeaderValue(target_address)}};
auto res = cli.Get("/users", headers);
}
}
Expand Down
8 changes: 4 additions & 4 deletions test/tracing_context_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ TEST_F(TracingContextTest, SW8CreateTest) {
std::string target_address("10.0.0.1:443");

// Entry span should be rejected as propagation context
EXPECT_FALSE(sc.createSW8HeaderValue(span, target_address).has_value());
EXPECT_FALSE(sc.createSW8HeaderValue(target_address).has_value());

auto span2 = sc.createExitSpan(span);

Expand All @@ -341,7 +341,7 @@ TEST_F(TracingContextTest, SW8CreateTest) {
std::string expect_sw8(
"1-MQ==-dXVpZA==-1-bWVzaA==-c2VydmljZV8w-c2FtcGxlMQ==-MTAuMC4wLjE6NDQz");

EXPECT_EQ(expect_sw8, *sc.createSW8HeaderValue(span2, target_address));
EXPECT_EQ(expect_sw8, *sc.createSW8HeaderValue(target_address));

std::vector<char> target_address_based_vector;
target_address_based_vector.reserve(target_address.size() * 2);
Expand All @@ -354,14 +354,14 @@ TEST_F(TracingContextTest, SW8CreateTest) {

EXPECT_EQ(target_address_based_vector.size(), target_address.size());
EXPECT_EQ(expect_sw8,
*sc.createSW8HeaderValue(span2, target_address_based_vector_view));
*sc.createSW8HeaderValue(target_address_based_vector_view));

// Make sure that the end of target_address_based_vector_view is not '\0'. We
// reserve enough memory for target_address_based_vector, so push back will
// not cause content to be re-allocated.
target_address_based_vector.push_back('x');
EXPECT_EQ(expect_sw8,
*sc.createSW8HeaderValue(span2, target_address_based_vector_view));
*sc.createSW8HeaderValue(target_address_based_vector_view));
}

TEST_F(TracingContextTest, ReadyToSendTest) {
Expand Down

0 comments on commit 838aa58

Please sign in to comment.