Skip to content
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.

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

Schedule option #573

Merged
merged 8 commits into from
Dec 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions api/client/options.proto
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ syntax = "proto3";
package nighthawk.client;

import "google/protobuf/duration.proto";
import "google/protobuf/timestamp.proto";
import "google/protobuf/wrappers.proto";
import "envoy/config/core/v3/base.proto";
import "envoy/config/metrics/v3/stats.proto";
Expand Down Expand Up @@ -219,4 +220,7 @@ message CommandLineOptions {
// "emit_previous_request_delta_in_response_header" to record elapsed time between request
// arrivals.
google.protobuf.StringValue latency_response_header_name = 36;
// Provide an execution starting date and time. Optional, any value specified must be in the
// future.
google.protobuf.Timestamp scheduled_start = 105;
}
1 change: 1 addition & 0 deletions api/client/output.proto
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ message Result {
repeated Statistic statistics = 2;
repeated Counter counters = 3;
google.protobuf.Duration execution_duration = 4;
google.protobuf.Timestamp execution_start = 5;
}

message Output {
Expand Down
3 changes: 2 additions & 1 deletion include/nighthawk/client/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <string>

#include "envoy/common/pure.h"
#include "envoy/common/time.h"
#include "envoy/config/cluster/v3/cluster.pb.h"
#include "envoy/config/core/v3/base.pb.h"
#include "envoy/config/metrics/v3/stats.pb.h"
Expand Down Expand Up @@ -74,7 +75,7 @@ class Options {
virtual std::vector<envoy::config::metrics::v3::StatsSink> statsSinks() const PURE;
virtual uint32_t statsFlushInterval() const PURE;
virtual std::string responseHeaderWithLatencyInput() const PURE;

virtual absl::optional<Envoy::SystemTime> scheduled_start() const PURE;
/**
* Converts an Options instance to an equivalent CommandLineOptions instance in terms of option
* values.
Expand Down
7 changes: 6 additions & 1 deletion include/nighthawk/client/output_collector.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
#include <memory>

#include "envoy/common/pure.h"
#include "envoy/common/time.h"

#include "nighthawk/common/statistic.h"

#include "absl/types/optional.h"

namespace Nighthawk {
namespace Client {

Expand All @@ -23,10 +26,12 @@ class OutputCollector {
* @param statistics Reference to a vector of statistics to add to the output.
* @param counters Reference to a map of counter values, keyed by name, to add to the output.
* @param execution_duration Execution duration associated to the to-be-added result.
* @param first_acquisition_time Timing of the first rate limiter acquisition.
*/
virtual void addResult(absl::string_view name, const std::vector<StatisticPtr>& statistics,
const std::map<std::string, uint64_t>& counters,
const std::chrono::nanoseconds execution_duration) PURE;
const std::chrono::nanoseconds execution_duration,
const absl::optional<Envoy::SystemTime>& first_acquisition_time) PURE;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add comment for first_acquisition_time?

Copy link
Contributor

Choose a reason for hiding this comment

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

will likely be addressed when the comment is added, but I'm not sure i understand what acquisition means in this context.

/**
* Directly sets the output value.
*
Expand Down
4 changes: 2 additions & 2 deletions include/nighthawk/common/factories.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class SequencerFactory {
const SequencerTarget& sequencer_target,
TerminationPredicatePtr&& termination_predicate,
Envoy::Stats::Scope& scope,
const Envoy::SystemTime scheduled_starting_time) const PURE;
const Envoy::MonotonicTime scheduled_starting_time) const PURE;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not actionable, just making sure I'm understanding - this is where we're moving back to MonotonicTime to prevent #569 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is right.

};

class StatisticFactory {
Expand All @@ -46,7 +46,7 @@ class TerminationPredicateFactory {
virtual ~TerminationPredicateFactory() = default;
virtual TerminationPredicatePtr
create(Envoy::TimeSource& time_source, Envoy::Stats::Scope& scope,
const Envoy::SystemTime scheduled_starting_time) const PURE;
const Envoy::MonotonicTime scheduled_starting_time) const PURE;
};

/**
Expand Down
6 changes: 6 additions & 0 deletions include/nighthawk/common/rate_limiter.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ class RateLimiter {
* @return Envoy::TimeSource& time_source used to track time.
*/
virtual Envoy::TimeSource& timeSource() PURE;

/**
* @return absl::optional<Envoy::SystemTime> Time of the first acquisition, if any.
*/
virtual absl::optional<Envoy::SystemTime> firstAcquisitionTime() const PURE;
Copy link
Contributor

Choose a reason for hiding this comment

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

I might have just missed it in this pull request (if so, please point me to the right file), but while I'm seeing code that writes this field or passes it around, I'm not finding where it's actually used functionally. What is its intended purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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


/**
* @return std::chrono::nanoseconds elapsed since the first call to tryAcquireOne(). Used by some
* rate limiter implementations to compute acquisition rate.
Expand Down
6 changes: 6 additions & 0 deletions include/nighthawk/common/sequencer.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "envoy/common/pure.h"

#include "nighthawk/common/operation_callback.h"
#include "nighthawk/common/rate_limiter.h"
#include "nighthawk/common/statistic.h"

namespace Nighthawk {
Expand Down Expand Up @@ -35,6 +36,11 @@ class Sequencer {
*/
virtual std::chrono::nanoseconds executionDuration() const PURE;

/**
* @return RateLimiter& reference to the rate limiter associated to this sequencer.
*/
virtual const RateLimiter& rate_limiter() const PURE;

/**
* @return double an up-to-date completions per second rate.
*/
Expand Down
2 changes: 1 addition & 1 deletion source/client/client_worker_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ ClientWorkerImpl::ClientWorkerImpl(Envoy::Api::Api& api, Envoy::ThreadLocal::Ins
const SequencerFactory& sequencer_factory,
const RequestSourceFactory& request_generator_factory,
Envoy::Stats::Store& store, const int worker_number,
const Envoy::SystemTime starting_time,
const Envoy::MonotonicTime starting_time,
Envoy::Tracing::HttpTracerSharedPtr& http_tracer,
const HardCodedWarmupStyle hardcoded_warmup_style)
: WorkerImpl(api, tls, store),
Expand Down
2 changes: 1 addition & 1 deletion source/client/client_worker_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ClientWorkerImpl : public WorkerImpl, virtual public ClientWorker {
const SequencerFactory& sequencer_factory,
const RequestSourceFactory& request_generator_factory,
Envoy::Stats::Store& store, const int worker_number,
const Envoy::SystemTime starting_time,
const Envoy::MonotonicTime starting_time,
Envoy::Tracing::HttpTracerSharedPtr& http_tracer,
const HardCodedWarmupStyle hardcoded_warmup_style);
StatisticPtrMap statistics() const override;
Expand Down
12 changes: 5 additions & 7 deletions source/client/factories_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,10 @@ BenchmarkClientPtr BenchmarkClientFactoryImpl::create(
SequencerFactoryImpl::SequencerFactoryImpl(const Options& options)
: OptionBasedFactoryImpl(options) {}

SequencerPtr SequencerFactoryImpl::create(Envoy::TimeSource& time_source,
Envoy::Event::Dispatcher& dispatcher,
const SequencerTarget& sequencer_target,
TerminationPredicatePtr&& termination_predicate,
Envoy::Stats::Scope& scope,
const Envoy::SystemTime scheduled_starting_time) const {
SequencerPtr SequencerFactoryImpl::create(
Envoy::TimeSource& time_source, Envoy::Event::Dispatcher& dispatcher,
const SequencerTarget& sequencer_target, TerminationPredicatePtr&& termination_predicate,
Envoy::Stats::Scope& scope, const Envoy::MonotonicTime scheduled_starting_time) const {
StatisticFactoryImpl statistic_factory(options_);
Frequency frequency(options_.requestsPerSecond());
RateLimiterPtr rate_limiter = std::make_unique<ScheduledStartingRateLimiter>(
Expand Down Expand Up @@ -211,7 +209,7 @@ TerminationPredicateFactoryImpl::TerminationPredicateFactoryImpl(const Options&

TerminationPredicatePtr
TerminationPredicateFactoryImpl::create(Envoy::TimeSource& time_source, Envoy::Stats::Scope& scope,
const Envoy::SystemTime scheduled_starting_time) const {
const Envoy::MonotonicTime scheduled_starting_time) const {
// We'll always link a predicate which checks for requests to cancel.
TerminationPredicatePtr root_predicate =
std::make_unique<StatsCounterAbsoluteThresholdTerminationPredicateImpl>(
Expand Down
4 changes: 2 additions & 2 deletions source/client/factories_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class SequencerFactoryImpl : public OptionBasedFactoryImpl, public SequencerFact
SequencerPtr create(Envoy::TimeSource& time_source, Envoy::Event::Dispatcher& dispatcher,
const SequencerTarget& sequencer_target,
TerminationPredicatePtr&& termination_predicate, Envoy::Stats::Scope& scope,
const Envoy::SystemTime scheduled_starting_time) const override;
const Envoy::MonotonicTime scheduled_starting_time) const override;
};

class StatisticFactoryImpl : public OptionBasedFactoryImpl, public StatisticFactory {
Expand Down Expand Up @@ -93,7 +93,7 @@ class TerminationPredicateFactoryImpl : public OptionBasedFactoryImpl,
public:
TerminationPredicateFactoryImpl(const Options& options);
TerminationPredicatePtr create(Envoy::TimeSource& time_source, Envoy::Stats::Scope& scope,
const Envoy::SystemTime scheduled_starting_time) const override;
const Envoy::MonotonicTime scheduled_starting_time) const override;
TerminationPredicate* linkConfiguredPredicates(
TerminationPredicate& last_predicate, const TerminationPredicateMap& predicates,
const TerminationPredicate::Status termination_status, Envoy::Stats::Scope& scope) const;
Expand Down
12 changes: 11 additions & 1 deletion source/client/options_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,12 @@ OptionsImpl::OptionsImpl(const nighthawk::client::CommandLineOptions& options) {
std::copy(options.labels().begin(), options.labels().end(), std::back_inserter(labels_));
latency_response_header_name_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
options, latency_response_header_name, latency_response_header_name_);

if (options.has_scheduled_start()) {
const auto elapsed_since_epoch = std::chrono::nanoseconds(options.scheduled_start().nanos()) +
std::chrono::seconds(options.scheduled_start().seconds());
scheduled_start_ =
Envoy::SystemTime(std::chrono::time_point<std::chrono::system_clock>(elapsed_since_epoch));
}
validate();
}

Expand Down Expand Up @@ -829,6 +834,11 @@ CommandLineOptionsPtr OptionsImpl::toCommandLineOptionsInternal() const {
command_line_options->mutable_stats_flush_interval()->set_value(stats_flush_interval_);
command_line_options->mutable_latency_response_header_name()->set_value(
latency_response_header_name_);
if (scheduled_start_.has_value()) {
*(command_line_options->mutable_scheduled_start()) =
Envoy::ProtobufUtil::TimeUtil::NanosecondsToTimestamp(
scheduled_start_.value().time_since_epoch().count());
}
return command_line_options;
}

Expand Down
2 changes: 2 additions & 0 deletions source/client/options_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class OptionsImpl : public Options, public Envoy::Logger::Loggable<Envoy::Logger
std::string responseHeaderWithLatencyInput() const override {
return latency_response_header_name_;
};
absl::optional<Envoy::SystemTime> scheduled_start() const override { return scheduled_start_; }

private:
void parsePredicates(const TCLAP::MultiArg<std::string>& arg,
Expand Down Expand Up @@ -149,6 +150,7 @@ class OptionsImpl : public Options, public Envoy::Logger::Loggable<Envoy::Logger
std::vector<envoy::config::metrics::v3::StatsSink> stats_sinks_;
uint32_t stats_flush_interval_{5};
std::string latency_response_header_name_;
absl::optional<Envoy::SystemTime> scheduled_start_;
};

} // namespace Client
Expand Down
15 changes: 11 additions & 4 deletions source/client/output_collector_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,19 @@ OutputCollectorImpl::OutputCollectorImpl(Envoy::TimeSource& time_source, const O

nighthawk::client::Output OutputCollectorImpl::toProto() const { return output_; }

void OutputCollectorImpl::addResult(absl::string_view name,
const std::vector<StatisticPtr>& statistics,
const std::map<std::string, uint64_t>& counters,
const std::chrono::nanoseconds execution_duration) {
void OutputCollectorImpl::addResult(
absl::string_view name, const std::vector<StatisticPtr>& statistics,
const std::map<std::string, uint64_t>& counters,
const std::chrono::nanoseconds execution_duration,
const absl::optional<Envoy::SystemTime>& first_acquisition_time) {
auto result = output_.add_results();
result->set_name(name.data(), name.size());
if (first_acquisition_time.has_value()) {
*(result->mutable_execution_start()) = Envoy::Protobuf::util::TimeUtil::NanosecondsToTimestamp(
std::chrono::duration_cast<std::chrono::nanoseconds>(
first_acquisition_time.value().time_since_epoch())
.count());
}
for (auto& statistic : statistics) {
// 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
Expand Down
3 changes: 2 additions & 1 deletion source/client/output_collector_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ class OutputCollectorImpl : public OutputCollector {

void addResult(absl::string_view name, const std::vector<StatisticPtr>& statistics,
const std::map<std::string, uint64_t>& counters,
const std::chrono::nanoseconds execution_duration) override;
const std::chrono::nanoseconds execution_duration,
const absl::optional<Envoy::SystemTime>& first_acquisition_time) override;
void setOutput(const nighthawk::client::Output& output) override { output_ = output; }

nighthawk::client::Output toProto() const override;
Expand Down
Loading