feat(aws-kinesis): flag-gated diagnostic latency logging (STRATCONN-6690) - #3946
Draft
harsh-joshi99 wants to merge 1 commit into
Draft
feat(aws-kinesis): flag-gated diagnostic latency logging (STRATCONN-6690)#3946harsh-joshi99 wants to merge 1 commit into
harsh-joshi99 wants to merge 1 commit into
Conversation
…NN-6690) Kinesis deliveries intermittently fail with a 504 that is not the AWS API Gateway 29s timeout but Segment's internal execution deadline aborting the request. The delivery path makes 3 sequential calls per batch with no credential caching (2x STS AssumeRole + 1 Kinesis PutRecords), and the abort branch previously threw RequestTimeoutError before any log/metric, so the failure was invisible in Grafana. Adds instrumentation behind the `actions-aws-kinesis-advanced-logging` Flagon gate (all emits gated; no behavior change when off): - Per-phase latency: sts_intermediary_ms, sts_customer_ms, sts_assume_role_ms, kinesis_put_records_ms (histograms + info logs). - Abort branch now emits actions_kinesis.request_timeout + an error log with the in-flight phase before throwing, closing the silent-failure gap. Logs contain only durations, stream name, AWS region, batch size, and phase label — no PII. Follows the salesforce-advanced-logging precedent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds flag-gated diagnostic instrumentation to the AWS Kinesis destination to make “execution deadline exceeded” (internal 504) timeouts observable and attributable to STS vs. PutRecords phases.
Changes:
- Plumbs a Flagon feature gate into the Kinesis
sendpath to enable/disable advanced logging. - Adds per-phase latency histograms and timeout/error logging (AbortError) to pinpoint where deadlines are breached.
- Splits STS assume-role latency into intermediary vs. customer role metrics.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/destination-actions/src/lib/AWS/sts.ts | Adds optional StsLogging and emits per-assume-role latency metrics/logs. |
| packages/destination-actions/src/lib/AWS/test/index.test.ts | Adds tests verifying STS per-call metrics/logging are gated by advancedLogging. |
| packages/destination-actions/src/destinations/aws-kinesis/utils.ts | Adds phase tracking, abort diagnostics, and latency metrics for STS-total + PutRecords. |
| packages/destination-actions/src/destinations/aws-kinesis/send/index.ts | Reads Flagon feature and passes advancedLogging into send/sendBatch. |
| packages/destination-actions/src/destinations/aws-kinesis/tests/utils.test.ts | Adds tests for silent/verbose behavior and AbortError diagnostics under the gate. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+179
to
+185
| const start = Date.now() | ||
| const result = await stsClient.send(command) | ||
| if (logging?.advancedLogging && metric) { | ||
| const durationMs = Date.now() - start | ||
| logging.statsContext?.statsClient?.histogram(`actions_kinesis.${metric}`, durationMs, logging.statsContext?.tags) | ||
| logging.logger?.info(`[aws-kinesis] assume_role phase=${metric} durationMs=${durationMs} region=${region}`) | ||
| } |
Comment on lines
+57
to
+58
| const start = Date.now() | ||
| let phase: 'sts' | 'kinesis' = 'sts' |
Comment on lines
+65
to
+67
| if (advancedLogging) { | ||
| statsContext?.statsClient?.histogram('actions_kinesis.sts_assume_role_ms', Date.now() - start, tags) | ||
| } |
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
Customer Whatnot (STRATCONN-6690) sees intermittent 504 Gateway Timeout on the Amazon Kinesis destination. The 504 is not the AWS API Gateway 29s integration timeout — it's Segment's internal execution deadline aborting the request. Two things made this un-diagnosable:
AssumeRole(intermediary role, then customer role) + 1 KinesisPutRecords. Their cumulative tail latency breaches the deadline.RequestTimeoutErrorbefore any log or metric, so the failure was completely invisible in Grafana/Loki — which is why the investigation stalled.This PR adds diagnostic instrumentation to attribute the timeout to a specific phase (STS#1 / STS#2 / PutRecords), gated behind a Flagon flag so there's zero footprint until enabled.
Changes
All emits are gated on
features['actions-aws-kinesis-advanced-logging'](nothing logs/emits when the flag is off — no behavior change). Follows the existingsalesforce-advanced-loggingprecedent.Flagon gate: https://flagon.segment.com/families/centrifuge-destinations/gates/actions-aws-kinesis-advanced-logging
aws-kinesis/send/index.ts— read the flag offfeatures, pass a boolean intosend.aws-kinesis/utils.ts— time the STS-total and PutRecords phases; track the in-flightphase; onAbortErroremitactions_kinesis.request_timeout+ an error log with the phase/elapsed before throwing (closes the silent-failure gap).lib/AWS/sts.ts— split the two assume-role calls intosts_intermediary_msvssts_customer_ms(optionalStsLoggingarg;assumeRoleis Kinesis-only,testAuthenticationunaffected).New metrics
actions_kinesis.sts_intermediary_ms,sts_customer_ms,sts_assume_role_ms,kinesis_put_records_ms(histograms), andactions_kinesis.request_timeout(counter).Sample log lines
PII
None. Logs contain only durations, stream name, AWS region, batch size (a count), and phase label — no payload, partition keys, or credentials.
Testing
aws-kinesis/__tests__/utils.test.ts(flag off = silent; flag on success = metrics + log; logging context forwarded; abort-with-flag = timeout metric + error log + still throws; abort-without-flag = silent + still throws).lib/AWS/__test__/index.test.ts(per-call latency metrics on/off).Rollout
actions-aws-kinesis-advanced-logging.6421c0a60cc41c0c431f13d9.🤖 Generated with Claude Code