-
Notifications
You must be signed in to change notification settings - Fork 6
Zipkin config for setting hostname #6
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,23 +61,31 @@ class ZipkinDriverTest : public testing::Test { | |
| random_, time_source_); | ||
| } | ||
|
|
||
| void setupValidDriver(const std::string& version) { | ||
| void setupValidDriverWithHostname(const std::string& version, const std::string& hostname) { | ||
| EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillRepeatedly(Return(&cm_.thread_local_cluster_)); | ||
|
|
||
| const std::string yaml_string = fmt::format(R"EOF( | ||
| std::string yaml_string = fmt::format(R"EOF( | ||
| collector_cluster: fake_cluster | ||
| collector_endpoint: /api/v1/spans | ||
| collector_endpoint_version: {} | ||
| )EOF", | ||
| version); | ||
| if (!hostname.empty()) { | ||
| yaml_string = yaml_string + fmt::format(R"EOF( | ||
| collector_hostname: {} | ||
| )EOF", | ||
| hostname); | ||
| } | ||
|
|
||
| envoy::config::trace::v3::ZipkinConfig zipkin_config; | ||
| TestUtility::loadFromYaml(yaml_string, zipkin_config); | ||
|
|
||
| setup(zipkin_config, true); | ||
| } | ||
|
|
||
| void expectValidFlushSeveralSpans(const std::string& version, const std::string& content_type) { | ||
| setupValidDriver(version); | ||
| void expectValidFlushSeveralSpansWithHostname(const std::string& version, const std::string& content_type, | ||
| const std::string& hostname) { | ||
| setupValidDriverWithHostname(version, hostname); | ||
|
|
||
| Http::MockAsyncClientRequest request(&cm_.async_client_); | ||
| Http::AsyncClient::Callbacks* callback; | ||
|
|
@@ -90,8 +98,9 @@ class ZipkinDriverTest : public testing::Test { | |
| const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { | ||
| callback = &callbacks; | ||
|
|
||
| const std::string& expected_hostname = !hostname.empty() ? hostname : "fake_cluster"; | ||
|
||
| EXPECT_EQ("/api/v1/spans", message->headers().getPathValue()); | ||
| EXPECT_EQ("fake_cluster", message->headers().getHostValue()); | ||
| EXPECT_EQ(expected_hostname, message->headers().getHostValue()); | ||
| EXPECT_EQ(content_type, message->headers().getContentTypeValue()); | ||
|
|
||
| return &request; | ||
|
|
@@ -127,6 +136,14 @@ class ZipkinDriverTest : public testing::Test { | |
| EXPECT_EQ(1U, stats_.counter("tracing.zipkin.reports_failed").value()); | ||
| } | ||
|
|
||
| void setupValidDriver(const std::string& version) { | ||
| setupValidDriverWithHostname(version, ""); | ||
| } | ||
|
|
||
| void expectValidFlushSeveralSpans(const std::string& version, const std::string& content_type) { | ||
| expectValidFlushSeveralSpansWithHostname(version, content_type, ""); | ||
| } | ||
|
|
||
| // TODO(#4160): Currently time_system_ is initialized from DangerousDeprecatedTestTime, which uses | ||
| // real time, not mock-time. When that is switched to use mock-time instead, I think | ||
| // generateRandom64() may not be as random as we want, and we'll need to inject entropy | ||
|
|
@@ -217,6 +234,11 @@ TEST_F(ZipkinDriverTest, FlushSeveralSpansHttpJson) { | |
| expectValidFlushSeveralSpans("HTTP_JSON", "application/json"); | ||
| } | ||
|
|
||
| TEST_F(ZipkinDriverTest, FlushSeveralSpansHttpJsonWithHostname) { | ||
| expectValidFlushSeveralSpansWithHostname("HTTP_JSON", "application/json", | ||
| "zipkin.fakedomain.io"); | ||
| } | ||
|
|
||
| TEST_F(ZipkinDriverTest, FlushSeveralSpansHttpProto) { | ||
| expectValidFlushSeveralSpans("HTTP_PROTO", "application/x-protobuf"); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh. Is it worth reordering the ternary values here to ditch the leading
!?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping it as it is, since either order is valid in its own way