Skip to content

estuary-cdk: slightly better error logging#3019

Merged
Alex-Bair merged 2 commits intomainfrom
bair/estuary-cdk-error-logging
Jul 4, 2025
Merged

estuary-cdk: slightly better error logging#3019
Alex-Bair merged 2 commits intomainfrom
bair/estuary-cdk-error-logging

Conversation

@Alex-Bair
Copy link
Copy Markdown
Member

@Alex-Bair Alex-Bair commented Jul 3, 2025

Description:

This PR includes some small logging improvements for CDK based captures & source-stripe-native. See the individual commits for more details.

Workflow steps:

(How does one use this feature, and how has it changed)

Documentation links affected:

(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)

Notes for reviewers:

(anything that might help someone review this PR)


This change is Reviewable

@Alex-Bair Alex-Bair requested a review from Copilot July 3, 2025 19:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refines task naming in the Stripe capture and enhances error logging when the first error has an empty message.

  • Removes the connector-specific prefix from spawned subtask names in priority_capture.py
  • Ensures a non-empty error description by injecting the exception type’s name when raising Stopped in base_capture_connector.py

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
source-stripe-native/source_stripe_native/priority_capture.py Simplified subtask names by dropping self.prefix
estuary-cdk/estuary_cdk/capture/base_capture_connector.py Fallback to exception class name when first_error has no text
Comments suppressed due to low confidence (1)

source-stripe-native/source_stripe_native/priority_capture.py:300

  • [nitpick] Removing self.prefix from subtask names may break consistency with other connector logs and metrics. Consider preserving the prefix or updating documentation to reflect the new naming convention.
                f"backfill.{work_item.account_id}",

Comment thread estuary-cdk/estuary_cdk/capture/base_capture_connector.py Outdated
@Alex-Bair Alex-Bair force-pushed the bair/estuary-cdk-error-logging branch from 3cc52ef to 1671d5c Compare July 3, 2025 19:26
@Alex-Bair Alex-Bair marked this pull request as ready for review July 3, 2025 19:36
@Alex-Bair Alex-Bair requested a review from JustinASmith July 3, 2025 19:36
Copy link
Copy Markdown
Contributor

@JustinASmith JustinASmith left a comment

Choose a reason for hiding this comment

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

LGTM

Alex-Bair added 2 commits July 4, 2025 10:39
When an error without any arguments is raised, like `asyncio` does for
`CancelledError` and `TimeoutError`, the string representation of the
error is an empty string. That's not useful at all, so when the error's
string representation is empty, replace it with the error's type so
something somewhat useful is included in the error log.
Previously, duplicate information were in the subtask suffixes,
resulting in names like
"flow.capture.Accounts.worker.1.Accounts.backfill.acct_12345".

This commit removes the additional "Account" prefix from the subtask name.
@Alex-Bair Alex-Bair force-pushed the bair/estuary-cdk-error-logging branch from 1671d5c to 274f2c9 Compare July 4, 2025 14:40
@Alex-Bair Alex-Bair merged commit 9bf33d1 into main Jul 4, 2025
95 of 104 checks passed
@Alex-Bair Alex-Bair deleted the bair/estuary-cdk-error-logging branch July 4, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants