Uh oh!
There was an error while loading. Please reload this page.
fix(aws cloudwatch, aws s3): emit component_sent_bytes_total through Driver to ensure component labels - #285
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAWS sinks now propagate resolved regions into request labels, emit component bytes-sent metrics, and avoid duplicate transport metrics. CloudWatch adds partial-upload byte accounting, while Kinesis and S3 responses report request byte sizes. ChangesShared metrics and AWS transport
CloudWatch partial upload metrics
Kinesis region and byte accounting
S3 region and byte accounting
SNS and SQS region labels
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Sink
participant Driver
participant AWSService
participant BytesSent
Sink->>Driver: configure HTTPS protocol and region label
Driver->>AWSService: send request
AWSService-->>Driver: return request byte size
Driver->>BytesSent: emit component bytes-sent metric
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Clee2691
commented
Jul 17, 2026
/hold |
Clee2691
commented
Jul 17, 2026
/hold cancel |
Clee2691
commented
Jul 20, 2026
/hold |
fdb9f35 to
0007855CompareClee2691
commented
Jul 21, 2026
/hold cancel |
Clee2691
commented
Jul 22, 2026
/retest |
Clee2691
commented
Jul 24, 2026
/hold |
jcantrill
commented
Jul 27, 2026
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Clee2691, jcantrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/sinks/aws_cloudwatch_logs/service.rs`:
- Around line 250-267: Prevent outer retries from replaying requests that
produce CloudwatchError::PutPartial, so the bytes_sent_handle emission in the
svc oneshot error path cannot be followed by full-request accounting on replay.
Update CloudwatchRetryLogic to classify PutPartial as non-retryable while
preserving existing retry behavior for other errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 69f88144-bf47-4663-a95f-ddc9b4f5e713
📒 Files selected for processing (31)
lib/vector-common/src/event_test_util.rslib/vector-common/src/internal_event/bytes_sent.rslib/vector-stream/src/driver.rssrc/aws/mod.rssrc/sinks/aws_cloudwatch_logs/config.rssrc/sinks/aws_cloudwatch_logs/integration_tests.rssrc/sinks/aws_cloudwatch_logs/request.rssrc/sinks/aws_cloudwatch_logs/request_builder.rssrc/sinks/aws_cloudwatch_logs/retry.rssrc/sinks/aws_cloudwatch_logs/service.rssrc/sinks/aws_cloudwatch_logs/sink.rssrc/sinks/aws_kinesis/config.rssrc/sinks/aws_kinesis/firehose/config.rssrc/sinks/aws_kinesis/firehose/integration_tests.rssrc/sinks/aws_kinesis/firehose/record.rssrc/sinks/aws_kinesis/service.rssrc/sinks/aws_kinesis/sink.rssrc/sinks/aws_kinesis/streams/config.rssrc/sinks/aws_kinesis/streams/integration_tests.rssrc/sinks/aws_kinesis/streams/record.rssrc/sinks/aws_s3/config.rssrc/sinks/aws_s3/integration_tests.rssrc/sinks/aws_s_s/service.rssrc/sinks/aws_s_s/sink.rssrc/sinks/aws_s_s/sns/config.rssrc/sinks/aws_s_s/sns/integration_tests.rssrc/sinks/aws_s_s/sqs/config.rssrc/sinks/aws_s_s/sqs/integration_tests.rssrc/sinks/s3_common/config.rssrc/sinks/s3_common/service.rssrc/sinks/s3_common/sink.rs
Uh oh!
There was an error while loading. Please reload this page.
…Driver to ensure component labels
Clee2691
commented
Jul 27, 2026
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/vector-common/src/internal_event/bytes_sent.rs`:
- Around line 9-16: Update the CloudWatch bytes-sent instrumentation around
CloudwatchResponse::bytes_sent, the service-level BytesSent registration, and
Driver emission so each successful request records component_sent_bytes_total
exactly once, preferably through Driver with component labels. Remove or exclude
the overlapping service-side emission, preserve the intended labels, and add a
regression assertion preventing duplicate or unlabeled series.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 16b6dc9e-d0bc-43ae-9cad-c29ce41375c6
📒 Files selected for processing (30)
lib/vector-common/src/event_test_util.rslib/vector-common/src/internal_event/bytes_sent.rslib/vector-stream/src/driver.rssrc/aws/mod.rssrc/sinks/aws_cloudwatch_logs/config.rssrc/sinks/aws_cloudwatch_logs/integration_tests.rssrc/sinks/aws_cloudwatch_logs/request.rssrc/sinks/aws_cloudwatch_logs/request_builder.rssrc/sinks/aws_cloudwatch_logs/service.rssrc/sinks/aws_cloudwatch_logs/sink.rssrc/sinks/aws_kinesis/config.rssrc/sinks/aws_kinesis/firehose/config.rssrc/sinks/aws_kinesis/firehose/integration_tests.rssrc/sinks/aws_kinesis/firehose/record.rssrc/sinks/aws_kinesis/service.rssrc/sinks/aws_kinesis/sink.rssrc/sinks/aws_kinesis/streams/config.rssrc/sinks/aws_kinesis/streams/integration_tests.rssrc/sinks/aws_kinesis/streams/record.rssrc/sinks/aws_s3/config.rssrc/sinks/aws_s3/integration_tests.rssrc/sinks/aws_s_s/service.rssrc/sinks/aws_s_s/sink.rssrc/sinks/aws_s_s/sns/config.rssrc/sinks/aws_s_s/sns/integration_tests.rssrc/sinks/aws_s_s/sqs/config.rssrc/sinks/aws_s_s/sqs/integration_tests.rssrc/sinks/s3_common/config.rssrc/sinks/s3_common/service.rssrc/sinks/s3_common/sink.rs
🚧 Files skipped from review as they are similar to previous changes (25)
- src/sinks/aws_s_s/sink.rs
- src/sinks/aws_cloudwatch_logs/sink.rs
- src/sinks/s3_common/sink.rs
- src/sinks/aws_kinesis/sink.rs
- src/sinks/aws_kinesis/firehose/integration_tests.rs
- src/sinks/s3_common/config.rs
- src/sinks/aws_kinesis/streams/record.rs
- src/sinks/aws_s_s/service.rs
- src/sinks/aws_kinesis/streams/integration_tests.rs
- src/sinks/aws_s_s/sqs/config.rs
- src/sinks/aws_kinesis/firehose/record.rs
- src/sinks/aws_s_s/sqs/integration_tests.rs
- lib/vector-stream/src/driver.rs
- src/sinks/aws_s_s/sns/config.rs
- src/sinks/s3_common/service.rs
- src/sinks/aws_kinesis/service.rs
- src/sinks/aws_s3/config.rs
- lib/vector-common/src/event_test_util.rs
- src/sinks/aws_kinesis/streams/config.rs
- src/sinks/aws_cloudwatch_logs/config.rs
- src/sinks/aws_s3/integration_tests.rs
- src/sinks/aws_cloudwatch_logs/request.rs
- src/sinks/aws_cloudwatch_logs/integration_tests.rs
- src/aws/mod.rs
- src/sinks/aws_cloudwatch_logs/service.rs
Uh oh!
There was an error while loading. Please reload this page.
Clee2691
commented
Jul 28, 2026
/retest |
jcantrill
commented
Jul 28, 2026
/lgtm |
Clee2691
commented
Jul 29, 2026
/hold cancel |
Uh oh!
There was an error while loading. Please reload this page.
Description
This is a forward port of #284 that also includes the fix for S3 for
v0.54.0-rhbranch.This is a bug for all AWS sinks.
/cc @vparfonov
/assign @jcantrill
Links
Related: #284
JIRA: https://redhat.atlassian.net/browse/LOG-9671