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

ref(iroh-net): Do not add the NodeId in the magicsock span field #2679

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

flub
Copy link
Contributor

@flub flub commented Aug 28, 2024

Description

Since we have made MagicSock private it should always be created via the Endpoint, unless we are internally messing around with tests or so. Because of this we can make sure that the Endpoint always attaches the correct "me" field to the span. Reducing the amount of noise in the default log output.

Breaking Changes

None

Notes & open questions

Please do not merge before #2595 is merged. I'm trying to not add any further changes to that PR.

Change checklist

  • Self-review.
  • [ ] Documentation updates following the style guide, if relevant.
  • [ ] Tests if relevant.
  • [ ] All breaking changes documented.

Copy link

github-actions bot commented Aug 28, 2024

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2679/docs/iroh/

Last updated: 2024-08-29T10:40:51Z

@flub flub requested review from matheus23 and divagant-martian and removed request for matheus23 August 29, 2024 08:32
Copy link
Contributor

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

Small comparison for people wondering:
main:

$ RUST_LOG=iroh_net=trace cargo test -p iroh-net --lib -- endpoint_connect_close --nocapture | grep re_stun
   Compiling iroh-net v0.23.0 (/home/philipp/program/work/iroh/iroh-net)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 26.56s
     Running unittests src/lib.rs (target/debug/deps/iroh_net-6c4d4b2dbab2ace8)
2024-08-29T09:59:51.769798Z DEBUG test-client:connect{me=4bgr2mpvame67knd remote=rzd5j76x5cgsvttn alpn="n0/iroh/test"}:try_send:re_stun{me=4bgr2mpvame67knd}: iroh_net::magicsock: 1249: re_stun: refresh-for-peering
2024-08-29T09:59:51.771685Z TRACE test-server:ep{me=rzd5j76x5cgsvttn}:magicsock{me=rzd5j76x5cgsvttn}:actor: iroh_net::magicsock: 1843: tick: re_stun Instant { tv_sec: 253792, tv_nsec: 894112852 }
2024-08-29T09:59:51.771733Z DEBUG test-server:ep{me=rzd5j76x5cgsvttn}:magicsock{me=rzd5j76x5cgsvttn}:actor:re_stun{me=rzd5j76x5cgsvttn}: iroh_net::magicsock: 1249: re_stun: periodic
2024-08-29T09:59:51.771943Z TRACE test-client:ep{me=4bgr2mpvame67knd}:magicsock{me=4bgr2mpvame67knd}:actor: iroh_net::magicsock: 1843: tick: re_stun Instant { tv_sec: 253792, tv_nsec: 896262050 }
2024-08-29T09:59:51.771974Z DEBUG test-client:ep{me=4bgr2mpvame67knd}:magicsock{me=4bgr2mpvame67knd}:actor:re_stun{me=4bgr2mpvame67knd}: iroh_net::magicsock: 1249: re_stun: periodic

This branch:

$ RUST_LOG=iroh_net=trace cargo test -p iroh-net --lib -- endpoint_connect_close --nocapture | grep re_stun
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.25s
     Running unittests src/lib.rs (target/debug/deps/iroh_net-6c4d4b2dbab2ace8)
2024-08-29T09:59:13.451701Z DEBUG test-client:connect{me=k5iwgxt73rr6hewa remote=65jc2o5fib5sqxv7 alpn="n0/iroh/test"}:try_send:re_stun: iroh_net::magicsock: 1249: re_stun: refresh-for-peering
2024-08-29T09:59:13.453721Z TRACE test-server:ep{me=65jc2o5fib5sqxv7}:magicsock:actor: iroh_net::magicsock: 1843: tick: re_stun Instant { tv_sec: 253754, tv_nsec: 575697553 }
2024-08-29T09:59:13.453760Z DEBUG test-server:ep{me=65jc2o5fib5sqxv7}:magicsock:actor:re_stun: iroh_net::magicsock: 1249: re_stun: periodic
2024-08-29T09:59:13.454010Z TRACE test-client:ep{me=k5iwgxt73rr6hewa}:magicsock:actor: iroh_net::magicsock: 1843: tick: re_stun Instant { tv_sec: 253754, tv_nsec: 578197257 }
2024-08-29T09:59:13.454051Z DEBUG test-client:ep{me=k5iwgxt73rr6hewa}:magicsock:actor:re_stun: iroh_net::magicsock: 1249: re_stun: periodic

So much better.

Since we have made MagicSock private it should always be created via
the Endpoint, unless we are internally messing around with tests or
so.  Because of this we can make sure that the Endpoint always
attaches the correct "me" field to the span.  Reducing the amount of
noise in the default log output.
@flub flub force-pushed the flub/magicsock-span-no-me branch from 8a09751 to eb9c4d2 Compare August 29, 2024 10:40
@flub flub changed the base branch from dig/quinn11 to main August 29, 2024 10:41
@flub flub enabled auto-merge August 29, 2024 10:41
@flub flub added this pull request to the merge queue Aug 29, 2024
Merged via the queue into main with commit 05fff6a Aug 29, 2024
27 of 28 checks passed
@flub flub deleted the flub/magicsock-span-no-me branch August 29, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants