Here's an example log line in the new format:
[RATE LIMITED] format string "open-conn-track: timeout opening %v; no associated peer node" (example: "open-conn-track: timeout opening ([ip] => [ip]); no associated peer node")
This should make debugging logging issues a bit easier, and give more
context as to why something was rate limited. This change was proposed
in a comment on #1110.
Signed-off-by: Smitty <me@smitop.com>
This reverts commit e4f53e9b6f.
At least two of these tests are flakey, reverting until they can be
made more robust.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
This is what every other DNS resolver I could find does, so tsdns
should do it to. This also helps avoid weird error messages about
non-existent records being unimplemented, and thus fixes#848.
Signed-off-by: Smitty <me@smitop.com>
* logtail: test parseAndRemoveLogLevel()
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
* logtail: test JSON log encoding.
Expand TestUploadMessages to also exercise the encoding functions
in logtail, like JSON logging and timestamps.
Other tests frequently send logs but a) don't check the result and
b) do so by happenstance, such that the lines in encode() were not
consistently being exercised and leading to spurious changes in
code coverage.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
* logtail: add a test for drainPendingMessages
Make the client buffer some messages before the upload server
becomes available.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
* logtail: use %q, raw strings, and io.WriteString
%q escapes binary characters for us.
raw strings avoid so much backslash escaping
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
Right now TestFastShutdown tries to upload logs to localhost:1234,
which will most likely respond with an error. However if one has an
actual service running on port 1234, it would receive a connection
attempting to POST every time the unit test runs.
Start a local server and direct the upload there instead.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
In sendDiscoMessage there is a check of whether the connection is
closed, which is not being reliably exercised by other tests.
This shows up in code coverage reports, the lines of code in
sendDiscoMessage are alternately added and subtracted from
code coverage.
Add a test to specifically exercise and verify this code path.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
Start an HTTP server to accept POST requests, and upload some logs to
it. Check that uploaded logs were received.
Code in logtail:drainPending was not being reliably exercised by other
tests. This shows up in code coverage reports, as lines of code in
drainPending are alternately added and subtracted from code coverage.
This test will reliably exercise and verify this code.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
All cases in lessThan are not reliably exercised by other tests.
This shows up in code coverage metrics as lines in lessThan are
alternately added and removed from coverage.
Add a test case to systematically test all conditions.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
In derpWriteChanOfAddr when we call derphttp.NewRegionClient(),
there is a check of whether the connection is already errored and
if so it returns before grabbing the lock. The lock might already
be held and would be a deadlock.
This corner case is not being reliably exercised by other tests.
This shows up in code coverage reports, the lines of code in
derpWriteChanOfAddr are alternately added and subtracted from
code coverage.
Add a test to specifically exercise this code path, and verify that
it doesn't deadlock.
This is the best tradeoff I could come up with:
+ the moment code calls Err() to check if there is an error, we
grab the lock to make sure it would deadlock if it tries to grab
the lock itself.
+ if a new call to Err() is added in this code path, only the
first one will be covered and the rest will not be tested.
+ this test doesn't verify whether code is checking for Err() in
the right place, which ideally I guess it would.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
Users in Amsterdam (as one example) were flipping back and forth
between equidistant London & Frankfurt relays too much.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
netaddr.IP no longer allocates, so don't need a cache or all its associated
code/complexity.
This totally removes groupcache/lru from the deps.
Also go mod tidy.
* wengine/netstack: bump gvisor to latest version
Signed-off-by: Naman Sood <naman@tailscale.com>
* update dependencies
Signed-off-by: Naman Sood <naman@tailscale.com>
* Don't change hardcoded IP
Signed-off-by: Naman Sood <naman@tailscale.com>
Not usefully functional yet (mostly a proof of concept), but getting
it submitted for some work @namansood is going to do atop this.
Updates #707
Updates #634
Updates #48
Updates #835
* show DNS name over hostname, removing domain's common MagicDNS suffix.
only show hostname if there's no DNS name.
but still show shared devices' MagicDNS FQDN.
* remove nerdy low-level details by default: endpoints, DERP relay,
public key. They're available in JSON mode still for those who need
them.
* only show endpoint or DERP relay when it's active with the goal of
making debugging easier. (so it's easier for users to understand
what's happening) The asterisks are gone.
* remove Tx/Rx numbers by default for idle peers; only show them when
there's traffic.
* include peers' owner login names
* add CLI option to not show peers (matching --self=true, --peers= also
defaults to true)
* sort by DNS/host name, not public key
* reorder columns
If any goroutine continues to use the logger in TestLocalLogLines
after the test finishes, the test panics.
The culprit for this was wireguard-go; the previous commit fixed that.
This commit adds suspenders: When the test is done, make logging calls
into no-ops.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
The log lines that wireguard-go prints as it starts
and stops its worker routines are mostly noise.
They also happen after other work is completed,
which causes failures in some of the log testing packages.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
This appears to have been the intent of the previous code,
but in practice, it only returned A records.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
To be honest I'm not fond of Golden Bytes tests like this, but
not so much as to want to rewrite the whole test. The DNS byte
format is essentially immutable at this point, the encoded bytes
aren't going to change. The rest of the test assumptions about
hostnames might, but we can fix that when it comes.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
Previously, any change to endpoints or hostinfo (or hostinfo's
netinfo) would result in the long-running map request HTTP stream
being torn down and restarted, losing all compression context along
with it.
This change makes us instead send a lite map request (OmitPeers: true,
Stream: false) that doesn't subscribe to anything, and then the
coordination server knows to not close other streams for that node
when it recives a lite request.
Fixestailscale/corp#797
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Josh Bleecher Snyder
Smitty
Denton Gentry
Brad Fitzpatrick
Christina Wen
Sonia Appasamy
Naman Sood