Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
- Fix EXPECTORATE test output
- Use both HTTP and native addresses from oximeter collector config
  • Loading branch information
bnaecker committed Oct 17, 2024
1 parent 516ee34 commit 5e96ce0
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 30 deletions.
7 changes: 5 additions & 2 deletions dev-tools/omdb/tests/usage_errors.out
Original file line number Diff line number Diff line change
Expand Up @@ -783,13 +783,16 @@ Usage: omdb oxql [OPTIONS]
Options:
--log-level <LOG_LEVEL> log level filter [env: LOG_LEVEL=] [default: warn]
--summaries Print summaries of each SQL query run against the database
--elapsed Print the total elapsed query duration
--color <COLOR> Color output [default: auto] [possible values: auto, always, never]
--elapsed Print the total elapsed query duration
-h, --help Print help

Connection Options:
--clickhouse-url <CLICKHOUSE_URL>
URL of the ClickHouse server to connect to [env: OMDB_CLICKHOUSE_URL=]
--clickhouse-native-url <CLICKHOUSE_NATIVE_URL>
URL of the ClickHouse server to connect to for the native protcol [env:
OMDB_CLICKHOUSE_NATIVE_URL=]
--dns-server <DNS_SERVER>
[env: OMDB_DNS_SERVER=]

Expand All @@ -808,7 +811,7 @@ error: unexpected argument '--summarizes' found

tip: a similar argument exists: '--summaries'

Usage: omdb oxql <--clickhouse-url <CLICKHOUSE_URL>|--summaries|--elapsed>
Usage: omdb oxql <--clickhouse-url <CLICKHOUSE_URL>|--clickhouse-native-url <CLICKHOUSE_NATIVE_URL>|--summaries|--elapsed>

For more information, try '--help'.
=============================================
55 changes: 27 additions & 28 deletions oximeter/collector/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use dropshot::HttpServer;
use dropshot::HttpServerStarter;
use internal_dns_types::names::ServiceName;
use omicron_common::address::get_internal_dns_server_addresses;
use omicron_common::address::CLICKHOUSE_TCP_PORT;
use omicron_common::address::DNS_PORT;
use omicron_common::api::internal::nexus::ProducerEndpoint;
use omicron_common::backoff;
Expand Down Expand Up @@ -240,42 +239,42 @@ impl Oximeter {
.map(|ip| SocketAddr::new(ip, DNS_PORT))
.collect();

// Closure to create a single resolver.
let make_resolver =
|maybe_address, srv_name: ServiceName| -> BoxedResolver {
if let Some(address) = maybe_address {
Box::new(SingleHostResolver::new(address))
} else {
Box::new(DnsResolver::new(
service::Name(srv_name.srv_name()),
bootstrap_dns.clone(),
DnsResolverConfig {
hardcoded_ttl: Some(tokio::time::Duration::MAX),
..Default::default()
},
))
}
};

// Closure to create _two_ resolvers, one to resolve the ClickHouse HTTP
// SRV record, and one for the native TCP record.
//
// TODO(cleanup): This should be removed if / when we entirely switch to
// the native protocol.
let make_clickhouse_resolvers = || -> (BoxedResolver, BoxedResolver) {
if let Some(address) = config.db.address {
let http = Box::new(SingleHostResolver::new(address));
let native_addr =
SocketAddr::new(address.ip(), CLICKHOUSE_TCP_PORT);
let native = Box::new(SingleHostResolver::new(native_addr));
(http, native)
} else {
let http_service = if config.db.replicated {
let http_resolver = make_resolver(
config.db.address,
if config.db.replicated {
ServiceName::ClickhouseServer
} else {
ServiceName::Clickhouse
};
let http = Box::new(DnsResolver::new(
service::Name(http_service.srv_name()),
bootstrap_dns.clone(),
DnsResolverConfig {
hardcoded_ttl: Some(tokio::time::Duration::MAX),
..Default::default()
},
));
let native = Box::new(DnsResolver::new(
service::Name(ServiceName::ClickhouseNative.srv_name()),
bootstrap_dns.clone(),
DnsResolverConfig {
hardcoded_ttl: Some(tokio::time::Duration::MAX),
..Default::default()
},
));
(http, native)
}
},
);
let native_resolver = make_resolver(
config.db.native_address,
ServiceName::ClickhouseNative,
);
(http_resolver, native_resolver)
};

let make_agent = || async {
Expand Down

0 comments on commit 5e96ce0

Please sign in to comment.