Skip to content

Commit

Permalink
client: Fix duration in operation statistics
Browse files Browse the repository at this point in the history
Previously, an incorrect duration was passed to the statistics handler
of all operations. Deferred function sent the time elapsed from the
start point to the almost instant calculation of the closure arguments
(nanoseconds).

Now client correctly measures the execution time. Also, as a slight
improvement, these actions are deferred conditionally now.

Signed-off-by: Leonard Lyubich <[email protected]>
  • Loading branch information
cthulhu-rider committed Nov 22, 2024
1 parent 774c74c commit a093108
Show file tree
Hide file tree
Showing 11 changed files with 167 additions and 85 deletions.
10 changes: 7 additions & 3 deletions client/accounting.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client

import (
"context"
"time"

v2accounting "github.com/nspcc-dev/neofs-api-go/v2/accounting"
protoaccounting "github.com/nspcc-dev/neofs-api-go/v2/accounting/grpc"
Expand Down Expand Up @@ -35,9 +36,12 @@ func (x *PrmBalanceGet) SetAccount(id user.ID) {
// - [ErrMissingAccount]
func (c *Client) BalanceGet(ctx context.Context, prm PrmBalanceGet) (accounting.Decimal, error) {
var err error
defer func() {
c.sendStatistic(stat.MethodBalanceGet, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodBalanceGet, time.Since(startTime), err)
}()
}

switch {
case prm.account.IsZero():
Expand Down
11 changes: 2 additions & 9 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,8 @@ func (c *Client) Close() error {
return c.Conn().Close()

Check warning on line 219 in client/client.go

View check run for this annotation

Codecov / codecov/patch

client/client.go#L219

Added line #L219 was not covered by tests
}

func (c *Client) sendStatistic(m stat.Method, err error) func() {
if c.prm.statisticCallback == nil {
return func() {}
}

ts := time.Now()
return func() {
c.prm.statisticCallback(c.nodeKey, c.endpoint, m, time.Since(ts), err)
}
func (c *Client) sendStatistic(m stat.Method, dur time.Duration, err error) {
c.prm.statisticCallback(c.nodeKey, c.endpoint, m, dur, err)
}

// PrmInit groups initialization parameters of Client instances.
Expand Down
64 changes: 43 additions & 21 deletions client/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"time"

v2container "github.com/nspcc-dev/neofs-api-go/v2/container"
protocontainer "github.com/nspcc-dev/neofs-api-go/v2/container/grpc"
Expand Down Expand Up @@ -71,9 +72,12 @@ func (x *PrmContainerPut) AttachSignature(sig neofscrypto.Signature) {
// - [ErrMissingSigner]
func (c *Client) ContainerPut(ctx context.Context, cont container.Container, signer neofscrypto.Signer, prm PrmContainerPut) (cid.ID, error) {
var err error
defer func() {
c.sendStatistic(stat.MethodContainerPut, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodContainerPut, time.Since(startTime), err)
}()
}

if signer == nil {
return cid.ID{}, ErrMissingSigner
Expand Down Expand Up @@ -174,9 +178,12 @@ type PrmContainerGet struct {
// Context is required and must not be nil. It is used for network communication.
func (c *Client) ContainerGet(ctx context.Context, id cid.ID, prm PrmContainerGet) (container.Container, error) {
var err error
defer func() {
c.sendStatistic(stat.MethodContainerGet, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodContainerGet, time.Since(startTime), err)
}()
}

var cidV2 refs.ContainerID
id.WriteToV2(&cidV2)
Expand Down Expand Up @@ -248,9 +255,12 @@ type PrmContainerList struct {
// Context is required and must not be nil. It is used for network communication.
func (c *Client) ContainerList(ctx context.Context, ownerID user.ID, prm PrmContainerList) ([]cid.ID, error) {
var err error
defer func() {
c.sendStatistic(stat.MethodContainerList, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodContainerList, time.Since(startTime), err)
}()
}

// form request body
var ownerV2 refs.OwnerID
Expand Down Expand Up @@ -359,9 +369,12 @@ func (x *PrmContainerDelete) AttachSignature(sig neofscrypto.Signature) {
// - [ErrMissingSigner]
func (c *Client) ContainerDelete(ctx context.Context, id cid.ID, signer neofscrypto.Signer, prm PrmContainerDelete) error {
var err error
defer func() {
c.sendStatistic(stat.MethodContainerDelete, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodContainerDelete, time.Since(startTime), err)
}()
}

if signer == nil {
return ErrMissingSigner
Expand Down Expand Up @@ -450,9 +463,12 @@ type PrmContainerEACL struct {
// Context is required and must not be nil. It is used for network communication.
func (c *Client) ContainerEACL(ctx context.Context, id cid.ID, prm PrmContainerEACL) (eacl.Table, error) {
var err error
defer func() {
c.sendStatistic(stat.MethodContainerEACL, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodContainerEACL, time.Since(startTime), err)
}()
}

var cidV2 refs.ContainerID
id.WriteToV2(&cidV2)
Expand Down Expand Up @@ -565,9 +581,12 @@ func (x *PrmContainerSetEACL) AttachSignature(sig neofscrypto.Signature) {
// Context is required and must not be nil. It is used for network communication.
func (c *Client) ContainerSetEACL(ctx context.Context, table eacl.Table, signer user.Signer, prm PrmContainerSetEACL) error {
var err error
defer func() {
c.sendStatistic(stat.MethodContainerSetEACL, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodContainerSetEACL, time.Since(startTime), err)
}()
}

if signer == nil {
return ErrMissingSigner
Expand Down Expand Up @@ -665,9 +684,12 @@ type PrmAnnounceSpace struct {
// - [ErrMissingAnnouncements]
func (c *Client) ContainerAnnounceUsedSpace(ctx context.Context, announcements []container.SizeEstimation, prm PrmAnnounceSpace) error {
var err error
defer func() {
c.sendStatistic(stat.MethodContainerAnnounceUsedSpace, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodContainerAnnounceUsedSpace, time.Since(startTime), err)
}()
}

if len(announcements) == 0 {
err = ErrMissingAnnouncements
Expand Down
28 changes: 19 additions & 9 deletions client/netmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package client
import (
"context"
"fmt"
"time"

v2netmap "github.com/nspcc-dev/neofs-api-go/v2/netmap"
protonetmap "github.com/nspcc-dev/neofs-api-go/v2/netmap/grpc"
Expand Down Expand Up @@ -60,9 +61,12 @@ func (x ResEndpointInfo) NodeInfo() netmap.NodeInfo {
// Reflects all internal errors in second return value (transport problems, response processing, etc.).
func (c *Client) EndpointInfo(ctx context.Context, prm PrmEndpointInfo) (*ResEndpointInfo, error) {
var err error
defer func() {
c.sendStatistic(stat.MethodEndpointInfo, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodEndpointInfo, time.Since(startTime), err)
}()
}

// form request
var req v2netmap.LocalNodeInfoRequest
Expand Down Expand Up @@ -146,9 +150,12 @@ type PrmNetworkInfo struct {
// Reflects all internal errors in second return value (transport problems, response processing, etc.).
func (c *Client) NetworkInfo(ctx context.Context, prm PrmNetworkInfo) (netmap.NetworkInfo, error) {
var err error
defer func() {
c.sendStatistic(stat.MethodNetworkInfo, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodNetworkInfo, time.Since(startTime), err)
}()
}

// form request
var req v2netmap.NetworkInfoRequest
Expand Down Expand Up @@ -215,9 +222,12 @@ type PrmNetMapSnapshot struct {
// Reflects all internal errors in second return value (transport problems, response processing, etc.).
func (c *Client) NetMapSnapshot(ctx context.Context, _ PrmNetMapSnapshot) (netmap.NetMap, error) {
var err error
defer func() {
c.sendStatistic(stat.MethodNetMapSnapshot, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodNetMapSnapshot, time.Since(startTime), err)
}()
}

// form request body
var body v2netmap.SnapshotRequestBody
Expand Down
10 changes: 7 additions & 3 deletions client/object_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"time"

"github.com/nspcc-dev/neofs-api-go/v2/acl"
v2object "github.com/nspcc-dev/neofs-api-go/v2/object"
Expand Down Expand Up @@ -74,9 +75,12 @@ func (c *Client) ObjectDelete(ctx context.Context, containerID cid.ID, objectID
err error
)

defer func() {
c.sendStatistic(stat.MethodObjectDelete, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodObjectDelete, time.Since(startTime), err)
}()
}

containerID.WriteToV2(&cidV2)
addr.SetContainerID(&cidV2)
Expand Down
48 changes: 33 additions & 15 deletions client/object_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"time"

"github.com/nspcc-dev/neofs-api-go/v2/acl"
v2object "github.com/nspcc-dev/neofs-api-go/v2/object"
Expand Down Expand Up @@ -86,6 +87,7 @@ type PayloadReader struct {
remainingPayloadLen int

statisticCallback shortStatisticCallback
startTime time.Time // if statisticCallback is set only
}

// readHeader reads header of the object. Result means success.
Expand Down Expand Up @@ -209,7 +211,7 @@ func (x *PayloadReader) Close() error {
var err error
if x.statisticCallback != nil {
defer func() {
x.statisticCallback(err)
x.statisticCallback(time.Since(x.startTime), err)

Check warning on line 214 in client/object_get.go

View check run for this annotation

Codecov / codecov/patch

client/object_get.go#L214

Added line #L214 was not covered by tests
}()
}
err = x.close(true)
Expand Down Expand Up @@ -266,9 +268,12 @@ func (c *Client) ObjectGetInit(ctx context.Context, containerID cid.ID, objectID
err error
)

defer func() {
c.sendStatistic(stat.MethodObjectGet, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodObjectGet, time.Since(startTime), err)
}()
}

if signer == nil {
return hdr, nil, ErrMissingSigner
Expand Down Expand Up @@ -309,8 +314,11 @@ func (c *Client) ObjectGetInit(ctx context.Context, containerID cid.ID, objectID
r.cancelCtxStream = cancel
r.stream = stream
r.client = c
r.statisticCallback = func(err error) {
c.sendStatistic(stat.MethodObjectGetStream, err)
if c.prm.statisticCallback != nil {
r.startTime = time.Now()
r.statisticCallback = func(dur time.Duration, err error) {
c.sendStatistic(stat.MethodObjectGetStream, dur, err)
}

Check warning on line 321 in client/object_get.go

View check run for this annotation

Codecov / codecov/patch

client/object_get.go#L320-L321

Added lines #L320 - L321 were not covered by tests
}

if !r.readHeader(&hdr) {
Expand Down Expand Up @@ -355,9 +363,12 @@ func (c *Client) ObjectHead(ctx context.Context, containerID cid.ID, objectID oi
err error
)

defer func() {
c.sendStatistic(stat.MethodObjectHead, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodObjectHead, time.Since(startTime), err)
}()
}

if signer == nil {
return nil, ErrMissingSigner
Expand Down Expand Up @@ -454,6 +465,7 @@ type ObjectRangeReader struct {
remainingPayloadLen int

statisticCallback shortStatisticCallback
startTime time.Time // if statisticCallback is set only
}

func (x *ObjectRangeReader) readChunk(buf []byte) (int, bool) {
Expand Down Expand Up @@ -553,7 +565,7 @@ func (x *ObjectRangeReader) Close() error {
var err error
if x.statisticCallback != nil {
defer func() {
x.statisticCallback(err)
x.statisticCallback(time.Since(x.startTime), err)

Check warning on line 568 in client/object_get.go

View check run for this annotation

Codecov / codecov/patch

client/object_get.go#L568

Added line #L568 was not covered by tests
}()
}
err = x.close(true)
Expand Down Expand Up @@ -606,9 +618,12 @@ func (c *Client) ObjectRangeInit(ctx context.Context, containerID cid.ID, object
err error
)

defer func() {
c.sendStatistic(stat.MethodObjectRange, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodObjectRange, time.Since(startTime), err)
}()
}

if length == 0 {
err = ErrZeroRangeLength
Expand Down Expand Up @@ -661,8 +676,11 @@ func (c *Client) ObjectRangeInit(ctx context.Context, containerID cid.ID, object
r.cancelCtxStream = cancel
r.stream = stream
r.client = c
r.statisticCallback = func(err error) {
c.sendStatistic(stat.MethodObjectRangeStream, err)()
if c.prm.statisticCallback != nil {
r.startTime = time.Now()
r.statisticCallback = func(dur time.Duration, err error) {
c.sendStatistic(stat.MethodObjectRangeStream, dur, err)
}

Check warning on line 683 in client/object_get.go

View check run for this annotation

Codecov / codecov/patch

client/object_get.go#L682-L683

Added lines #L682 - L683 were not covered by tests
}

return &r, nil
Expand Down
10 changes: 7 additions & 3 deletions client/object_hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package client
import (
"context"
"fmt"
"time"

"github.com/nspcc-dev/neofs-api-go/v2/acl"
v2object "github.com/nspcc-dev/neofs-api-go/v2/object"
Expand Down Expand Up @@ -109,9 +110,12 @@ func (c *Client) ObjectHash(ctx context.Context, containerID cid.ID, objectID oi
err error
)

defer func() {
c.sendStatistic(stat.MethodObjectHash, err)()
}()
if c.prm.statisticCallback != nil {
startTime := time.Now()
defer func() {
c.sendStatistic(stat.MethodObjectHash, time.Since(startTime), err)
}()
}

if len(prm.body.GetRanges()) == 0 {
err = ErrMissingRanges
Expand Down
Loading

0 comments on commit a093108

Please sign in to comment.