Repository navigation
fix(aws-sqs): emit queue-state CloudWatch gauges and resolve SenderId from caller - #1539
Open
Satyam-Trivedi-ZS wants to merge 2 commits into
Open
Satyam-Trivedi-ZS wants to merge 2 commits into
Satyam-Trivedi-ZS wants to merge 2 commits into
Conversation
Publish the AWS/SQS queue-state gauges (ApproximateNumberOfMessagesVisible, ApproximateNumberOfMessagesNotVisible, ApproximateNumberOfMessagesDelayed, ApproximateAgeOfOldestMessage and, for FIFO queues, ApproximateNumberOfGroupsWithInflightMessages) on the QueueName dimension after every action that changes a queue's messages, including DLQ redrive and StartMessageMoveTask. Metrics are now published outside the queue lock so an alarm action that delivers back into the queue cannot deadlock. SenderId is now the caller's IAM unique id (user id, or role id:session for an assumed role) resolved by the shared awsidentity.Resolver, matching GetCallerIdentity's UserId; library and service-to-service sends keep the account id. Part of stackshy#1514 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… low Part of stackshy#1514 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of #1514. This PR closes the AWS SQS CloudWatch gauge gaps and makes
SenderIdthe caller's IAM unique id, which is what real SQS reports.ApproximateNumberOfGroupsWithInflightMessagesApproximateNumberOfMessagesVisiblenever emittedApproximateNumberOfMessagesNotVisiblenever emittedApproximateNumberOfMessagesDelayednever emittedApproximateAgeOfOldestMessagenot tracked or emittedSenderIdalways the bare account IDawsidentity.Resolver, the same one STSGetCallerIdentityand EKS useNumberOfMessagesMovednot emitted on DLQ redrive / StartMessageMoveTaskWhat real AWS does (sources)
QueueNamedimension.ApproximateNumberOfGroupsWithInflightMessages.NumberOfMessagesMovedis not one of them. For DLQs it recommendsApproximateNumberOfMessagesVisible.ApproximateNumberOfMessagesMovedinListMessageMoveTasksandCancelMessageMoveTask, which cloudemu already returns.ABCDEFGHI1JKLMNOPQ23R. For an IAM role, returns the IAM role ID, for exampleABCDE1F2GH3I4JK5LMNOP:i-a123b456". This is the same value as theUserIdfromGetCallerIdentity.Changes
New file
providers/aws/sqs/metrics.gosampleGaugescounts, in one pass, the visible, in-flight and delayed messages, the FIFO groups with in-flight messages, and the oldest counted message.emitQueueGaugespublishes them onAWS/SQSwith theQueueNamedimension, in a singlePutMetricDatacall so alarms are evaluated once.Metrics are published after the queue lock is released
DeleteMessageused to publishNumberOfMessagesDeletedunder the lock. That is fixed too, andTestQueueGaugesPublishOutsideQueueLockcovers it.SenderIddriver.SendMessageInputanddriver.BatchSendEntrygain an optionalSenderIDfield.awsidentity.Resolver:<role id>:<session>AIDA…id thatGetCallerIdentityreports--enforce-auth, the verified principal is usedserver/aws/aws.gonow builds the resolver at the top ofnewServer, so STS, EKS and SQS share it.No new persisted state.
SenderIDwas already in the SQS snapshot, and the gauges are computed from message state.go generate ./...produces no coverage diff because no operations changed.Provider Coverage
Checklist
go test -p 2 ./...)developmentalready has existing findings. This PR adds no new findings in touched packages:golangci-lint run --new-from-rev=upstream/developmentover./providers/aws/sqs/... ./server/aws/sqs/... ./services/messagequeue/... ./server/aws/reports 0 issuescloudemu_test.go: covered by SDK wire tests inserver/aws/sqs_metrics_sdk_test.goinsteadgo generate ./..., no diff)Test Plan
Provider tests in
providers/aws/sqs/metrics_test.go(FakeClock):TestQueueGaugesTrackVisibleInFlightDelayedAndAge: send, delayed send, receive, ChangeVisibility, delete and purge, with exact gauge values and the age in seconds. An empty queue publishes no age.TestQueueGaugesAgeSkipsStandardPoisonPills: the third receive removes the message from the age.TestQueueGaugesFIFOCountsInflightGroupsTestQueueGaugesFollowDeadLetterRedriveAndMoveTask: the DLQ gauge rises on redrive, the age resets, and StartMessageMoveTask updates both queues.TestQueueGaugesFollowLambdaESMDeadLetterRedriveTestQueueGaugesPublishOutsideQueueLock: calls back into the queue from inside PutMetricData. The old in-lock publish would deadlock here.TestQueueGaugesNotEmittedWithoutMonitoringTestSenderIDFromCallerElseAccountSDK wire tests with real aws-sdk-go-v2 clients, in
server/aws/sqs_metrics_sdk_test.go:TestSDKSQSQueueGaugesReachCloudWatch: SQS send, delayed send and receive, then CloudWatchListMetricsandGetMetricStatistics.TestSDKSQSSenderIDMatchesCallerIdentity:SenderIdequals STSGetCallerIdentityUserId. After IAM CreateRole and STS AssumeRole, it equalsAssumedRoleId(AROA…:worker-1).Both SDK tests fail on
developmentand pass with this change. The failures ondevelopmentwere:ListMetrics AWS/SQS missing ApproximateNumberOfMessagesVisible (got [NumberOfMessagesReceived NumberOfMessagesSent SentMessageSize])expected "AIDA9F86D081884C7D65" actual "123456789012"Full checks
go build ./...is clean.go test -p 2 ./...passes (exit 0), including the SQS, SNS, Lambda ESM, CloudWatch, STS, EventBridge and S3-notification packages.golangci-lintadds no new findings in the touched packages; the run with--new-from-rev=upstream/developmentreports 0 issues. Without that flag, the touched SQS packages report 5 findings, all on lines this PR doesn't change:sqs/authz.gogoconst and nolintlint,sqs.go:1009prealloc incollectVisibleMessages, and 2 wsl findings inmovetask.go. They are also present ondevelopment.Real user run:
cloudemu serve --aws-port 4604driven with the AWS CLI (--endpoint-url http://127.0.0.1:4604,AWS_ACCESS_KEY_ID=test):Notes / known limits
GetQueueAttributesApproximateNumberOfMessagesNotVisiblestill counts delayed messages as not visible, while real AWS counts only in-flight messages. The new gauge uses the AWS definition. The attribute can be fixed separately....InQuietGroups,ApproximateNumberOfNoisyGroups) are not emitted, because cloudemu doesn't model noisy groups. TheNumberOfDeduplicatedSentMessagescounter is not part of this issue's gauge items.SenderId. Real AWS reports the delivering service's principal there.Related Issues
Part of #1514
🤖 Generated with Claude Code