track bytes sent through egress server #279
Conversation
| var rc *redis.Client | ||
|
|
||
| const reportIngressKey = "ingress" | ||
| const reportEgressKey = "egress" | ||
|
|
||
| // Write the number of bytes to redis, if a client was given when starting the server, otherwise do nothing | ||
| func recordBytes(transferType, userId string, nBytes int64) { | ||
| common.Debugf("WebSocket connection: user: '%v' transferred(%v) %v bytes", userId, transferType, nBytes) | ||
| if rc == nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
I don't love global vars, so experimented with an alternative approach. Let me know what you think of that.
@Crosse - do you have any opinion on how we can implement a redis reporter in the Unbounded egress lib in the most friendly/generic way that let's it be optional?
There was a problem hiding this comment.
What I don't love about my approach is that I'm passing the redis.Client pointer to the listener struct, then also passing it down to the websocket connection struct.
Is that an anti-pattern? Would it be better to stuff that kind of dependency injection into a context that's passed to everybody?
| return | ||
| } | ||
|
|
||
| userID := r.Header.Get("lantern-user-id") //TODO: which header to use for userID? |
There was a problem hiding this comment.
Can this be declared as a const (as you did with reporting.go)?
There was a problem hiding this comment.
Pull Request Overview
This PR introduces byte tracking through the egress server by recording ingress and egress data to Redis. Key changes include:
- Adding a new function (recordBytes) to log byte transfers.
- Modifying the websocketPacketConn to include a user identifier and asynchronously report bytes transferred.
- Updating the listener initialization to accept a Redis client, with corresponding changes in the command-line entry.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| egress/reporting.go | Implements byte tracking via Redis using recordBytes. |
| egress/egresslib.go | Propagates the user identifier and triggers byte reporting. |
| egress/cmd/egress.go | Initializes a Redis client and passes it to the NewListener. |
Files not reviewed (1)
- go.mod: Language not supported
Comments suppressed due to low confidence (2)
egress/reporting.go:16
- Consider renaming the parameter 'userId' to 'userID' for consistency with other parts of the code where 'userID' is used.
func recordBytes(transferType, userId string, nBytes int64) {
egress/cmd/egress.go:51
- [nitpick] Consider renaming 'testRedis' to a more descriptive name such as 'reportingRedis' to better reflect its usage in production code.
testRedis := redis.NewClient(&redis.Options{
| return | ||
| } | ||
| ctx := context.Background() | ||
| err := rc.HIncrBy(ctx, userId, transferType, nBytes).Err() |
There was a problem hiding this comment.
I would batch these vs creating a new goroutine for each write. You could also just wrap the net.Conn, similar to what I just did here:
There was a problem hiding this comment.
This talks about it, for example. A goroutine for every read and write will quickly become super inefficient:
https://bytes.swiggy.com/optimizing-batch-writes-to-redis-using-pipelining-d480ebaf4653
|
closed because using the existing tracking in |
This is one way of tracking the bytes that each user moves. An another approach was described here getlantern/http-proxy#646, but I ended up doing it all in the egress server since I was having problems with getting the reporting functions in http-proxy to work for this. This method would require that some kind of identifier is sent in the request to identify each user. Another piece that could be added here is a field in Redis for number of connections made.