Skip to content

Levon/make bgp dispatch listen configurable#654

Draft
internet-diglett wants to merge 4 commits intomainfrom
levon/make-bgp-dispatch-listen-configurable
Draft

Levon/make bgp dispatch listen configurable#654
internet-diglett wants to merge 4 commits intomainfrom
levon/make-bgp-dispatch-listen-configurable

Conversation

@internet-diglett
Copy link
Contributor

No description provided.

We'd like the ability to create a topology of BGP routers in
our test and dev tooling. To do so with the current omicron-dev
and testing contexts, we need the peering to occur over localhost,
which means we need to be able to configure unique localhost
socketaddrs for each instance.
The omicron dev and test contexts provide :0 as the listen
port and rely on the log messages to output the actual port number
that the daemon bound to.
Copy link
Contributor

@taspelund taspelund left a comment

Choose a reason for hiding this comment

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

Looks good overall. If you wouldn't mind using the preexisting logging macros that wrap slog, that would be great. Otherwise, LGTM!

};
use mg_common::lock;
use slog::Logger;
use slog::{Logger, info};
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using slog::info!, please use crate::log::connection_log or connection_log_lite.
They have a bunch of the slog KV values pre-populated that add additional context to the logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears that the custom macros do not support debug formatting (the ? prefix):

error: no rules expected `?`
   --> bgp/src/connection_tcp.rs:120:73
    |
120 |         connection_log!(log, info, "TcpListener created"; "listener" => ?listener);
    |                                                                         ^ no rules expected this token in macro call
    |
   ::: bgp/src/log.rs:251:1
    |
251 | macro_rules! connection_log {
    | --------------------------- when calling this macro
    |
note: while trying to match meta-variable `$value:expr`
   --> bgp/src/log.rs:252:58
    |
252 |     ($self:expr, $level:ident, $msg:expr; $($key:expr => $value:expr),*) => {
    |                                                          ^^^^^^^^^^^

    Checking ddmd v0.1.0 (/disk1/workspace/maghemite/ddmd)
error: could not compile `bgp` (lib) due to 1 previous error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per our conversation in chat, I've updated this logic to use the slog::o! method of updating and carrying forward the log context:

pfexec ./target/debug/mgd run --bgp-dispatcher-addr "[::]:0" | looker
18:35:48.595Z INFO slog-rs (mgd): signal handler waiting for context
    module = admin
    unit = signal
18:35:48.663Z INFO slog-rs (mgd): interface monitor started
    module = unnumbered manager
    unit = daemon
18:35:48.663Z INFO slog-rs (bgp): dispatcher started
    module = neighbor
    unit = dispatcher
18:35:48.663Z INFO slog-rs (bgp): starting listener with bind arg: [::]:0
    module = neighbor
    unit = dispatcher
18:35:48.663Z INFO slog-rs (bgp): TcpListener created
    listener = TcpListener { addr: [::]:48382, fd: 10 }
    module = neighbor
    unit = dispatcher
18:35:48.663Z INFO slog-rs (bgp): transitioning to accept loop
    bind_addr = [::]:48382
    module = neighbor
    unit = dispatcher
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants