From a093108259370f885c277eb2326debd99e5a29d4 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Fri, 22 Nov 2024 19:32:43 +0300 Subject: [PATCH] client: Fix duration in operation statistics 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 --- client/accounting.go | 10 +++++-- client/client.go | 11 ++----- client/container.go | 64 +++++++++++++++++++++++++++-------------- client/netmap.go | 28 ++++++++++++------ client/object_delete.go | 10 +++++-- client/object_get.go | 48 +++++++++++++++++++++---------- client/object_hash.go | 10 +++++-- client/object_put.go | 22 +++++++++----- client/object_search.go | 20 +++++++++---- client/reputation.go | 19 ++++++++---- client/session.go | 10 +++++-- 11 files changed, 167 insertions(+), 85 deletions(-) diff --git a/client/accounting.go b/client/accounting.go index 8a1f3e2e..8be2ec4e 100644 --- a/client/accounting.go +++ b/client/accounting.go @@ -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" @@ -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(): diff --git a/client/client.go b/client/client.go index 4cad72bf..59e44f16 100644 --- a/client/client.go +++ b/client/client.go @@ -219,15 +219,8 @@ func (c *Client) Close() error { return c.Conn().Close() } -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. diff --git a/client/container.go b/client/container.go index 58287ad3..9e4c87fe 100644 --- a/client/container.go +++ b/client/container.go @@ -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" @@ -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 @@ -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) @@ -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 @@ -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 @@ -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) @@ -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 @@ -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 diff --git a/client/netmap.go b/client/netmap.go index 46861367..823d55d8 100644 --- a/client/netmap.go +++ b/client/netmap.go @@ -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" @@ -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 @@ -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 @@ -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 diff --git a/client/object_delete.go b/client/object_delete.go index 435e1709..f4e93470 100644 --- a/client/object_delete.go +++ b/client/object_delete.go @@ -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" @@ -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) diff --git a/client/object_get.go b/client/object_get.go index 09360d8c..b0c38bfd 100644 --- a/client/object_get.go +++ b/client/object_get.go @@ -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" @@ -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. @@ -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) }() } err = x.close(true) @@ -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 @@ -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) + } } if !r.readHeader(&hdr) { @@ -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 @@ -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) { @@ -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) }() } err = x.close(true) @@ -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 @@ -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) + } } return &r, nil diff --git a/client/object_hash.go b/client/object_hash.go index a8b633c6..2564597e 100644 --- a/client/object_hash.go +++ b/client/object_hash.go @@ -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" @@ -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 diff --git a/client/object_put.go b/client/object_put.go index 9ffa00b0..75b993a7 100644 --- a/client/object_put.go +++ b/client/object_put.go @@ -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" @@ -37,7 +38,7 @@ type putObjectStream interface { // shortStatisticCallback is a shorter version of [stat.OperationCallback] which is calling from [client.Client]. // The difference is the client already know some info about itself. Despite it the client doesn't know // duration and error from writer/reader. -type shortStatisticCallback func(err error) +type shortStatisticCallback func(dur time.Duration, err error) // PrmObjectPutInit groups parameters of ObjectPutInit operation. type PrmObjectPutInit struct { @@ -90,6 +91,7 @@ type DefaultObjectWriter struct { partChunk v2object.PutObjectPartChunk statisticCallback shortStatisticCallback + startTime time.Time // if statisticCallback is set only buf []byte bufCleanCallback func() @@ -223,7 +225,7 @@ func (x *DefaultObjectWriter) Write(chunk []byte) (n int, err error) { func (x *DefaultObjectWriter) Close() error { if x.statisticCallback != nil { defer func() { - x.statisticCallback(x.err) + x.statisticCallback(time.Since(x.startTime), x.err) }() } @@ -294,12 +296,18 @@ func (x *DefaultObjectWriter) GetResult() ResObjectPut { // - [ErrMissingSigner] func (c *Client) ObjectPutInit(ctx context.Context, hdr object.Object, signer user.Signer, prm PrmObjectPutInit) (ObjectWriter, error) { var err error - defer func() { - c.sendStatistic(stat.MethodObjectPut, err)() - }() + if c.prm.statisticCallback != nil { + startTime := time.Now() + defer func() { + c.sendStatistic(stat.MethodObjectPut, time.Since(startTime), err) + }() + } var w DefaultObjectWriter - w.statisticCallback = func(err error) { - c.sendStatistic(stat.MethodObjectPutStream, err)() + if c.prm.statisticCallback != nil { + w.startTime = time.Now() + w.statisticCallback = func(dur time.Duration, err error) { + c.sendStatistic(stat.MethodObjectPutStream, dur, err) + } } if signer == nil { diff --git a/client/object_search.go b/client/object_search.go index b1303713..5e7cf8d4 100644 --- a/client/object_search.go +++ b/client/object_search.go @@ -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" @@ -75,6 +76,7 @@ type ObjectListReader struct { tail []v2refs.ObjectID statisticCallback shortStatisticCallback + startTime time.Time // if statisticCallback is set only } // Read reads another list of the object identifiers. Works similar to @@ -172,7 +174,7 @@ func (x *ObjectListReader) Close() error { var err error if x.statisticCallback != nil { defer func() { - x.statisticCallback(err) + x.statisticCallback(time.Since(x.startTime), err) }() } @@ -201,9 +203,12 @@ func (x *ObjectListReader) Close() error { // - [ErrMissingSigner] func (c *Client) ObjectSearchInit(ctx context.Context, containerID cid.ID, signer user.Signer, prm PrmObjectSearch) (*ObjectListReader, error) { var err error - defer func() { - c.sendStatistic(stat.MethodObjectSearch, err)() - }() + if c.prm.statisticCallback != nil { + startTime := time.Now() + defer func() { + c.sendStatistic(stat.MethodObjectSearch, time.Since(startTime), err) + }() + } if signer == nil { return nil, ErrMissingSigner @@ -239,8 +244,11 @@ func (c *Client) ObjectSearchInit(ctx context.Context, containerID cid.ID, signe return nil, err } r.client = c - r.statisticCallback = func(err error) { - c.sendStatistic(stat.MethodObjectSearchStream, err)() + if c.prm.statisticCallback != nil { + r.startTime = time.Now() + r.statisticCallback = func(dur time.Duration, err error) { + c.sendStatistic(stat.MethodObjectSearchStream, dur, err) + } } return &r, nil diff --git a/client/reputation.go b/client/reputation.go index 444c5012..d96a5496 100644 --- a/client/reputation.go +++ b/client/reputation.go @@ -2,6 +2,7 @@ package client import ( "context" + "time" v2reputation "github.com/nspcc-dev/neofs-api-go/v2/reputation" protoreputation "github.com/nspcc-dev/neofs-api-go/v2/reputation/grpc" @@ -29,9 +30,12 @@ type PrmAnnounceLocalTrust struct { // Parameter trusts must not be empty. func (c *Client) AnnounceLocalTrust(ctx context.Context, epoch uint64, trusts []reputation.Trust, prm PrmAnnounceLocalTrust) error { var err error - defer func() { - c.sendStatistic(stat.MethodAnnounceLocalTrust, err)() - }() + if c.prm.statisticCallback != nil { + startTime := time.Now() + defer func() { + c.sendStatistic(stat.MethodAnnounceLocalTrust, time.Since(startTime), err) + }() + } // check parameters switch { @@ -117,9 +121,12 @@ func (x *PrmAnnounceIntermediateTrust) SetIteration(iter uint32) { // Parameter epoch must not be zero. func (c *Client) AnnounceIntermediateTrust(ctx context.Context, epoch uint64, trust reputation.PeerToPeerTrust, prm PrmAnnounceIntermediateTrust) error { var err error - defer func() { - c.sendStatistic(stat.MethodAnnounceIntermediateTrust, err)() - }() + if c.prm.statisticCallback != nil { + startTime := time.Now() + defer func() { + c.sendStatistic(stat.MethodAnnounceIntermediateTrust, time.Since(startTime), err) + }() + } if epoch == 0 { err = ErrZeroEpoch diff --git a/client/session.go b/client/session.go index 99631ab8..46930e11 100644 --- a/client/session.go +++ b/client/session.go @@ -2,6 +2,7 @@ package client import ( "context" + "time" "github.com/nspcc-dev/neofs-api-go/v2/refs" v2session "github.com/nspcc-dev/neofs-api-go/v2/session" @@ -78,9 +79,12 @@ func (x ResSessionCreate) PublicKey() []byte { // - [ErrMissingSigner] func (c *Client) SessionCreate(ctx context.Context, signer user.Signer, prm PrmSessionCreate) (*ResSessionCreate, error) { var err error - defer func() { - c.sendStatistic(stat.MethodSessionCreate, err)() - }() + if c.prm.statisticCallback != nil { + startTime := time.Now() + defer func() { + c.sendStatistic(stat.MethodSessionCreate, time.Since(startTime), err) + }() + } if signer == nil { return nil, ErrMissingSigner