Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
81 commits
Select commit Hold shift + click to select a range
99b7595
Fortio output: add version field
oschaaf Dec 11, 2019
7c267eb
Fix ambiguous include path
oschaaf Dec 11, 2019
88f2e03
Swap ot fmt and use absl::StrCat instead
oschaaf Dec 11, 2019
6f23597
Review feedback
oschaaf Dec 11, 2019
a8e8f50
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Dec 11, 2019
aa28310
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Dec 12, 2019
dbab3b4
Review feedback + iterate on release procedure
oschaaf Dec 12, 2019
5cc8a79
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Dec 13, 2019
315be10
Consolidate version in the output
oschaaf Dec 14, 2019
640ea06
Back out accidentally comitted file
oschaaf Dec 14, 2019
8e93a74
Fix achieved qps / duration / connections / qps
oschaaf Dec 14, 2019
e9306fa
Remove TODO(#186)
oschaaf Dec 14, 2019
4568133
Add missing fields to the proto definition
oschaaf Dec 14, 2019
6076b70
Define & set min/max/sum/avg/stdev/percentiles
oschaaf Dec 15, 2019
8a6bee0
Back out RELEASE_PROCEDURE.md
oschaaf Dec 15, 2019
3a1f428
Fix error ratio
oschaaf Dec 15, 2019
98b91e0
Stub the jitter flag with a TODO
oschaaf Dec 15, 2019
ba703cb
Add doc comments for durationToSeconds()
oschaaf Dec 15, 2019
28f2f38
Fix start time precision, clean up the diff
oschaaf Dec 15, 2019
2acaa34
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Dec 16, 2019
a8c5c0f
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Dec 18, 2019
7de573f
Review: VersionString as a top-level function
oschaaf Dec 18, 2019
08bd9e5
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Dec 19, 2019
abddf23
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Dec 23, 2019
716e3f2
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Jan 3, 2020
576b15b
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Jan 4, 2020
f7bdbb1
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Jan 6, 2020
a0f557b
Fix jitter output & add RunType, test version
oschaaf Jan 7, 2020
3abd912
Fix fortio percentile data
oschaaf Jan 7, 2020
d3de072
Fix output formatter expectation
oschaaf Jan 7, 2020
5bd89b1
Update Envoy
oschaaf Jan 9, 2020
6b25f86
Regenerate README.md
oschaaf Jan 9, 2020
ccfe682
Upgrade and fix transport socket config
oschaaf Jan 9, 2020
20f07d6
Upgrade and fix UpstreamTlsContext
oschaaf Jan 9, 2020
6748422
Upgrade the remaineder & clean up
oschaaf Jan 9, 2020
2346949
Clean up the diff
oschaaf Jan 9, 2020
ce9f3c1
Merge branch 'envoy-dep-update-8' into fortio-version-field
oschaaf Jan 9, 2020
5f28389
Add minimal structured versioning support
oschaaf Jan 9, 2020
24782ea
Fix command to avoid check_format flagging code
oschaaf Jan 9, 2020
f7b8eaf
Fix includes & small tweaks
oschaaf Jan 9, 2020
111aee5
Amend according to clang-tidy suggestion
oschaaf Jan 9, 2020
cbb70f6
Wire in new stats & save state.
oschaaf Jan 9, 2020
de90843
save state
oschaaf Jan 9, 2020
bc81751
Save state before switching context
oschaaf Jan 9, 2020
812df3e
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Jan 10, 2020
8239486
Merge remote-tracking branch 'upstream/master' into envoy-dep-update-8
oschaaf Jan 10, 2020
a5fec96
Merge remote-tracking branch 'upstream/master' into response-size-tra…
oschaaf Jan 10, 2020
cfcf7bc
Save state
oschaaf Jan 10, 2020
7cb8866
Some cleanup
oschaaf Jan 10, 2020
23dc8ae
Merge remote-tracking branch 'upstream/master' into envoy-dep-update-8
oschaaf Jan 10, 2020
838dcdd
Merge branch 'envoy-dep-update-8' into fortio-version-field
oschaaf Jan 10, 2020
5578d27
Merge remote-tracking branch 'upstream/master' into response-size-tra…
oschaaf Jan 10, 2020
2840070
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Jan 10, 2020
6b341fc
Fix TODO comment / link to issue number
oschaaf Jan 10, 2020
f150f6a
Fortio conn. count fix for experimental h2 pool
oschaaf Jan 10, 2020
0d69b89
save state
oschaaf Jan 11, 2020
cd00a6b
Fix Sizes/HeaderSizes. More tests. Fix transforms.
oschaaf Jan 11, 2020
85df631
Add test / enhance test
oschaaf Jan 11, 2020
9bb43a9
Enhance test
oschaaf Jan 11, 2020
49bf48f
Whoops
oschaaf Jan 11, 2020
b1cd2c3
Fix & simplify StatisticImpl::toString()
oschaaf Jan 11, 2020
f218d70
Document the interface
oschaaf Jan 11, 2020
4303f0c
Fix StreamingStatistic bug + add NullStatistic
oschaaf Jan 11, 2020
2ad69d0
Add some comments in output.proto
oschaaf Jan 11, 2020
fb4e6ce
More code comments
oschaaf Jan 11, 2020
61c5a9d
Deduplicate code in output formatter
oschaaf Jan 11, 2020
ccaa54b
Tidy up the dotted string output formatter
oschaaf Jan 11, 2020
dca3c73
Merge branch 'fortio-version-field' into response-size-tracking
oschaaf Jan 11, 2020
c2f9798
Fix a typo
oschaaf Jan 12, 2020
858c232
python test: add size histograms expectations
oschaaf Jan 12, 2020
13b5222
Address review feedback
oschaaf Jan 13, 2020
a5dbace
Review feedback: remove enum from proto
oschaaf Jan 14, 2020
de3fb47
Merge remote-tracking branch 'upstream/master' into fortio-version-field
oschaaf Jan 14, 2020
2ac3386
Sync up with earlier Fortio output conversion work
oschaaf Jan 14, 2020
ac675da
Merge remote-tracking branch 'origin/fortio-version-field' into respo…
oschaaf Jan 14, 2020
d1aee22
Merge remote-tracking branch 'upstream/master' into response-size-tra…
oschaaf Jan 15, 2020
77a7eb2
Add doc comments as promised
oschaaf Jan 15, 2020
6a79510
Merge remote-tracking branch 'upstream/master' into response-size-tra…
oschaaf Jan 15, 2020
95fb0df
Merge remote-tracking branch 'upstream/master' into response-size-tra…
oschaaf Jan 16, 2020
5082cfb
Merge remote-tracking branch 'upstream/master' into response-size-tra…
oschaaf Jan 17, 2020
9aa9e98
Review feedback
oschaaf Jan 21, 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
23 changes: 20 additions & 3 deletions api/client/output.proto
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,34 @@ message Counter {
}

message Percentile {
google.protobuf.Duration duration = 1;
oneof duration_type {
Comment thread
htuch marked this conversation as resolved.
google.protobuf.Duration duration = 1;
double raw_value = 4;
}
double percentile = 2;
uint64 count = 3;
}

message Statistic {
uint64 count = 1;
string id = 2;
google.protobuf.Duration mean = 3;
google.protobuf.Duration pstdev = 4;
oneof mean_type {
google.protobuf.Duration mean = 3;
double raw_mean = 8;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if "raw_" is the best prefix / name. Can we try to document what is the meaning of raw in each one of these cases and then we will see? Is it always bytes? Something else?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The definition of the _raw post-fixed fields map 1:1 to the return types of the corresponding getters from our internal Statistics c++ interface. In this PR we will use them to track the number of header bytes and response bytes, but possibly we may want to re-use these in the future to track things that aren't a "number of bytes"? (e.g. track number of requests over a single connection)

}
oneof pstdev_type {
google.protobuf.Duration pstdev = 4;
double raw_pstdev = 10;
}
repeated Percentile percentiles = 5;
oneof min_type {
google.protobuf.Duration min = 6;
uint64 raw_min = 12;
}
oneof max_type {
google.protobuf.Duration max = 7;
uint64 raw_max = 13;
}
}

message Result {
Expand Down
33 changes: 31 additions & 2 deletions api/client/transform/fortio.proto
Original file line number Diff line number Diff line change
Expand Up @@ -24,37 +24,66 @@ message FortioResult {
// concatenated into this field, using ' ' as a separator.
string Labels = 1;

// Start time of the load test execution.
google.protobuf.Timestamp StartTime = 2;

// Configured qps
uint32 RequestedQPS = 3;

// Configured duration
google.protobuf.Duration RequestedDuration = 4;

// Effective qps
double ActualQPS = 5;

// Effective duration
double ActualDuration = 6;

// The configured number of used connections
uint32 NumThreads = 7;

// Latency histogram
DurationHistogram DurationHistogram = 8;

// Fortio tracks per-return-code. We group by 2xx,3xx, etc.
map<string, uint64> RetCodes = 11;

// Effective URI.
string URL = 12;

// (Unstructured) version. We serialize a representation of the Nighthawk version into this field.
string Version = 13;

// Was jitter used Y/N.
bool Jitter = 14;

// Run type. Can be "HTTP", "GRPC", and an empty string has also been observed.
// We only set HTTP for now.
string RunType = 15;

// Exactly can be used to stop fortio after a fixed amount of replies.
// Sizes and HeaderSizes seems to mean different things in Fortio depending on if the go stdclient
// or DIY fastclient was used. We populate this field with response body sizes, and HeaderSizes
// with header sizes. Both are tracked in bytes. Note: we don't use full histograms here, but
// rather rely on StreamingStatistic. This means we don't populate percentiles, and only populate
// min/mean/max/etc with respect to DurationHistogram.
DurationHistogram Sizes = 16;

// See the 'Sizes' field for detals.
DurationHistogram HeaderSizes = 17;

// Some notes on fields that we do not populate today:

// "Exactly" can be used to stop fortio after a fixed amount of replies.
// We don't seem to need this, and going forward it might be hard/annoying to
// translate this field as Nighthawk's concepts diverge a bit. If end up
// needing to we can probably inspect the main phase and then figure out if there's a
// termination predicate configured for a fixed number of responses observed
// via stats. For now, however, skip this field.
// uint64 Exactly = 16;

// We don't populate SocketCount (sometimes used to track #connections)

// We don't populate AbortOn (a status code that may abort termination) for reasons
// similar to Exactly.
}

message DurationHistogram {
Expand Down
38 changes: 36 additions & 2 deletions include/nighthawk/common/statistic.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,51 @@ using StatisticPtrMap = std::map<std::string, Statistic const*>;
*/
class Statistic : Envoy::NonCopyable {
public:
enum class SerializationDomain { RAW, DURATION };

virtual ~Statistic() = default;

/**
* Method for adding a sample value.
* @param value the value of the sample to add
*/
virtual void addValue(uint64_t sample_value) PURE;

/**
* @return uint64_t The number of sampled values.
*/
virtual uint64_t count() const PURE;

/**
* @return double Mean derived from the sampled values.
*/
virtual double mean() const PURE;

/**
* @return double Variance derived from the sampled values.
*/
virtual double pvariance() const PURE;

/**
* @return double Standard deviation derived from the sampled values.
*/
virtual double pstdev() const PURE;

/**
* @return uint64_t The smallest sampled value.
*/
virtual uint64_t min() const PURE;

/**
* @return uint64_t The largest sampled value.
*/
virtual uint64_t max() const PURE;

/**
* @return StatisticPtr Yields a new instance of the same type as the instance this is called on.
*/
virtual StatisticPtr createNewInstanceOfSameType() const PURE;

/**
* Only used in tests to match expectations to the right precision level.
* @return uint64_t the number of significant digits. 0 is assumed to be max precision.
Expand All @@ -51,14 +84,15 @@ class Statistic : Envoy::NonCopyable {
virtual bool resistsCatastrophicCancellation() const { return false; }

/**
* @return std::string a representation of the statistic as a std::string.
* @return std::string Gets a string representation of the statistic as a std::string.
*/
virtual std::string toString() const PURE;

/**
* @param domain Used to indicate if serialization should represent durations or raw values.
* @return nighthawk::client::Statistic a representation of the statistic as a protobuf message.
*/
virtual nighthawk::client::Statistic toProto() PURE;
virtual nighthawk::client::Statistic toProto(SerializationDomain domain) const PURE;

/**
* Combines two Statistics into one, and returns a new, merged, Statistic.
Expand Down
22 changes: 15 additions & 7 deletions source/client/benchmark_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,24 @@ void Http2PoolImpl::checkForDrained() {

BenchmarkClientHttpImpl::BenchmarkClientHttpImpl(
Envoy::Api::Api& api, Envoy::Event::Dispatcher& dispatcher, Envoy::Stats::Scope& scope,
StatisticPtr&& connect_statistic, StatisticPtr&& response_statistic, bool use_h2,
Envoy::Upstream::ClusterManagerPtr& cluster_manager, Envoy::Tracing::HttpTracerPtr& http_tracer,
absl::string_view cluster_name, RequestGenerator request_generator,
const bool provide_resource_backpressure)
StatisticPtr&& connect_statistic, StatisticPtr&& response_statistic,
StatisticPtr&& response_header_size_statistic, StatisticPtr&& response_body_size_statistic,
bool use_h2, Envoy::Upstream::ClusterManagerPtr& cluster_manager,
Envoy::Tracing::HttpTracerPtr& http_tracer, absl::string_view cluster_name,
RequestGenerator request_generator, const bool provide_resource_backpressure)
: api_(api), dispatcher_(dispatcher), scope_(scope.createScope("benchmark.")),
connect_statistic_(std::move(connect_statistic)),
response_statistic_(std::move(response_statistic)), use_h2_(use_h2),
response_statistic_(std::move(response_statistic)),
response_header_size_statistic_(std::move(response_header_size_statistic)),
response_body_size_statistic_(std::move(response_body_size_statistic)), use_h2_(use_h2),
benchmark_client_stats_({ALL_BENCHMARK_CLIENT_STATS(POOL_COUNTER(*scope_))}),
cluster_manager_(cluster_manager), http_tracer_(http_tracer),
cluster_name_(std::string(cluster_name)), request_generator_(std::move(request_generator)),
provide_resource_backpressure_(provide_resource_backpressure) {
connect_statistic_->setId("benchmark_http_client.queue_to_connect");
response_statistic_->setId("benchmark_http_client.request_to_response");
response_header_size_statistic_->setId("benchmark_http_client.response_header_size");
response_body_size_statistic_->setId("benchmark_http_client.response_body_size");
}

void BenchmarkClientHttpImpl::terminate() {
Expand All @@ -119,6 +124,8 @@ StatisticPtrMap BenchmarkClientHttpImpl::statistics() const {
StatisticPtrMap statistics;
statistics[connect_statistic_->id()] = connect_statistic_.get();
statistics[response_statistic_->id()] = response_statistic_.get();
statistics[response_header_size_statistic_->id()] = response_header_size_statistic_.get();
statistics[response_body_size_statistic_->id()] = response_body_size_statistic_.get();
return statistics;
};

Expand Down Expand Up @@ -157,8 +164,9 @@ bool BenchmarkClientHttpImpl::tryStartRequest(CompletionCallback caller_completi
std::string x_request_id = generator_.uuid();
auto stream_decoder = new StreamDecoder(
dispatcher_, api_.timeSource(), *this, std::move(caller_completion_callback),
*connect_statistic_, *response_statistic_, request->header(), measureLatencies(),
content_length, x_request_id, http_tracer_);
*connect_statistic_, *response_statistic_, *response_header_size_statistic_,
*response_body_size_statistic_, request->header(), measureLatencies(), content_length,
x_request_id, http_tracer_);
requests_initiated_++;
pool_ptr->newStream(*stream_decoder, *stream_decoder);
return true;
Expand Down
6 changes: 5 additions & 1 deletion source/client/benchmark_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ class BenchmarkClientHttpImpl : public BenchmarkClient,
public:
BenchmarkClientHttpImpl(Envoy::Api::Api& api, Envoy::Event::Dispatcher& dispatcher,
Envoy::Stats::Scope& scope, StatisticPtr&& connect_statistic,
StatisticPtr&& response_statistic, bool use_h2,
StatisticPtr&& response_statistic,
StatisticPtr&& response_header_size_statistic,
StatisticPtr&& response_body_size_statistic, bool use_h2,
Envoy::Upstream::ClusterManagerPtr& cluster_manager,
Envoy::Tracing::HttpTracerPtr& http_tracer,
absl::string_view cluster_name, RequestGenerator request_generator,
Expand Down Expand Up @@ -161,6 +163,8 @@ class BenchmarkClientHttpImpl : public BenchmarkClient,
// destruction when tls has been involved during usage.
StatisticPtr connect_statistic_;
StatisticPtr response_statistic_;
StatisticPtr response_header_size_statistic_;
StatisticPtr response_body_size_statistic_;
const bool use_h2_;
std::chrono::seconds timeout_{5s};
uint32_t connection_limit_{1};
Expand Down
9 changes: 8 additions & 1 deletion source/client/factories_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,15 @@ BenchmarkClientPtr BenchmarkClientFactoryImpl::create(
Envoy::Upstream::ClusterManagerPtr& cluster_manager, Envoy::Tracing::HttpTracerPtr& http_tracer,
absl::string_view cluster_name, RequestSource& request_generator) const {
StatisticFactoryImpl statistic_factory(options_);
// While we lack options to configure which statistic backend goes where, we directly pass
// StreamingStatistic for the stats that track response sizes. Ideally we would have options
// for this to route the right stat to the right backend (HdrStatistic, SimpleStatistic,
// NullStatistic).
// TODO(#292): Create options and have the StatisticFactory consider those when instantiating
// statistics.
auto benchmark_client = std::make_unique<BenchmarkClientHttpImpl>(
api, dispatcher, scope, statistic_factory.create(), statistic_factory.create(), options_.h2(),
api, dispatcher, scope, statistic_factory.create(), statistic_factory.create(),
std::make_unique<StreamingStatistic>(), std::make_unique<StreamingStatistic>(), options_.h2(),
cluster_manager, http_tracer, cluster_name, request_generator.get(), !options_.openLoop());
auto request_options = options_.toCommandLineOptions()->request_options();
benchmark_client->setConnectionLimit(options_.connections());
Expand Down
9 changes: 8 additions & 1 deletion source/client/output_collector_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,14 @@ void OutputCollectorImpl::addResult(absl::string_view name,
auto result = output_.add_results();
result->set_name(name.data(), name.size());
for (auto& statistic : statistics) {
*(result->add_statistics()) = statistic->toProto();
// TODO(#292): Looking at if the statistic id ends with "_size" to determine how it should be
// serialized is kind of hacky. Maybe we should have a lookup table of sorts, to determine how
// statistics should we serialized. Doing so may give us a canonical place to consolidate their
// ids as well too.
Statistic::SerializationDomain serialization_domain =
absl::EndsWith(statistic->id(), "_size") ? Statistic::SerializationDomain::RAW
: Statistic::SerializationDomain::DURATION;
*(result->add_statistics()) = statistic->toProto(serialization_domain);
}
for (const auto& counter : counters) {
auto new_counters = result->add_counters();
Expand Down
Loading