fix forwarded metrics #2273
fix forwarded metrics #2273
Conversation
|
| //forward the message to the target peer | ||
| if err := dstPeer.Stream.Send(msg); err != nil { | ||
| log.Errorf("error while forwarding message from peer [%s] to peer [%s] %v", msg.Key, msg.RemoteKey, err) | ||
| //todo respond to the sender? | ||
|
|
||
| s.metrics.MessageForwardFailures.Add(ctx, 1, metric.WithAttributes(attribute.String(labelType, labelTypeError))) | ||
| } else { | ||
| s.metrics.MessageForwardLatency.Record(ctx, float64(time.Since(start).Nanoseconds())/1e6, metric.WithAttributes(attribute.String(labelType, labelTypeMessage))) |
There was a problem hiding this comment.
Instead of /1e6 we could use time.Since(start).Milliseconds(), I think.
There was a problem hiding this comment.
If you do that we'll lose precision, as Milliseconds returns an int, but we have fractional milliseconds in the histogram buckets:
https://github.com/netbirdio/netbird/blob/main/signal%2Fmetrics%2Fapp.go#L90-L104
But maybe we don't need those small buckets in the first place. We should take a look at the actual data.
There was a problem hiding this comment.
let's keep them for now
| //forward the message to the target peer | ||
| if err := dstPeer.Stream.Send(msg); err != nil { | ||
| log.Errorf("error while forwarding message from peer [%s] to peer [%s] %v", msg.Key, msg.RemoteKey, err) | ||
| //todo respond to the sender? | ||
|
|
||
| s.metrics.MessageForwardFailures.Add(ctx, 1, metric.WithAttributes(attribute.String(labelType, labelTypeError))) | ||
| } else { | ||
| s.metrics.MessageForwardLatency.Record(ctx, float64(time.Since(start).Nanoseconds())/1e6, metric.WithAttributes(attribute.String(labelType, labelTypeMessage))) |
There was a problem hiding this comment.
If you do that we'll lose precision, as Milliseconds returns an int, but we have fractional milliseconds in the histogram buckets:
https://github.com/netbirdio/netbird/blob/main/signal%2Fmetrics%2Fapp.go#L90-L104
But maybe we don't need those small buckets in the first place. We should take a look at the actual data.
Describe your changes
Issue ticket number and link
Checklist