-
Notifications
You must be signed in to change notification settings - Fork 180
RUST-1587 Implement server selection tracing events #805
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
9ef4a1f
d4a4e95
51a965b
62f4303
ce53f0f
88dba9b
50c4182
175fb2d
d18f1b9
a58179d
687989f
6a2c9f5
a672066
9ccb9c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,8 @@ use crate::{ | |
|
||
/// A description of the most up-to-date information known about a topology. Further details can | ||
/// be found in the [Server Discovery and Monitoring specification](https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst). | ||
#[derive(Clone)] | ||
#[derive(Clone, derive_more::Display)] | ||
#[display(fmt = "{}", description)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this just means that the output will be the Display impl for also, it was an intentional choice to leave |
||
pub struct TopologyDescription { | ||
pub(crate) description: crate::sdam::TopologyDescription, | ||
} | ||
|
@@ -102,38 +103,3 @@ impl fmt::Debug for TopologyDescription { | |
.finish() | ||
} | ||
} | ||
|
||
impl fmt::Display for TopologyDescription { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this implementation was combined with the existing and more bare-bones implementation of |
||
fn fmt(&self, f: &mut fmt::Formatter) -> std::result::Result<(), fmt::Error> { | ||
write!(f, "{{ Type: {:?}", self.description.topology_type)?; | ||
|
||
if let Some(ref set_name) = self.description.set_name { | ||
write!(f, ", Set Name: {}", set_name)?; | ||
} | ||
|
||
if let Some(max_set_version) = self.description.max_set_version { | ||
write!(f, ", Max Set Version: {}", max_set_version)?; | ||
} | ||
|
||
if let Some(max_election_id) = self.description.max_election_id { | ||
write!(f, ", Max Election ID: {}", max_election_id)?; | ||
} | ||
|
||
if let Some(ref compatibility_error) = self.description.compatibility_error { | ||
write!(f, ", Compatibility Error: {}", compatibility_error)?; | ||
} | ||
|
||
if !self.description.servers.is_empty() { | ||
write!(f, ", Servers: ")?; | ||
let mut iter = self.description.servers.values(); | ||
if let Some(server) = iter.next() { | ||
write!(f, "{}", ServerInfo::new_borrowed(server))?; | ||
} | ||
for server in iter { | ||
write!(f, ", {}", ServerInfo::new_borrowed(server))?; | ||
} | ||
} | ||
|
||
write!(f, " }}") | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ use crate::{ | |
use self::server_selection::IDLE_WRITE_PERIOD; | ||
|
||
/// The possible types for a topology. | ||
#[derive(Debug, Clone, Copy, Eq, PartialEq, Deserialize, Serialize)] | ||
#[derive(Debug, Clone, Copy, Eq, PartialEq, Deserialize, Serialize, derive_more::Display)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seemed worth adding Display here for use in |
||
#[non_exhaustive] | ||
pub enum TopologyType { | ||
/// A single mongod server. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,11 @@ impl SelectedServer { | |
server.increment_operation_count(); | ||
Self { server } | ||
} | ||
|
||
#[cfg(feature = "tracing-unstable")] | ||
pub(crate) fn address(&self) -> &ServerAddress { | ||
&self.server.address | ||
} | ||
} | ||
|
||
impl Deref for SelectedServer { | ||
|
@@ -362,12 +367,38 @@ impl TopologyDescription { | |
} | ||
|
||
impl fmt::Display for TopologyDescription { | ||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
write!(f, "{{ Type: {:?}, Servers: [ ", self.topology_type)?; | ||
for server_info in self.servers.values().map(ServerInfo::new_borrowed) { | ||
write!(f, "{}, ", server_info)?; | ||
fn fmt(&self, f: &mut fmt::Formatter) -> std::result::Result<(), fmt::Error> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this logic is largely copied over from the |
||
write!(f, "{{ Type: {}", self.topology_type)?; | ||
|
||
if let Some(ref set_name) = self.set_name { | ||
write!(f, ", Set Name: {}", set_name)?; | ||
} | ||
|
||
if let Some(max_set_version) = self.max_set_version { | ||
write!(f, ", Max Set Version: {}", max_set_version)?; | ||
} | ||
|
||
if let Some(max_election_id) = self.max_election_id { | ||
write!(f, ", Max Election ID: {}", max_election_id)?; | ||
} | ||
write!(f, "] }}") | ||
|
||
if let Some(ref compatibility_error) = self.compatibility_error { | ||
write!(f, ", Compatibility Error: {}", compatibility_error)?; | ||
} | ||
|
||
if !self.servers.is_empty() { | ||
write!(f, ", Servers: [ ")?; | ||
let mut iter = self.servers.values(); | ||
if let Some(server) = iter.next() { | ||
write!(f, "{}", ServerInfo::new_borrowed(server))?; | ||
} | ||
for server in iter { | ||
write!(f, ", {}", ServerInfo::new_borrowed(server))?; | ||
} | ||
write!(f, " ]")?; | ||
} | ||
|
||
write!(f, " }}") | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,18 +13,20 @@ use crate::{ | |
}; | ||
|
||
/// Describes which servers are suitable for a given operation. | ||
#[derive(Clone, Derivative)] | ||
#[derive(Clone, Derivative, derive_more::Display)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see unit tests in src/test/spec/trace.rs to see how the |
||
#[derivative(Debug)] | ||
#[non_exhaustive] | ||
pub enum SelectionCriteria { | ||
/// A read preference that describes the suitable servers based on the server type, max | ||
/// staleness, and server tags. | ||
/// | ||
/// See the documentation [here](https://www.mongodb.com/docs/manual/core/read-preference/) for more details. | ||
#[display(fmt = "ReadPreference {}", _0)] | ||
ReadPreference(ReadPreference), | ||
|
||
/// A predicate used to filter servers that are considered suitable. A `server` will be | ||
/// considered suitable by a `predicate` if `predicate(server)` returns true. | ||
#[display(fmt = "Custom predicate")] | ||
Predicate(#[derivative(Debug = "ignore")] Predicate), | ||
} | ||
|
||
|
@@ -129,6 +131,48 @@ pub enum ReadPreference { | |
Nearest { options: ReadPreferenceOptions }, | ||
} | ||
|
||
impl std::fmt::Display for ReadPreference { | ||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
write!(f, "{{ Mode: ")?; | ||
let opts_ref = match self { | ||
ReadPreference::Primary => { | ||
write!(f, "Primary")?; | ||
None | ||
} | ||
ReadPreference::Secondary { options } => { | ||
write!(f, "Secondary")?; | ||
Some(options) | ||
} | ||
ReadPreference::PrimaryPreferred { options } => { | ||
write!(f, "PrimaryPreferred")?; | ||
Some(options) | ||
} | ||
ReadPreference::SecondaryPreferred { options } => { | ||
write!(f, "SecondaryPreferred")?; | ||
Some(options) | ||
} | ||
ReadPreference::Nearest { options } => { | ||
write!(f, "Nearest")?; | ||
Some(options) | ||
} | ||
}; | ||
if let Some(opts) = opts_ref { | ||
if !opts.is_default() { | ||
if let Some(ref tag_sets) = opts.tag_sets { | ||
write!(f, ", Tag Sets: {:?}", tag_sets)?; | ||
} | ||
if let Some(ref max_staleness) = opts.max_staleness { | ||
write!(f, ", Max Staleness: {:?}", max_staleness)?; | ||
} | ||
if let Some(ref hedge) = opts.hedge { | ||
write!(f, ", Hedge: {}", hedge.enabled)?; | ||
} | ||
} | ||
} | ||
write!(f, " }}") | ||
} | ||
} | ||
|
||
impl<'de> Deserialize<'de> for ReadPreference { | ||
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error> | ||
where | ||
|
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.
It's really unfortunate how much noise this adds to the function - can logging be factored out to helper methods?
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.
great idea, done. I ended up adding a helper type to store the shared info and avoid having to pass it with each message. this matches nicely with our existing
XEventEmitter
types for CMAP and command tracing.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.
Very nice!