Skip to content

tracer: Improve test coverage for x-ray #10890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 5 commits into from
Apr 23, 2020
Merged

tracer: Improve test coverage for x-ray #10890

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e828a07
Improve test coverage
marcomagdy Apr 21, 2020
c22e9c8
revert copying the payload into the buffer
marcomagdy Apr 22, 2020
057e382
Verify UDP packet contents
marcomagdy Apr 23, 2020
cfe071e
Use dynamic port
marcomagdy Apr 23, 2020
e5eb6a6
Parameterize the test for IPv4 and IPv6
marcomagdy Apr 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions source/extensions/tracers/xray/daemon_broker.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "common/buffer/buffer_impl.h"
#include "common/network/utility.h"
#include "common/protobuf/utility.h"

#include "source/extensions/tracers/xray/daemon.pb.h"

Expand All @@ -20,13 +21,8 @@ std::string createHeader(const std::string& format, uint32_t version) {
source::extensions::tracers::xray::daemon::Header header;
header.set_format(format);
header.set_version(version);

Protobuf::util::JsonPrintOptions json_options;
json_options.preserve_proto_field_names = true;
std::string json;
const auto status = Protobuf::util::MessageToJsonString(header, &json, json_options);
ASSERT(status.ok());
return json;
return MessageUtil::getJsonStringFromMessage(header, false /* pretty_print */,
false /* always_print_primitive_fields */);
}

} // namespace
Expand All @@ -40,11 +36,9 @@ void DaemonBrokerImpl::send(const std::string& data) const {
constexpr auto version = 1;
constexpr auto format = "json";
const std::string payload = absl::StrCat(createHeader(format, version), "\n", data);
Buffer::RawSlice buf;
buf.mem_ = const_cast<char*>(payload.data());
buf.len_ = payload.length();
const auto rc = Network::Utility::writeToSocket(*io_handle_, &buf, 1 /*num_slices*/,
nullptr /*local_ip*/, *address_);
Buffer::OwnedImpl buf(payload);
Comment thread
marcomagdy marked this conversation as resolved.
Outdated
const auto rc =
Network::Utility::writeToSocket(*io_handle_, buf, nullptr /*local_ip*/, *address_);

if (rc.rc_ != payload.length()) {
// TODO(marcomagdy): report this in stats
Expand Down
6 changes: 0 additions & 6 deletions source/extensions/tracers/xray/localized_sampling.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,7 @@ class LocalizedSamplingRule {
* Set the percentage of requests to sample _after_ sampling |fixed_target| requests per second.
*/
void setRate(double rate) { rate_ = rate; }

const std::string& host() const { return host_; }
const std::string& httpMethod() const { return http_method_; }
const std::string& urlPath() const { return url_path_; }
uint32_t fixedTarget() const { return fixed_target_; }
double rate() const { return rate_; }
const Reservoir& reservoir() const { return reservoir_; }
Reservoir& reservoir() { return reservoir_; }

private:
Expand Down
22 changes: 8 additions & 14 deletions source/extensions/tracers/xray/tracer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,31 +71,25 @@ void Span::finishSpan() {

daemon::Segment s;
s.set_name(name());
s.set_id(Id());
s.set_id(id());
s.set_trace_id(traceId());
s.set_start_time(time_point_cast<SecondsWithFraction>(startTime()).time_since_epoch().count());
s.set_end_time(
time_point_cast<SecondsWithFraction>(time_source_.systemTime()).time_since_epoch().count());
s.set_parent_id(parentId());

// HTTP annotations
using StructField = Protobuf::MapPair<std::string, ProtobufWkt::Value>;

ProtobufWkt::Struct* request = s.mutable_http()->mutable_request();
auto* request_fields = request->mutable_fields();
auto* request_fields = s.mutable_http()->mutable_request()->mutable_fields();
for (const auto& field : http_request_annotations_) {
request_fields->insert(StructField{field.first, field.second});
request_fields->insert({field.first, field.second});
}

ProtobufWkt::Struct* response = s.mutable_http()->mutable_response();
auto* response_fields = response->mutable_fields();
auto* response_fields = s.mutable_http()->mutable_response()->mutable_fields();
for (const auto& field : http_response_annotations_) {
response_fields->insert(StructField{field.first, field.second});
response_fields->insert({field.first, field.second});
}

using KeyValue = Protobuf::Map<std::string, std::string>::value_type;
for (const auto& item : custom_annotations_) {
s.mutable_annotations()->insert(KeyValue{item.first, item.second});
s.mutable_annotations()->insert({item.first, item.second});
}

const std::string json = MessageUtil::getJsonStringFromMessage(
Expand All @@ -106,7 +100,7 @@ void Span::finishSpan() {

void Span::injectContext(Http::RequestHeaderMap& request_headers) {
const std::string xray_header_value =
fmt::format("Root={};Parent={};Sampled={}", traceId(), Id(), sampled() ? "1" : "0");
fmt::format("Root={};Parent={};Sampled={}", traceId(), id(), sampled() ? "1" : "0");
request_headers.setCopy(Http::LowerCaseString(XRayTraceHeader), xray_header_value);
}

Expand All @@ -118,7 +112,7 @@ Tracing::SpanPtr Span::spawnChild(const Tracing::Config&, const std::string& ope
child_span->setName(name());
child_span->setOperation(operation_name);
child_span->setStartTime(start_time);
child_span->setParentId(Id());
child_span->setParentId(id());
child_span->setTraceId(traceId());
child_span->setSampled(sampled());
return child_span;
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/tracers/xray/tracer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
/**
* Gets this Span's ID.
*/
const std::string& Id() const { return id_; }
const std::string& id() const { return id_; }

const std::string& parentId() const { return parent_segment_id_; }

Expand Down
16 changes: 13 additions & 3 deletions test/extensions/tracers/xray/tracer_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,15 @@ TEST_F(XRayTracerTest, ChildSpanHasParentInfo) {
absl::nullopt /*headers*/);

const XRay::Span* xray_parent_span = static_cast<XRay::Span*>(parent_span.get());
const std::string expected_parent_id = xray_parent_span->Id();
auto on_send = [&](const std::string& json) {
const std::string expected_parent_id = xray_parent_span->id();
auto on_send = [xray_parent_span, expected_parent_id](const std::string& json) {
ASSERT_FALSE(json.empty());
daemon::Segment s;
MessageUtil::loadFromJson(json, s, ProtobufMessage::getNullValidationVisitor());
ASSERT_STREQ(expected_parent_id.c_str(), s.parent_id().c_str());
ASSERT_STREQ(expected_span_name, s.name().c_str());
ASSERT_STREQ(xray_parent_span->traceId().c_str(), s.trace_id().c_str());
ASSERT_STRNE(xray_parent_span->Id().c_str(), s.id().c_str());
ASSERT_STRNE(xray_parent_span->id().c_str(), s.id().c_str());
};

EXPECT_CALL(broker, send(_)).WillOnce(Invoke(on_send));
Expand Down Expand Up @@ -183,6 +183,16 @@ TEST_F(XRayTracerTest, TraceIDFormatTest) {
ASSERT_EQ(24, parts[2].length());
}

TEST(XRayDaemon, SendBytesOverUDP) {
Comment thread
marcomagdy marked this conversation as resolved.
Outdated
NiceMock<Server::MockInstance> server;
DaemonBrokerPtr broker_ptr = std::make_unique<DaemonBrokerImpl>("127.0.0.1:2000");
Tracer tracer{"my_segment", std::move(broker_ptr), server.timeSource()};
auto span = tracer.startSpan("ingress" /*operation name*/, server.timeSource().systemTime(),
absl::nullopt /*headers*/);
span->setTag("user_agent", "envoy");
span->finishSpan();
}

} // namespace
} // namespace XRay
} // namespace Tracers
Expand Down