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.

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

source-zendesk-support: checkpoint during AuditLogs stream #2070

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

Alex-Bair
Copy link
Contributor

@Alex-Bair Alex-Bair commented Oct 21, 2024

Description:

For tasks that have a significant number of audit logs in Zendesk Support, it can take more than 24 hours to backfill. With the current AuditLogs stream strategy of reading records in descending order then stopping when we've reached a record we already read, we can't checkpoint until the backfill is completed. This means tasks that take more than 24 hours to backfill AuditLogs can get stuck restarting & attempting to backfill.

This PR updates the AuditLogs stream to read records in ascending order. This lets the connector checkpoint after each response & pick up where it left off when it's restarted.
Some noteable changes include:

  • Passing a list containing start_time and end_time for the filter[created_at][] query param to bound the responses' results to the specified timespan.
  • Pushing end_time 30 seconds in the past to avoid missing records if we query Zendesk in the middle of creating multiple records with the same created_at.
  • Keeping AuditLogs on the older get_updated_state method of updating state instead of migrating to the newer state property. This is to avoid rebackfilling all existing tasks.

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:

The upper time bound for the AuditLogs stream is moved a little in the past to avoid potential issues where we query Zendesk Support when its in the middle of creating multiple records with the same created_at. I don't have evidence that this can actually occur, but it sounds possible to me and a 30 second delay is relatively insignificant when the connector is running on 5 minute intervals.

Existing tasks do not need backfilled. The AuditLogs cursor value has remained the same (created_at for the most recent record) and the way the connector manages this state is the same.

Tested on a local stack. Confirmed:

  • Audit log records are received in ascending order.
  • State is checkpointed & documents are emitted after each response.
  • Checkpointed AuditLogs state is used after restarting the connector during a backfill.

This change is Reviewable

For tasks that have a significant number of audit logs in Zendesk
Support, it can take more than 24 hours to backfill. With the current
`AuditLogs` stream strategy of reading records in descending order then
stopping when we've reached a record we already read, we can't
checkpoint until the backfill is completed. This means tasks that take
more than 24 hours to backfill `AuditLogs` can get stuck restarting &
attempting to backfill.

This PR updates the `AuditLogs` stream to read records in ascending
order. This lets the connector checkpoint after each response & pick up
where it left off whenever it's restarted.
Some noteable changes include:
- Passing a list containing `start_time` and `end_time` for the
  `filter[created_at][]` query param to bound the responses' results to
  the specified timespan.
- Pushing `end_time` 30 seconds in the past to avoid missing records
  if we query Zendesk in the middle of creating multiple records with
  the same `created_at`.
- Keeping `AuditLogs` on the older `get_updated_state` method of
  updating state instead of migrating to the newer `state` property.
  This is to avoid rebackfilling all existing tasks.
@Alex-Bair Alex-Bair added the change:unplanned Unplanned change, useful for things like doc updates label Oct 21, 2024
@Alex-Bair Alex-Bair requested a review from a team October 21, 2024 15:41
Copy link
Member

@williamhbaker williamhbaker 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 Alex-Bair merged commit c65157a into main Oct 21, 2024
72 of 77 checks passed
@Alex-Bair Alex-Bair deleted the bair/zendesk-support-audit-logs-checkpoints branch October 21, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:unplanned Unplanned change, useful for things like doc updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants