Skip to content
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

Adds remote-address connection info to server #148

Merged
merged 3 commits into from
Oct 15, 2021

Conversation

bnaecker
Copy link
Contributor

  • Reworks how a hyper::Server is created, to get access to the local
    address before starting the server itself. This allows it to be put
    into the key-value pairs for the server's logger, so they're printed
    on each subsequent message.
  • Updates the internal connection/request handler types to get the
    remote address and store it in the logger passed to the request
    handling functions. The remote address is also printed on each
    subsequent log message.

- Reworks how a `hyper::Server` is created, to get access to the local
  address before starting the server itself. This allows it to be put
  into the key-value pairs for the server's logger, so they're printed
  on each subsequent message.
- Updates the internal connection/request handler types to get the
  remote address and store it in the logger passed to the request
  handling functions. The remote address is also printed on each
  subsequent log message.
@bnaecker
Copy link
Contributor Author

This should address #46 when merged.

Copy link
Collaborator

@davepacheco davepacheco left a comment

Choose a reason for hiding this comment

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

Looks great! Would you mind posting some log output from before and after the change, maybe for server startup and handling a request? I think there's an existing simple example that you should be able to use.

This might be worth adding to the change log, too.

dropshot/src/server.rs Show resolved Hide resolved
@bnaecker
Copy link
Contributor Author

bnaecker commented Oct 15, 2021

Here's how this change manifests in server logs. I ran cargo run --example basic, and then curl localhost:$PORT/counter from another shell, for reference.

From the current main:

bnaecker@shale : ~/dropshot $ cargo run --example basic
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `target/debug/examples/basic`
Oct 15 07:48:02.273 INFO listening, local_addr: 127.0.0.1:43415
Oct 15 07:48:12.226 INFO accepted connection, remote_addr: 127.0.0.1:38077
Oct 15 07:48:12.227 INFO request completed, response_code: 200, uri: /counter, method: GET, req_id: ae866998-39da-4945-802f-c3c90856ddef
^C

From this PR commit:

bnaecker@shale : ~/dropshot $ cargo run --example basic
   Compiling dropshot v0.5.2-dev (/home/bnaecker/dropshot/dropshot)
    Finished dev [unoptimized + debuginfo] target(s) in 4.81s
     Running `target/debug/examples/basic`
Oct 15 07:49:19.399 INFO listening, local_addr: 127.0.0.1:39903
Oct 15 07:49:31.245 INFO accepted connection, remote_addr: 127.0.0.1:46529, local_addr: 127.0.0.1:39903
Oct 15 07:49:31.246 INFO request completed, response_code: 200, uri: /counter, method: GET, req_id: 814ba79a-51b9-48e8-b104-a7dd6455b389, remote_addr: 127.0.0.1:46529, local_addr: 127.0.0.1:39903
^C

The remote_addr and local_addr keys are in every log message, and are passed in the RequestContext object forward to actual endpoint handlers, meaning the child loggers receive them as well.

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