Use the zstdframe package where sensible instead of plumbing
around our own zstd.Encoder just for stateless operations.
This causes logtail to have a dependency on zstd,
but that's arguably okay since zstd support is implicit
to the protocol between a client and the logging service.
Also, virtually every caller to logger.NewLogger was
manually setting up a zstd.Encoder anyways,
meaning that zstd was functionally always a dependency.
Updates #cleanup
Updates tailscale/corp#18514
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The Go zstd package is not friendly for stateless zstd compression.
Passing around multiple zstd.Encoder just for stateless compression
is a waste of memory since the memory is never freed and seldom
used if no compression operations are happening.
For performance, we pool the relevant Encoder/Decoder
with the specific options set.
Functionally, this package is a wrapper over the Go zstd package
with a more ergonomic API for stateless operations.
This package can be used to cleanup various pre-existing zstd.Encoder
pools or one-off handlers spread throughout our codebases.
Performance:
BenchmarkEncode/Best 1690 610926 ns/op 25.78 MB/s 1 B/op 0 allocs/op
zstd_test.go:137: memory: 50.336 MiB
zstd_test.go:138: ratio: 3.269x
BenchmarkEncode/Better 10000 100939 ns/op 156.04 MB/s 0 B/op 0 allocs/op
zstd_test.go:137: memory: 20.399 MiB
zstd_test.go:138: ratio: 3.131x
BenchmarkEncode/Default 15775 74976 ns/op 210.08 MB/s 105 B/op 0 allocs/op
zstd_test.go:137: memory: 1.586 MiB
zstd_test.go:138: ratio: 3.064x
BenchmarkEncode/Fastest 23222 53977 ns/op 291.81 MB/s 26 B/op 0 allocs/op
zstd_test.go:137: memory: 599.458 KiB
zstd_test.go:138: ratio: 2.898x
BenchmarkEncode/FastestLowMemory 23361 50789 ns/op 310.13 MB/s 15 B/op 0 allocs/op
zstd_test.go:137: memory: 334.458 KiB
zstd_test.go:138: ratio: 2.898x
BenchmarkEncode/FastestNoChecksum 23086 50253 ns/op 313.44 MB/s 26 B/op 0 allocs/op
zstd_test.go:137: memory: 599.458 KiB
zstd_test.go:138: ratio: 2.900x
BenchmarkDecode/Checksum 70794 17082 ns/op 300.96 MB/s 4 B/op 0 allocs/op
zstd_test.go:163: memory: 316.438 KiB
BenchmarkDecode/NoChecksum 74935 15990 ns/op 321.51 MB/s 4 B/op 0 allocs/op
zstd_test.go:163: memory: 316.438 KiB
BenchmarkDecode/LowMemory 71043 16739 ns/op 307.13 MB/s 0 B/op 0 allocs/op
zstd_test.go:163: memory: 79.347 KiB
We can see that the options are taking effect where compression ratio improves
with higher levels and compression speed diminishes.
We can also see that LowMemory takes effect where the pooled coder object
references less memory than other cases.
We can see that the pooling is taking effect as there are 0 amortized allocations.
Additional performance:
BenchmarkEncodeParallel/zstd-24 1857 619264 ns/op 1796 B/op 49 allocs/op
BenchmarkEncodeParallel/zstdframe-24 1954 532023 ns/op 4293 B/op 49 allocs/op
BenchmarkDecodeParallel/zstd-24 5288 197281 ns/op 2516 B/op 49 allocs/op
BenchmarkDecodeParallel/zstdframe-24 6441 196254 ns/op 2513 B/op 49 allocs/op
In concurrent usage, handling the pooling in this package
has a marginal benefit over the zstd package,
which relies on a Go channel as the pooling mechanism.
In particular, coders can be freed by the GC when not in use.
Coders can be shared throughout the program if they use this package
instead of multiple independent pools doing the same thing.
The allocations are unrelated to pooling as they're caused by the spawning of goroutines.
Updates #cleanup
Updates tailscale/corp#18514
Updates tailscale/corp#17653
Updates tailscale/corp#18005
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Providing a hash.Block512 is an implementation detail of how deephash
works today, but providing an opaque type with mostly equivalent API
(i.e., HashUint8, HashBytes, etc. methods) is still sensible.
Thus, define a public Hasher type that exposes exactly the API
that an implementation of SelfHasher would want to call.
This gives us freedom to change the hashing algorithm of deephash
at some point in the future.
Also, this type is likely going to be called by types that are
going to memoize their own hash results, we additionally add
a HashSum method to simplify this use case.
Add documentation to SelfHasher on how a type might implement it.
Updates: corp#16409
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Also perform minor cleanups on the ctxkey package itself.
Provide guidance on when to use ctxkey.Key[T] over ctxkey.New.
Also, allow for interface kinds because the value wrapping trick
also happens to fix edge cases with interfaces in Go.
Updates #cleanup
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The lack of type-safety in context.WithValue leads to the common pattern
of defining of package-scoped type to ensure global uniqueness:
type fooKey struct{}
func withFoo(ctx context, v Foo) context.Context {
return context.WithValue(ctx, fooKey{}, v)
}
func fooValue(ctx context) Foo {
v, _ := ctx.Value(fooKey{}).(Foo)
return v
}
where usage becomes:
ctx = withFoo(ctx, foo)
foo := fooValue(ctx)
With many different context keys, this can be quite tedious.
Using generics, we can simplify this as:
var fooKey = ctxkey.New("mypkg.fooKey", Foo{})
where usage becomes:
ctx = fooKey.WithValue(ctx, foo)
foo := fooKey.Value(ctx)
See https://go.dev/issue/49189
Updates #cleanup
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Simply reading the taildrop directory can pop up security dialogs
on platforms like macOS. Avoid this by only performing garbage collection
of partial and deleted files after the first received taildrop file,
which would have prompted the security dialog window.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Go has no way to explicitly identify Go struct as effectively a tuple,
so staticcheck assumes any external use of unkeyed literals is wrong.
Updates #cleanup
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
It is possible that upon a cold-start, we enqueue a partial file
for deletion that is resumed shortly after startup.
If the file transfer happens to last longer than deleteDelay,
we will delete the partial file, which is unfortunate.
The client spent a long time uploading a file,
only for it to be accidentally deleted.
It's a very rare race, but also a frustrating one
if it happens to manifest.
Fix the code to only delete partial files that
do not have an active puts against it.
We also fix a minor bug in ResumeReader
where we read b[:blockSize] instead of b[:cs.Size].
The former is the fixed size of 64KiB,
while the latter is usually 64KiB,
but may be less for the last block.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The progress printer was buggy where it would not print correctly
and some of the truncation logic was faulty.
The progress printer now prints something like:
go1.21.3.linux-amd64.tar.gz 21.53MiB 13.83MiB/s 33.88% ETA 00:00:03
where it shows
* the number of bytes transferred so far
* the rate of bytes transferred
(using a 1-second half-life for an exponentially weighted average)
* the progress made as a percentage
* the estimated time
(as calculated from the rate of bytes transferred)
Other changes:
* It now correctly prints the progress for very small files
* It prints at a faster rate (4Hz instead of 1Hz)
* It uses IEC units for byte quantities
(to avoid ambiguities of "kb" being kilobits or kilobytes)
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Previously, the test simply relied on:
defer close()
to cleanup file handles.
This works fine on Unix-based systems,
but not on Windows, which dislikes deleting files
where an open file handle continues to exist.
Fix the test by explicitly closing the file handle
after we are done with the resource.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The LoadFunc loads a value and calls a user-provided function.
The utility of this method is to ensure that the map lock is held
while executing user-provided logic.
This allows us to solve TOCTOU bugs that would be nearly imposible
to the solve without this API.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
WaitGroup.Wait should not be concurrently called WaitGroup.Add.
In other words, we should not start new goroutines after shutodwn is called.
Thus, add a conditional to check that shutdown has not been called
before starting off a new waitAndDelete goroutine.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
While the previous logic was correct, it did not perform well.
Resuming is a dance between the client and server, where
1. the client requests hashes for a partial file,
2. the server then computes those hashes,
3. the client computes hashes locally and compares them.
4. goto 1 while the partial file still has data
While step 2 is running, the client is sitting idle.
While step 3 is running, the server is sitting idle.
By streaming over the block hash immediately after the server
computes it, the client can start checking the hash,
while the server works on the next hash (in a pipelined manner).
This performs dramatically better and also uses less memory
as we don't need to hold a list of hashes, but only need to
handle one hash at a time.
There are two detriments to this approach:
* The HTTP API relies on a JSON stream,
which is not a standard REST-like pattern.
However, since we implement both client and server,
this is fine.
* While the stream is on-going, we hold an open file handle
on the server side while the file is being hashed.
On really slow streams, this could hold a file open forever.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Rhea Ghosh <rhea@tailscale.com>
Minor fixes:
* The branch for listing or hashing partial files was inverted.
* The host for peerapi call needs to be real (rather than bogus).
* Handle remote peers that don't support resuming.
* Make resume failures non-fatal (since we can still continue).
This was tested locally, end-to-end system test is future work.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Rhea Ghosh <rhea@tailscale.com>
File resumption requires keeping partial files around for some time,
but we must still eventually delete them if never resumed.
Thus, we implement asynchronous file deletion, which could
spawn a background goroutine to delete the files.
We also use the same mechanism for deleting files on Windows,
where a file can't be deleted if there is still an open file handle.
We can enqueue those with the asynchronous file deleter as well.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This adds support for parsing Range and Content-Range headers
according to RFC 7230. The package could be extended in the future
to handle other headers.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Perform the same m==nil check in Manager.{PartialFiles,HashPartialFile}
as we do in the other methods.
Fix HashPartialFile is properly handle a length of -1.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
We add the following API:
* type FileChecksums
* type Checksum
* func Manager.PartialFiles
* func Manager.HashPartialFile
* func ResumeReader
The Manager methods provide the ability to query for partial files
and retrieve a list of checksums for a given partial file.
The ResumeReader function is a helper that wraps an io.Reader
to discard content that is identical locally and remotely.
The FileChecksums type represents the checksums of a file
and is safe to JSON marshal and send over the wire.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Rhea Ghosh <rhea@tailscale.com>
In almost every single use of Clock, there is a default behavior
we want to use when the interface is nil,
which is to use the the standard time package.
The Clock interface exists only for testing,
and so tests that care about mocking time
can adequately plumb the the Clock down the stack
and through various data structures.
However, the problem with Clock is that there are many
situations where we really don't care about mocking time
(e.g., measuring execution time for a log message),
where making sure that Clock is non-nil is not worth the burden.
In fact, in a recent refactoring, the biggest pain point was
dealing with nil-interface panics when calling tstime.Clock methods
where mocking time wasn't even needed for the relevant tests.
This required wasted time carefully reviewing the code to
make sure that tstime.Clock was always populated,
and even then we're not statically guaranteed to avoid a nil panic.
Ideally, what we want are default methods on Go interfaces,
but such a language construct does not exist.
However, we can emulate that behavior by declaring
a concrete type that embeds the interface.
If the underlying interface value is nil,
it provides some default behavior (i.e., use StdClock).
This provides us a nice balance of two goals:
* We can plumb tstime.DefaultClock in all relevant places
for use with mocking time in the tests that care.
* For all other logic that don't care about,
we never need to worry about whether tstime.DefaultClock
is nil or not. This is especially relevant in production code
where we don't want to panic.
Longer-term, we may want to perform a large-scale change
where we rename Clock to ClockInterface
and rename DefaultClock to just Clock.
Updates #cleanup
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Changes made:
* Move all HTTP related functionality from taildrop to ipnlocal.
* Add two arguments to taildrop.Manager.PutFile to specify
an opaque client ID and a resume offset (both unused for now).
* Cleanup the logic of taildrop.Manager.PutFile
to be easier to follow.
* Implement file conflict handling where duplicate files are renamed
(e.g., "IMG_1234.jpg" -> "IMG_1234 (2).jpg").
* Implement file de-duplication where "renaming" a partial file
simply deletes it if it already exists with the same contents.
* Detect conflicting active puts where a second concurrent put
results in an error.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Rhea Ghosh <rhea@tailscale.com>
Changes made:
* Unexport declarations specific to internal taildrop functionality.
* Document all exported functionality.
* Move TestRedactErr to the taildrop package.
* Rename and invert Handler.DirectFileDoFinalRename as AvoidFinalRename.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This is being moved to taildrop, so clean it up to stop depending
on so much unreleated functionality by removing a dependency
on peerAPIHandler.
Updates tailscale/corp#14772
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Rhea Ghosh <rhea@tailscale.com>
Update github.com/go-json-experiment/json to the latest version
and fix the build in light of some breaking API changes.
Updates #cleanup
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
9538e9f970 broke LocalBackend.WhoIs
where you can no longer lookup yourself in WhoIs.
This occurs because the LocalBackend.peers map only contains peers.
If we fail to lookup a peer, double-check whether it is ourself.
Fixes#9470
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Rhea Ghosh <rhea@tailscale.com>
This eventually allows encoding packages that may respect
the proposed encoding.TextAppender interface.
The performance gains from this is between 10-30%.
Updates tailscale/corp#14379
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This makes it more maintainable for other code to statically depend
on the exact value of this string. It also makes it easier to
identify what code might depend on this string by looking up
references to this constant.
Updates tailscale/corp#13777
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
If the connection provided to sftp.NewServer is closed,
Serve returns the io.EOF error verbatim from io.Reader.Read.
This is an odd error since this is an expected situation,
so we manually ignore io.EOF.
This is somewhat buggy since the sftp package itself
incorrectly reports io.EOF in cases where it should actually
be reporting io.ErrUnexpectedEOF.
See https://github.com/pkg/sftp/pull/554 which patches Serve to
return nil on clean closes and fixes buggy uses of io.ReadFull.
Fixes#8592
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Calling both mono.Now() and time.Now() is slow and
leads to unnecessary precision errors.
Instead, directly compute mono.Time relative to baseMono and baseWall.
This is the opposite calculation as mono.Time.WallTime.
Updates tailscale/corp#8427
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
There are two race conditions in output handling.
The first race condition is due to a misuse of exec.Cmd.StdoutPipe.
The documentation explicitly forbids concurrent use of StdoutPipe
with exec.Cmd.Wait (see golang/go#60908) because Wait will
close both sides of the pipe once the process ends without
any guarantees that all data has been read from the pipe.
To fix this, we allocate the os.Pipes ourselves and
manage cleanup ourselves when the process has ended.
The second race condition is because sshSession.run waits
upon exec.Cmd to finish and then immediately proceeds to call ss.Exit,
which will close all output streams going to the SSH client.
This may interrupt any asynchronous io.Copy still copying data.
To fix this, we close the write-side of the os.Pipes after
the process has finished (and before calling ss.Exit) and
synchronously wait for the io.Copy routines to finish.
Fixes#7601
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Maisem Ali <maisem@tailscale.com>
The retry logic was pathological in the following ways:
* If we restarted the logging service, any pending uploads
would be placed in a retry-loop where it depended on backoff.Backoff,
which was too aggresive. It would retry failures within milliseconds,
taking at least 10 retries to hit a delay of 1 second.
* In the event where a logstream was rate limited,
the aggressive retry logic would severely exacerbate the problem
since each retry would also log an error message.
It is by chance that the rate of log error spam
does not happen to exceed the rate limit itself.
We modify the retry logic in the following ways:
* We now respect the "Retry-After" header sent by the logging service.
* Lacking a "Retry-After" header, we retry after a hard-coded period of
30 to 60 seconds. This avoids the thundering-herd effect when all nodes
try reconnecting to the logging service at the same time after a restart.
* We do not treat a status 400 as having been uploaded.
This is simply not the behavior of the logging service.
Updates #tailscale/corp#11213
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Exclude traffic with 100.100.100.100 (for IPv4) and
with fd7a:115c:a1e0::53 (for IPv6) since this traffic with the
Tailscale service running locally on the node.
This traffic never left the node.
It also happens to be a high volume amount of traffic since
DNS requests occur over UDP with each request coming from a
unique port, thus resulting in many discrete traffic flows.
Fixestailscale/corp#10554
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The addition of WaitGroup.Go in the standard library has been
repeatedly proposed and rejected.
See golang/go#18022, golang/go#23538, and golang/go#39863
In summary, the argument for WaitGroup.Go is that it avoids bugs like:
go func() {
wg.Add(1)
defer wg.Done()
...
}()
where the increment happens after execution (not before)
and also (to a lesser degree) because:
wg.Go(func() {
...
})
is shorter and more readble.
The argument against WaitGroup.Go is that the provided function
takes no arguments and so inputs and outputs must closed over
by the provided function. The most common race bug for goroutines
is that the caller forgot to capture the loop iteration variable,
so this pattern may make it easier to be accidentally racy.
However, that is changing with golang/go#57969.
In my experience the probability of race bugs due to the former
still outwighs the latter, but I have no concrete evidence to prove it.
The existence of errgroup.Group.Go and frequent utility of the method
at least proves that this is a workable pattern and
the possibility of accidental races do not appear to
manifest as frequently as feared.
A reason *not* to use errgroup.Group everywhere is that there are many
situations where it doesn't make sense for the goroutine to return an error
since the error is handled in a different mechanism
(e.g., logged and ignored, formatted and printed to the frontend, etc.).
While you can use errgroup.Group by always returning nil,
the fact that you *can* return nil makes it easy to accidentally return
an error when nothing is checking the return of group.Wait.
This is not a hypothetical problem, but something that has bitten us
in usages that was only using errgroup.Group without intending to use
the error reporting part of it.
Thus, add a (yet another) variant of WaitGroup here that
is identical to sync.WaitGroup, but with an extra method.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Add Value, which measures the rate at which an event occurs,
exponentially weighted towards recent activity.
It is guaranteed to occupy O(1) memory, operate in O(1) runtime,
and is safe for concurrent use.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The time.Parse function has been optimized to the point
where it is faster than our custom implementation.
See upstream changes in:
* https://go.dev/cl/429862
* https://go.dev/cl/425197
* https://go.dev/cl/425116
Performance:
BenchmarkGoParse3339/Z 38.75 ns/op 0 B/op 0 allocs/op
BenchmarkGoParse3339/TZ 54.02 ns/op 0 B/op 0 allocs/op
BenchmarkParse3339/Z 40.17 ns/op 0 B/op 0 allocs/op
BenchmarkParse3339/TZ 87.06 ns/op 0 B/op 0 allocs/op
We can see that the stdlib implementation is now faster.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Share the same underlying implementation for both PrivateID and PublicID.
For the shared methods, declare them in the same order.
Only keep documentation on methods without obvious meaning.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Ever since the introduction of the "must" package,
most MustXXX functions are no longer necessary.
Remove this as it is no longer depended upon
from within this repository and by the internal private repository.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The log ID types were moved to a separate package so that
code that only depend on log ID types do not need to link
in the logic for the logtail client itself.
Not all code need the logtail client.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The Tailscale logging service has a hard limit on the maximum
log message size that can be accepted.
We want to ensure that netlog messages never exceed
this limit otherwise a client cannot transmit logs.
Move the goroutine for periodically dumping netlog messages
from wgengine/netlog to net/connstats.
This allows net/connstats to manage when it dumps messages,
either based on time or by size.
Updates tailscale/corp#8427
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Consider the following pattern:
err1 := foo()
err2 := bar()
err3 := baz()
return multierr.New(err1, err2, err3)
If err1, err2, and err3 are all nil, then multierr.New should not allocate.
Thus, modify the logic of New to count the number of distinct error values
and allocate the exactly needed slice. This also speeds up non-empty error
situation since repeatedly growing with append is slow.
Performance:
name old time/op new time/op delta
Empty-24 41.8ns ± 2% 6.4ns ± 1% -84.73% (p=0.000 n=10+10)
NonEmpty-24 120ns ± 3% 69ns ± 1% -42.01% (p=0.000 n=9+10)
name old alloc/op new alloc/op delta
Empty-24 64.0B ± 0% 0.0B -100.00% (p=0.000 n=10+10)
NonEmpty-24 168B ± 0% 88B ± 0% -47.62% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
Empty-24 1.00 ± 0% 0.00 -100.00% (p=0.000 n=10+10)
NonEmpty-24 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=10+10)
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Use multierr.Range to iterate through an error tree
instead of multiple invocations of errors.As.
This scales better as we add more Go error types to the switch.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Errors in Go are no longer viewed as a linear chain, but a tree.
See golang/go#53435.
Add a Range function that iterates through an error
in a pre-order, depth-first order.
This matches the iteration order of errors.As in Go 1.20.
This adds the logic (but currently commented out) for having
Error implement the multi-error version of Unwrap in Go 1.20.
It is commented out currently since it causes "go vet"
to complain about having the "wrong" signature.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Previously, tstun.Wrapper and magicsock.Conn managed their
own statistics data structure and relied on an external call to
Extract to extract (and reset) the statistics.
This makes it difficult to ensure a maximum size on the statistics
as the caller has no introspection into whether the number
of unique connections is getting too large.
Invert the control flow such that a *connstats.Statistics
is registered with tstun.Wrapper and magicsock.Conn.
Methods on non-nil *connstats.Statistics are called for every packet.
This allows the implementation of connstats.Statistics (in the future)
to better control when it needs to flush to ensure
bounds on maximum sizes.
The value registered into tstun.Wrapper and magicsock.Conn could
be an interface, but that has two performance detriments:
1. Method calls on interface values are more expensive since
they must go through a virtual method dispatch.
2. The implementation would need a sync.Mutex to protect the
statistics value instead of using an atomic.Pointer.
Given that methods on constats.Statistics are called for every packet,
we want reduce the CPU cost on this hot path.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Many packages reference the logtail ID types,
but unfortunately pull in the transitive dependencies of logtail.
Fix this problem by putting the log ID types in its own package
with minimal dependencies.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The cutset provided to strings.TrimRight was missing the digit '6',
making it such that we couldn't parse something like "365d".
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This function is no longer necessary as you can trivially rewrite:
logtail.MustParsePublicID(...)
with:
must.Get(logtail.ParsePublicID(...))
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Map is a concurrent safe map that is a trivial wrapper
over a Go map and a sync.RWMutex.
It is optimized for use-cases where the entries change often,
which is the opposite use-case of what sync.Map is optimized for.
The API is patterned off of sync.Map, but made generic.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
There is a finite limit to the maximum message size that logtail can upload.
We need to make sure network logging messages remain under this size.
These constants allow us to compute the maximum number of ConnectionCounts
we can buffer before we must flush.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Exit node traffic is aggregated to protect the privacy
of those using an exit node. However, it is reasonable to
at least log which nodes are making most use of an exit node.
For a node using an exit node,
the source will be the taiscale IP address of itself,
while the destination will be zeroed out.
For a node that serves as an exit node,
the source will be zeroed out,
while the destination will be tailscale IP address
of the node that initiated the exit traffic.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
In the future this will cause a node to be unable to join the tailnet
if network logging is enabled.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
If the network logging configruation changes (and nothing else)
we will tear down the network logger and start it back up.
However, doing so will lose the router configuration state.
Manually reconfigure it with the routing state.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Make netlogfmt useful regardless of the exact schema of the input.
If a JSON object looks like a network log message,
then unmarshal it as one and then print it.
This allows netlogfmt to support both a stream of JSON objects
directly serialized from netlogtype.Message, or the schema
returned by the /api/v2/tailnet/{{tailnet}}/network-logs API endpoint.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This is a temporary hack to prevent logtail getting stuck
uploading the same excessive message over and over.
A better solution will be discussed and implemented.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
There is utility in logging traffic statistics that occurs at the physical layer.
That is, in order to send packets virtually to a particular tailscale IP address,
what physical endpoints did we need to communicate with?
This functionality logs IP addresses identical to
what had always been logged in magicsock prior to #5823,
so there is no increase in PII being logged.
ExtractStatistics returns a mapping of connections to counts.
The source is always a Tailscale IP address (without port),
while the destination is some endpoint reachable on WAN or LAN.
As a special case, traffic routed through DERP will use 127.3.3.40
as the destination address with the port being the DERP region.
This entire feature is only enabled if data-plane audit logging
is enabled on the tailnet (by default it is disabled).
Example of type of information logged:
------------------------------------ Tx[P/s] Tx[B/s] Rx[P/s] Rx[B/s]
PhysicalTraffic: 25.80 3.39Ki 38.80 5.57Ki
100.1.2.3 -> 143.11.22.33:41641 15.40 2.00Ki 23.20 3.37Ki
100.4.5.6 -> 192.168.0.100:41641 10.20 1.38Ki 15.60 2.20Ki
100.7.8.9 -> 127.3.3.40:2 0.20 6.40 0.00 0.00
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The wireguard-go code unfortunately calls this unconditionally
even when verbose logging is disabled.
Partial revert of #5911.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This package parses a JSON stream of netlog.Message from os.Stdin
and pretty prints the contents as a stream of tables.
It supports reverse lookup of tailscale IP addresses if given
an API key and the tailnet that these traffic logs belong to.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This field seems seldom used and the documentation is wrong.
It is simpler to just derive its original value dynamically
when endpoint.DstToString is called.
This method is potentially used by wireguard-go,
but not in any code path is performance sensitive.
All calls to it use it in conjunction with fmt.Printf,
which is going to be slow anyways since it uses Go reflection.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This sets up Logger to handle statistics at the magicsock layer,
where we can correlate traffic between a particular tailscale IP address
and any number of physical endpoints used to contact the node
that hosts that tailscale address.
We also export Message and TupleCounts to better document the JSON format
that is being sent to the logging infrastructure.
This commit does NOT yet enable the actual logging of magicsock statistics.
That will be a future commit.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
If the wgcfg.Config is specified with network logging arguments,
then Userspace.Reconfig starts up an asynchronous network logger,
which is shutdown either upon Userspace.Close or when Userspace.Reconfig
is called again without network logging or route arguments.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The Logger type managers a logtail.Logger for extracting
statistics from a tstun.Wrapper.
So long as Shutdown is called, it ensures that logtail
and statistic gathering resources are properly cleared up.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The node and domain audit log IDs are provided in the map response,
but are ultimately going to be used in wgengine since
that's the layer that manages the tstun.Wrapper.
Do the plumbing work to get this field passed down the stack.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Rename StatisticsEnable as SetStatisticsEnabled to be consistent
with other similarly named methods.
Rename StatisticsExtract as ExtractStatistics to follow
the convention where methods start with a verb.
It was originally named with Statistics as a prefix so that
statistics related methods would sort well in godoc,
but that property no longer holds.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Upstream optimizations to the Go time package will make
unmarshaling of time.Time 3-6x faster. See:
* https://go.dev/cl/425116
* https://go.dev/cl/425197
* https://go.dev/cl/429862
The last optimization avoids a []byte -> string allocation
if the timestamp string less than than 32B.
Unfortunately, the presence of a timezone breaks that optimization.
Drop recording of timezone as this is non-essential information.
Most of the performance gains is upon unmarshal,
but there is also a slight performance benefit to
not marshaling the timezone as well.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The copy ID operates similar to a CC in email where
a message is sent to both the primary ID and also the copy ID.
A given log message is uploaded once, but the log server
records it twice for each ID.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
If Wrapper.StatisticsEnable is enabled,
then per-connection counters are maintained.
If enabled, Wrapper.StatisticsExtract must be periodically called
otherwise there is unbounded memory growth.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
High-level API:
type Statistics struct { ... }
type Counts struct { TxPackets, TxBytes, RxPackets, RxBytes uint64 }
func (*Statistics) UpdateTx([]byte)
func (*Statistics) UpdateRx([]byte)
func (*Statistics) Extract() map[flowtrack.Tuple]Counts
The API accepts a []byte instead of a packet.Parsed so that a future
implementation can directly hash the address and port bytes,
which are contiguous in most IP packets.
This will be useful for a custom concurrent-safe hashmap implementation.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Callers of LogHost often jump through hoops to undo the
loss of information dropped by LogHost (e.g., the HTTP scheme).
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The version.CmdName implementation is buggy such that it does not correctly
identify the binary name if it embeds other go binaries.
For now, add a NewWithConfigPath API that allows the caller to explicitly
specify this information.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
It is unclear whether the lack of checking nil-ness of slices
was an oversight or a deliberate feature.
Lacking a comment, the assumption is that this was an oversight.
Also, expand the logic to perform cycle detection for recursive slices.
We do this on a per-element basis since a slice is semantically
equivalent to a list of pointers.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Add a new lookupTypeHasher function that is just a cached front-end
around the makeTypeHasher function.
We do not need to worry about the recursive type cycle issue that
made getTypeInfo more complicated since makeTypeHasher
is not directly recursive. All calls to itself happen lazily
through a sync.Once upon first use.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
The entry logic of Hash has extra complexity to make sure
we always have an addressable value on hand.
If not, we heap allocate the input.
For this reason we document that there are performance benefits
to always providing a pointer.
Rather than documenting this, just enforce it through generics.
Also, delete the unused HasherForType function.
It's an interesting use of generics, but not well tested.
We can resurrect it from code history if there's a need for it.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This helps pprof better identify which Go kinds take the most time
since the kind is always in the function name.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This helps pprof better identify which Go kinds take the most time
since the kind is always in the function name.
There is a minor adjustment where we hash the length of the map
to be more on the cautious side.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Rather than having two copies []fieldInfo,
just maintain one and perform merging in the same pass.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This helps pprof better identify which Go kinds take the most time
since the kind is always in the function name.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Use of reflect.Value.SetXXX panics if the provided argument was
obtained from an unexported struct field.
Instead, pass an unsafe.Pointer around and convert to a
reflect.Value when necessary (i.e., for maps and interfaces).
Converting from unsafe.Pointer to reflect.Value guarantees that
none of the read-only bits will be populated.
When running in race mode, we attach type information to the pointer
so that we can type check every pointer operation.
This also type-checks that direct memory hashing is within
the valid range of a struct value.
We add test cases that previously caused deephash to panic,
but now pass.
Performance:
name old time/op new time/op delta
Hash 14.1µs ± 1% 14.1µs ± 1% ~ (p=0.590 n=10+9)
HashPacketFilter 2.53µs ± 2% 2.44µs ± 1% -3.79% (p=0.000 n=9+10)
TailcfgNode 1.45µs ± 1% 1.43µs ± 0% -1.36% (p=0.000 n=9+9)
HashArray 318ns ± 2% 318ns ± 2% ~ (p=0.541 n=10+10)
HashMapAcyclic 32.9µs ± 1% 31.6µs ± 1% -4.16% (p=0.000 n=10+9)
There is a slight performance gain due to the use of unsafe.Pointer
over reflect.Value methods. Also, passing an unsafe.Pointer (1 word)
on the stack is cheaper than passing a reflect.Value (3 words).
Performance gains are diminishing since SHA-256 hashing now dominates the runtime.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
When built with "deephash_debug", print the set of HashXXX methods.
Example usage:
$ go test -run=GetTypeHasher/string_slice -tags=deephash_debug
U64(2)+U64(3)+S("foo")+U64(3)+S("bar")+FIN
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Rather than separate functions to hash each kind,
just rely on the fact that these are direct memory hashable,
thus simplifying the code.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Every implementation of typeHasherFunc always returns true,
which implies that the slow path is no longer executed.
Delete it.
h.hashValueWithType(v, ti, ...) is deleted as it is equivalent to:
ti.hasher()(h, v)
h.hashValue(v, ...) is deleted as it is equivalent to:
ti := getTypeInfo(v.Type())
ti.hasher()(h, v)
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Add support for maps and interfaces to the fast path.
Add cycle-detection to the pointer handling logic.
This logic is mostly copied from the slow path.
A future commit will delete the slow path once
the fast path never falls back to the slow path.
Performance:
name old time/op new time/op delta
Hash-24 18.5µs ± 1% 14.9µs ± 2% -19.52% (p=0.000 n=10+10)
HashPacketFilter-24 2.54µs ± 1% 2.60µs ± 1% +2.19% (p=0.000 n=10+10)
HashMapAcyclic-24 31.6µs ± 1% 30.5µs ± 1% -3.42% (p=0.000 n=9+8)
TailcfgNode-24 1.44µs ± 2% 1.43µs ± 1% ~ (p=0.171 n=10+10)
HashArray-24 324ns ± 1% 324ns ± 2% ~ (p=0.425 n=9+9)
The additional cycle detection logic doesn't incur much slow down
since it only activates if a type is recursive, which does not apply
for any of the types that we care about.
There is a notable performance boost since we switch from the fath path
to the slow path less often. Most notably, a struct with a field that
could not be handled by the fast path would previously cause
the entire struct to go through the slow path.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
There are 5 types that we care about that implement AppendTo:
key.DiscoPublic
key.NodePublic
netip.Prefix
netipx.IPRange
netip.Addr
The key types are thin wrappers around [32]byte and are memory hashable.
The netip.Prefix and netipx.IPRange types are thin wrappers over netip.Addr
and are hashable by default if netip.Addr is hashable.
The netip.Addr type is the only one with a complex structure where
the default behavior of deephash does not hash it correctly due to the presence
of the intern.Value type.
Drop support for AppendTo and instead add specialized hashing for netip.Addr
that would be semantically equivalent to == on the netip.Addr values.
The AppendTo support was already broken prior to this change.
It was fully removed (intentionally or not) in #4870.
It was partially restored in #4858 for the fast path,
but still broken in the slow path.
Just drop support for it altogether.
This does mean we lack any ability for types to self-hash themselves.
In the future we can add support for types that implement:
interface { DeepHash() Sum }
Test and fuzz cases were added for the relevant types that
used to rely on the AppendTo method.
FuzzAddr has been executed on 1 billion samples without issues.
Signed-off-by: Joe Tsai joetsai@digital-static.net
Rename Hash as Block512 to indicate that this is a general-purpose
hash.Hash for any algorithm that operates on 512-bit block sizes.
While we rename the package as hashx in this commit,
a subsequent commit will move the sha256x package to hashx.
This is done separately to avoid confusing git.
Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Joe TsaiandGitHub
