Skip to content

stats: DogStatsd sink supports custom metric name prefix #4506

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 4 commits into from
Oct 3, 2018
Merged

stats: DogStatsd sink supports custom metric name prefix #4506

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c7e8dd1
stats: DogStatsd sink supports custom name prefix
Sep 23, 2018
36980c5
Test custom prefix
Oct 2, 2018
64bf276
Merge remote-tracking branch 'origin/master' into dogstatsd-sink-supp…
Oct 3, 2018
561dfe4
Reword prefix document
Oct 3, 2018
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/envoy/config/metrics/v2/stats.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ message DogStatsdSink {
}

reserved 2;

// Optional custom metric name prefix the same as :ref:`StatsdSink's prefix field

@mrice32 mrice32 Sep 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: maybe rephrase to something like:

// Optional custom metric name prefix. See 
// :ref:`StatsdSink's prefix field <envoy_api_field_config.metrics.v2.StatsdSink.prefix>`
// for more details.

@mrice32 mrice32 Oct 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reminder to fix this nit :)

@taiki45 taiki45 Oct 3, 2018

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.

Oh, I though I fixed it... I'll fix it now.

@taiki45 taiki45 Oct 3, 2018

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.

Thanks, now I fixed this.

// <envoy_api_field_config.metrics.v2.StatsdSink.prefix>`.
string prefix = 3;
}

// Stats configuration proto schema for built-in *envoy.stat_sinks.hystrix* sink.
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ Version history
* rest-api: added ability to set the :ref:`request timeout <envoy_api_field_core.ApiConfigSource.request_timeout>` for REST API requests.
* route checker: Added v2 config support and removed support for v1 configs.
* router: added ability to set request/response headers at the :ref:`envoy_api_msg_route.Route` level.
* stats: added :ref:`option to configure the DogStatsD metric name prefix<envoy_api_field_config.metrics.v2.DogStatsdSink.prefix>` to DogStatsdSink.
* tcp_proxy: added support for :ref:`weighted clusters <envoy_api_field_config.filter.network.tcp_proxy.v2.TcpProxy.weighted_clusters>`.
* thrift_proxy: introduced thrift routing, moved configuration to correct location
* thrift_proxy: introduced thrift configurable decoder filters
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/stat_sinks/dog_statsd/config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Stats::SinkPtr DogStatsdSinkFactory::createStatsSink(const Protobuf::Message& co
Network::Address::resolveProtoAddress(sink_config.address());
ENVOY_LOG(debug, "dog_statsd UDP ip address: {}", address->asString());
return std::make_unique<Common::Statsd::UdpStatsdSink>(server.threadLocal(), std::move(address),
true);
true, sink_config.prefix());
}

ProtobufTypes::MessagePtr DogStatsdSinkFactory::createEmptyConfigProto() {
Expand Down
16 changes: 16 additions & 0 deletions test/extensions/stats_sinks/common/statsd/statsd_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,22 @@ TEST_F(TcpStatsdSinkTest, BasicFlow) {
tls_.shutdownThread();
}

TEST_F(TcpStatsdSinkTest, WithCustomPrefix) {
sink_.reset(new TcpStatsdSink(local_info_, "fake_cluster", tls_, cluster_manager_,
cluster_manager_.thread_local_cluster_.cluster_.info_->stats_store_,
"test_prefix"));

auto counter = std::make_shared<NiceMock<Stats::MockCounter>>();
counter->name_ = "test_counter";
counter->latch_ = 1;
counter->used_ = true;
source_.counters_.push_back(counter);

expectCreateConnection();
EXPECT_CALL(*connection_, write(BufferStringEqual("test_prefix.test_counter:1|c\n"), _));
sink_->flush(source_);
}

TEST_F(TcpStatsdSinkTest, BufferReallocate) {
InSequence s;

Expand Down
20 changes: 20 additions & 0 deletions test/extensions/stats_sinks/common/statsd/udp_statsd_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,26 @@ TEST(UdpStatsdSinkTest, CheckActualStats) {
tls_.shutdownThread();
}

TEST(UdpStatsdSinkTest, CheckActualStatsWithCustomPrefix) {
NiceMock<Stats::MockSource> source;
auto writer_ptr = std::make_shared<NiceMock<MockWriter>>();
NiceMock<ThreadLocal::MockInstance> tls_;
UdpStatsdSink sink(tls_, writer_ptr, false, "test_prefix");

auto counter = std::make_shared<NiceMock<Stats::MockCounter>>();
counter->name_ = "test_counter";
counter->used_ = true;
counter->latch_ = 1;
source.counters_.push_back(counter);

EXPECT_CALL(*std::dynamic_pointer_cast<NiceMock<MockWriter>>(writer_ptr),
write("test_prefix.test_counter:1|c"));
sink.flush(source);
counter->used_ = false;

tls_.shutdownThread();
}

TEST(UdpStatsdSinkWithTagsTest, CheckActualStats) {
NiceMock<Stats::MockSource> source;
auto writer_ptr = std::make_shared<NiceMock<MockWriter>>();
Expand Down
35 changes: 33 additions & 2 deletions test/extensions/stats_sinks/dog_statsd/config_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ TEST_P(DogStatsdConfigLoopbackTest, ValidUdpIp) {
NiceMock<Server::MockInstance> server;
Stats::SinkPtr sink = factory->createStatsSink(*message, server);
EXPECT_NE(sink, nullptr);
EXPECT_NE(dynamic_cast<Common::Statsd::UdpStatsdSink*>(sink.get()), nullptr);
EXPECT_EQ(dynamic_cast<Common::Statsd::UdpStatsdSink*>(sink.get())->getUseTagForTest(), true);
auto udp_sink = dynamic_cast<Common::Statsd::UdpStatsdSink*>(sink.get());
EXPECT_NE(udp_sink, nullptr);
EXPECT_EQ(udp_sink->getUseTagForTest(), true);
EXPECT_EQ(udp_sink->getPrefix(), Common::Statsd::getDefaultPrefix());
}

// Negative test for protoc-gen-validate constraints for dog_statsd.
Expand All @@ -64,6 +66,35 @@ TEST(DogStatsdConfigTest, ValidateFail) {
ProtoValidationException);
}

TEST_P(DogStatsdConfigLoopbackTest, WithCustomPrefix) {
const std::string name = StatsSinkNames::get().DogStatsd;

envoy::config::metrics::v2::DogStatsdSink sink_config;
envoy::api::v2::core::Address& address = *sink_config.mutable_address();
envoy::api::v2::core::SocketAddress& socket_address = *address.mutable_socket_address();
socket_address.set_protocol(envoy::api::v2::core::SocketAddress::UDP);
auto loopback_flavor = Network::Test::getCanonicalLoopbackAddress(GetParam());
socket_address.set_address(loopback_flavor->ip()->addressAsString());
socket_address.set_port_value(8125);

const std::string customPrefix = "prefix.test";
sink_config.set_prefix(customPrefix);

Server::Configuration::StatsSinkFactory* factory =
Registry::FactoryRegistry<Server::Configuration::StatsSinkFactory>::getFactory(name);
ASSERT_NE(factory, nullptr);

ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto();
MessageUtil::jsonConvert(sink_config, *message);

NiceMock<Server::MockInstance> server;
Stats::SinkPtr sink = factory->createStatsSink(*message, server);
ASSERT_NE(sink, nullptr);
auto udp_sink = dynamic_cast<Common::Statsd::UdpStatsdSink*>(sink.get());
ASSERT_NE(udp_sink, nullptr);
EXPECT_EQ(udp_sink->getPrefix(), customPrefix);
}

} // namespace DogStatsd
} // namespace StatSinks
} // namespace Extensions
Expand Down