Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal: true up internal metrics collection for post-SRA middleware #2642

Merged
merged 2 commits into from
May 15, 2024
Merged

internal: true up internal metrics collection for post-SRA middleware #2642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0fd81c8
true up internal metrics collection for post-SRA middleware
lucix-aws May 13, 2024
3266adf
suffix Time
lucix-aws May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changelog/ce369d28a2594b29aa49cd801c4ff959.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "ce369d28-a259-4b29-aa49-cd801c4ff959",
"type": "bugfix",
"description": "Adjust internal metrics collection for revised authentication workflow.",
"modules": [
"."
]
}
4 changes: 2 additions & 2 deletions aws/middleware/private/metrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ type MetricData struct {
ResolveEndpointStartTime time.Time
ResolveEndpointEndTime time.Time
EndpointResolutionDuration time.Duration
GetIdentityStartTime time.Time
GetIdentityEndTime time.Time
InThroughput float64
OutThroughput float64
RetryCount int
Expand Down Expand Up @@ -144,8 +146,6 @@ type AttemptMetrics struct {
ConnRequestedTime time.Time
ConnObtainedTime time.Time
ConcurrencyAcquireDuration time.Duration
CredentialFetchStartTime time.Time
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a bit more context as to why are we removing these

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's no longer a per-attempt metric with the SRA auth refactor - credential (abstractly referred to now as identity) retrieval happened as part of the signing process in the now-deprecated sigv4 middleware, as part of an attempt lifecycle. Now it's its own middleware (GetIdentity) separate from signing and before the retry middleware i.e. before the attempt loop starts.

Long-term I think we do want to pull the start of the retry loop further back but that's out of scope for now.

CredentialFetchEndTime time.Time
SignStartTime time.Time
SignEndTime time.Time
SigningDuration time.Duration
Expand Down
4 changes: 0 additions & 4 deletions aws/middleware/private/metrics/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ func TestMetricData_ComputeRequestMetrics(t *testing.T) {
FirstByteTime: time.Unix(1334, 0),
ConnRequestedTime: time.Unix(1234, 0),
ConnObtainedTime: time.Unix(1434, 0),
CredentialFetchStartTime: time.Unix(1234, 0),
CredentialFetchEndTime: time.Unix(1434, 0),
SignStartTime: time.Unix(1234, 0),
SignEndTime: time.Unix(1434, 0),
DeserializeStartTime: time.Unix(1234, 0),
Expand All @@ -133,8 +131,6 @@ func TestMetricData_ComputeRequestMetrics(t *testing.T) {
FirstByteTime: time.Unix(1334, 0),
ConnRequestedTime: time.Unix(1234, 0),
ConnObtainedTime: time.Unix(1434, 0),
CredentialFetchStartTime: time.Unix(1234, 0),
CredentialFetchEndTime: time.Unix(1434, 0),
SignStartTime: time.Unix(1234, 0),
SignEndTime: time.Unix(1434, 0),
DeserializeStartTime: time.Unix(1234, 0),
Expand Down
13 changes: 10 additions & 3 deletions aws/middleware/private/metrics/middleware/configuration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package middleware

import (
"net/http"

"github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics"
"github.com/aws/smithy-go/middleware"
"net/http"
)

func WithMetricMiddlewares(
Expand All @@ -20,10 +21,10 @@ func WithMetricMiddlewares(
if err := stack.Serialize.Add(GetRecordStackSerializeEndMiddleware(), middleware.After); err != nil {
return err
}
if err := stack.Serialize.Insert(GetRecordEndpointResolutionStartMiddleware(), "ResolveEndpoint", middleware.Before); err != nil {
if err := stack.Finalize.Insert(GetRecordEndpointResolutionStartMiddleware(), "ResolveEndpointV2", middleware.Before); err != nil {
return err
}
if err := stack.Serialize.Insert(GetRecordEndpointResolutionEndMiddleware(), "ResolveEndpoint", middleware.After); err != nil {
if err := stack.Finalize.Insert(GetRecordEndpointResolutionEndMiddleware(), "ResolveEndpointV2", middleware.After); err != nil {
return err
}
if err := stack.Build.Add(GetWrapDataStreamMiddleware(), middleware.After); err != nil {
Expand All @@ -47,6 +48,12 @@ func WithMetricMiddlewares(
if err := stack.Deserialize.Insert(GetTransportMetricsMiddleware(), "StackDeserializeStart", middleware.After); err != nil {
return err
}
if err := timeGetIdentity(stack); err != nil {
return err
}
if err := timeSigning(stack); err != nil {
return err
}
return nil
}
}
13 changes: 13 additions & 0 deletions aws/middleware/private/metrics/middleware/endpoint_resolution_end.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package middleware

import (
"context"

"github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics"
"github.com/aws/aws-sdk-go-v2/internal/sdk"
"github.com/aws/smithy-go/middleware"
Expand All @@ -21,6 +22,8 @@ func (m *EndpointResolutionEnd) ID() string {
return "EndpointResolutionEnd"
}

// Deprecated: Endpoint resolution now occurs in Finalize. The ResolveEndpoint
// middleware remains in serialize but is largely a no-op.
func (m *EndpointResolutionEnd) HandleSerialize(
ctx context.Context, in middleware.SerializeInput, next middleware.SerializeHandler,
) (
Expand All @@ -34,3 +37,13 @@ func (m *EndpointResolutionEnd) HandleSerialize(

return out, metadata, err
}

func (m *EndpointResolutionEnd) HandleFinalize(
ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler,
) (
middleware.FinalizeOutput, middleware.Metadata, error,
) {
mctx := metrics.Context(ctx)
mctx.Data().ResolveEndpointEndTime = sdk.NowTime()
return next.HandleFinalize(ctx, in)
}
13 changes: 13 additions & 0 deletions aws/middleware/private/metrics/middleware/endpoint_resolution_start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package middleware

import (
"context"

"github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics"
"github.com/aws/aws-sdk-go-v2/internal/sdk"
"github.com/aws/smithy-go/middleware"
Expand All @@ -21,6 +22,8 @@ func (m *EndpointResolutionStart) ID() string {
return "EndpointResolutionStart"
}

// Deprecated: Endpoint resolution now occurs in Finalize. The ResolveEndpoint
// middleware remains in serialize but is largely a no-op.
func (m *EndpointResolutionStart) HandleSerialize(
ctx context.Context, in middleware.SerializeInput, next middleware.SerializeHandler,
) (
Expand All @@ -34,3 +37,13 @@ func (m *EndpointResolutionStart) HandleSerialize(

return out, metadata, err
}

func (m *EndpointResolutionStart) HandleFinalize(
ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler,
) (
middleware.FinalizeOutput, middleware.Metadata, error,
) {
mctx := metrics.Context(ctx)
mctx.Data().ResolveEndpointStartTime = sdk.NowTime()
return next.HandleFinalize(ctx, in)
}
47 changes: 47 additions & 0 deletions aws/middleware/private/metrics/middleware/identity.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package middleware

import (
"context"

"github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics"
"github.com/aws/aws-sdk-go-v2/internal/sdk"
"github.com/aws/smithy-go/middleware"
)

func timeGetIdentity(stack *middleware.Stack) error {
if err := stack.Finalize.Insert(getIdentityStart{}, "GetIdentity", middleware.Before); err != nil {
return err
}
if err := stack.Finalize.Insert(getIdentityEnd{}, "GetIdentity", middleware.After); err != nil {
return err
}
return nil
}

type getIdentityStart struct{}

func (m getIdentityStart) ID() string { return "getIdentityStart" }

func (m getIdentityStart) HandleFinalize(
ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler,
) (
out middleware.FinalizeOutput, md middleware.Metadata, err error,
) {
mctx := metrics.Context(ctx)
mctx.Data().GetIdentityStartTime = sdk.NowTime()
return next.HandleFinalize(ctx, in)
}

type getIdentityEnd struct{}

func (m getIdentityEnd) ID() string { return "getIdentityEnd" }

func (m getIdentityEnd) HandleFinalize(
ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler,
) (
out middleware.FinalizeOutput, md middleware.Metadata, err error,
) {
mctx := metrics.Context(ctx)
mctx.Data().GetIdentityEndTime = sdk.NowTime()
return next.HandleFinalize(ctx, in)
}
57 changes: 57 additions & 0 deletions aws/middleware/private/metrics/middleware/signing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package middleware

import (
"context"

"github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics"
"github.com/aws/aws-sdk-go-v2/internal/sdk"
"github.com/aws/smithy-go/middleware"
)

func timeSigning(stack *middleware.Stack) error {
if err := stack.Finalize.Insert(signingStart{}, "Signing", middleware.Before); err != nil {
return err
}
if err := stack.Finalize.Insert(signingEnd{}, "Signing", middleware.After); err != nil {
return err
}
return nil
}

type signingStart struct{}

func (m signingStart) ID() string { return "signingStart" }

func (m signingStart) HandleFinalize(
ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler,
) (
out middleware.FinalizeOutput, md middleware.Metadata, err error,
) {
mctx := metrics.Context(ctx)
attempt, err := mctx.Data().LatestAttempt()
if err != nil {
return out, md, err
}

attempt.SignStartTime = sdk.NowTime()
return next.HandleFinalize(ctx, in)
}

type signingEnd struct{}

func (m signingEnd) ID() string { return "signingEnd" }

func (m signingEnd) HandleFinalize(
ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler,
) (
out middleware.FinalizeOutput, md middleware.Metadata, err error,
) {
mctx := metrics.Context(ctx)
attempt, err := mctx.Data().LatestAttempt()
if err != nil {
return out, md, err
}

attempt.SignEndTime = sdk.NowTime()
return next.HandleFinalize(ctx, in)
}
4 changes: 0 additions & 4 deletions aws/middleware/private/metrics/publisher/emf_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ func TestPostRequestMetrics(t *testing.T) {
FirstByteTime: time.Unix(1234, 0),
ConnRequestedTime: time.Unix(1234, 0),
ConnObtainedTime: time.Unix(1434, 0),
CredentialFetchStartTime: time.Unix(1234, 0),
CredentialFetchEndTime: time.Unix(1434, 0),
SignStartTime: time.Unix(1234, 0),
SignEndTime: time.Unix(1434, 0),
DeserializeStartTime: time.Unix(1234, 0),
Expand All @@ -102,8 +100,6 @@ func TestPostRequestMetrics(t *testing.T) {
FirstByteTime: time.Unix(1234, 0),
ConnRequestedTime: time.Unix(1234, 0),
ConnObtainedTime: time.Unix(1434, 0),
CredentialFetchStartTime: time.Unix(1234, 0),
CredentialFetchEndTime: time.Unix(1434, 0),
SignStartTime: time.Unix(1234, 0),
SignEndTime: time.Unix(1434, 0),
DeserializeStartTime: time.Unix(1234, 0),
Expand Down
29 changes: 0 additions & 29 deletions aws/signer/v4/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"github.com/aws/aws-sdk-go-v2/aws"
awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
"github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics"
v4Internal "github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4"
internalauth "github.com/aws/aws-sdk-go-v2/internal/auth"
"github.com/aws/aws-sdk-go-v2/internal/sdk"
Expand Down Expand Up @@ -301,22 +300,7 @@ func (s *SignHTTPRequestMiddleware) HandleFinalize(ctx context.Context, in middl
return out, metadata, &SigningError{Err: fmt.Errorf("computed payload hash missing from context")}
}

mctx := metrics.Context(ctx)

if mctx != nil {
if attempt, err := mctx.Data().LatestAttempt(); err == nil {
attempt.CredentialFetchStartTime = sdk.NowTime()
}
}

credentials, err := s.credentialsProvider.Retrieve(ctx)

if mctx != nil {
if attempt, err := mctx.Data().LatestAttempt(); err == nil {
attempt.CredentialFetchEndTime = sdk.NowTime()
}
}

if err != nil {
return out, metadata, &SigningError{Err: fmt.Errorf("failed to retrieve credentials: %w", err)}
}
Expand All @@ -337,20 +321,7 @@ func (s *SignHTTPRequestMiddleware) HandleFinalize(ctx context.Context, in middl
})
}

if mctx != nil {
if attempt, err := mctx.Data().LatestAttempt(); err == nil {
attempt.SignStartTime = sdk.NowTime()
}
}

err = s.signer.SignHTTP(ctx, credentials, req.Request, payloadHash, signingName, signingRegion, sdk.NowTime(), signerOptions...)

if mctx != nil {
if attempt, err := mctx.Data().LatestAttempt(); err == nil {
attempt.SignEndTime = sdk.NowTime()
}
}

if err != nil {
return out, metadata, &SigningError{Err: fmt.Errorf("failed to sign http request, %w", err)}
}
Expand Down
Loading