chore: add DRPC server implementation for network telemetry #13675
Conversation
620c148 to
fb9d5aa
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @deansheather and the rest of your teammates on |
|
Graphite messed up all the commits in my branch and now they're co-authored by Ethan... the actual commits were only written by me. When I do the merge I'll remove the co-authored by line. |
937bf83 to
479df1f
Compare
tailnet/service.go
Outdated
| func (b *NetworkTelemetryBatcher) Close() error { | ||
| close(b.closed) | ||
| <-b.done | ||
| return nil | ||
| } |
There was a problem hiding this comment.
This should probably accept a context so it can't block forever, or move the waiting to a separate WaitUntilEmpty() or similar
There was a problem hiding this comment.
I'm not going to make it accept a context, but I've added a 5s timeout and an error.
| if b.ticker != nil { | ||
| b.ticker.Reset(b.frequency) | ||
| } | ||
| go b.batchFn(events) |
There was a problem hiding this comment.
This feels a bit racey. We should probably have a separate channel to signal to the other goroutine that it should tick.
There was a problem hiding this comment.
This isn't racey because we hold a lock. If a tick happens while we're handling messages, the pending events list will be emptied and the periodic batcher goroutine will do nothing because there are no events. If somehow we went above the batch size anyways and overflow, it'll just send two telemetry events in short succession which is acceptable. I've added a comment that the ticker reset is best effort
tailnetproto.DRPCTailnetServerPostTelemetrymethodtelemetrypackage and converter functionsTODO: