Skip to content

Commit 1ef180a

Browse files
committed
fix: log upstream duration excluding request delay
Move duration logging from proxy to client, measuring only the actual upstream request time. This prevents the configured request delay from being included in the logged duration.
1 parent ef2cb24 commit 1ef180a

File tree

3 files changed

+20
-32
lines changed

3 files changed

+20
-32
lines changed

SPEC.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,8 @@ func (c *Cache) Set(key string, data []byte, ttl time.Duration) {
193193
level=INFO msg="starting proxy" listen=:5502 upstream=192.168.1.100:502
194194
level=DEBUG msg="cache hit" slave_id=1 func=0x03 addr=0 qty=10
195195
level=DEBUG msg="cache miss" slave_id=1 func=0x03 addr=0 qty=10
196-
level=DEBUG msg="upstream read" slave_id=1 func=0x03 addr=0 qty=10 duration=15ms
197-
level=DEBUG msg="applying request delay" duration=100ms
196+
level=DEBUG msg="upstream request completed" slave_id=1 func=0x03 addr=0 qty=10 duration=15ms
197+
level=DEBUG msg="applying request delay" delay=100ms
198198
level=WARN msg="upstream error, serving stale" slave_id=1 error="timeout"
199199
level=INFO msg="shutting down"
200200
```

internal/modbus/client.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ func (c *Client) Execute(ctx context.Context, req *Request) ([]byte, error) {
8989
// Set slave ID
9090
c.conn.SlaveID = req.SlaveID
9191

92-
// Execute request
92+
// Execute request and measure time
93+
start := time.Now()
9394
resp, err := c.executeRequest(ctx, req)
9495
if err != nil {
9596
// Try reconnect once
@@ -98,15 +99,25 @@ func (c *Client) Execute(ctx context.Context, req *Request) ([]byte, error) {
9899
return nil, fmt.Errorf("reconnect failed: %w", reconnErr)
99100
}
100101
c.conn.SlaveID = req.SlaveID
102+
start = time.Now() // Reset timer for retry
101103
resp, err = c.executeRequest(ctx, req)
102104
if err != nil {
103105
return nil, err
104106
}
105107
}
108+
duration := time.Since(start)
109+
110+
c.logger.Debug("upstream request completed",
111+
"slave_id", req.SlaveID,
112+
"func", fmt.Sprintf("0x%02X", req.FunctionCode),
113+
"addr", req.Address,
114+
"qty", req.Quantity,
115+
"duration", duration,
116+
)
106117

107118
// Apply request delay if configured (only after successful requests)
108119
if c.requestDelay > 0 {
109-
c.logger.Debug("applying request delay", "duration", c.requestDelay)
120+
c.logger.Debug("applying request delay", "delay", c.requestDelay)
110121
select {
111122
case <-time.After(c.requestDelay):
112123
case <-ctx.Done():

internal/proxy/proxy.go

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,12 @@ func (p *Proxy) Shutdown(timeout time.Duration) error {
8787

8888
// HandleRequest implements modbus.Handler interface.
8989
func (p *Proxy) HandleRequest(ctx context.Context, req *modbus.Request) ([]byte, error) {
90-
start := time.Now()
91-
9290
if modbus.IsWriteFunction(req.FunctionCode) {
93-
return p.handleWrite(ctx, req, start)
91+
return p.handleWrite(ctx, req)
9492
}
9593

9694
if modbus.IsReadFunction(req.FunctionCode) {
97-
return p.handleRead(ctx, req, start)
95+
return p.handleRead(ctx, req)
9896
}
9997

10098
// Unknown function code
@@ -105,7 +103,7 @@ func (p *Proxy) HandleRequest(ctx context.Context, req *modbus.Request) ([]byte,
105103
return modbus.BuildExceptionResponse(req.FunctionCode, modbus.ExcIllegalFunction), nil
106104
}
107105

108-
func (p *Proxy) handleRead(ctx context.Context, req *modbus.Request, start time.Time) ([]byte, error) {
106+
func (p *Proxy) handleRead(ctx context.Context, req *modbus.Request) ([]byte, error) {
109107
key := cache.Key(req.SlaveID, req.FunctionCode, req.Address, req.Quantity)
110108

111109
// Use GetOrFetch for request coalescing
@@ -117,20 +115,7 @@ func (p *Proxy) handleRead(ctx context.Context, req *modbus.Request, start time.
117115
"qty", req.Quantity,
118116
)
119117

120-
resp, err := p.client.Execute(ctx, req)
121-
if err != nil {
122-
return nil, err
123-
}
124-
125-
p.logger.Debug("upstream read",
126-
"slave_id", req.SlaveID,
127-
"func", fmt.Sprintf("0x%02X", req.FunctionCode),
128-
"addr", req.Address,
129-
"qty", req.Quantity,
130-
"duration", time.Since(start),
131-
)
132-
133-
return resp, nil
118+
return p.client.Execute(ctx, req)
134119
})
135120

136121
if err != nil {
@@ -159,7 +144,7 @@ func (p *Proxy) handleRead(ctx context.Context, req *modbus.Request, start time.
159144
return data, nil
160145
}
161146

162-
func (p *Proxy) handleWrite(ctx context.Context, req *modbus.Request, start time.Time) ([]byte, error) {
147+
func (p *Proxy) handleWrite(ctx context.Context, req *modbus.Request) ([]byte, error) {
163148
switch p.cfg.ReadOnly {
164149
case config.ReadOnlyOn:
165150
// Silently ignore, return success response
@@ -186,14 +171,6 @@ func (p *Proxy) handleWrite(ctx context.Context, req *modbus.Request, start time
186171
return nil, err
187172
}
188173

189-
p.logger.Debug("upstream write",
190-
"slave_id", req.SlaveID,
191-
"func", fmt.Sprintf("0x%02X", req.FunctionCode),
192-
"addr", req.Address,
193-
"qty", req.Quantity,
194-
"duration", time.Since(start),
195-
)
196-
197174
// Invalidate exact matching cache entries for all read function codes
198175
p.invalidateCache(req)
199176

0 commit comments

Comments
 (0)