source-linkedin-ads-v2: fix bugs causing missing data #2107
Conversation
This file contains 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
…` records
Background: LinkedIn does not always include the name of the `creative`
when we request multiple creatives, but multiple users would like a
`creative_name` field on each record if the creative has a name. We make
additional requests to a different endpoint to fetch `creative_name`s.
I've fixed some bugs in the existing logic to fetch the `creative_name`.
These improvements include:
- Supporting tasks that authenticate with non-OAuth2 methods.
- Emitting records even when an exception is raised trying to fetch a
`creative_name`.
- Logging a more descriptive warning when an exception is raised.
- Using the versioned endpoint `/rest/posts/{encoded_share_urn}` to
fetch creative names since LinkedIn is sunsetting legacy `/v2`
endpoints.
- Added a new parameter to `read_records` that controls whether
additional properties are fetched for each record. This is useful when
we only need the ID of the parent record for the ad analytics streams.
…streams Multiple different methods were being used to update the state for `LinkedInAdAnalyticsStream` streams: - `get_updated_state` was inspecting each emitted record and updating the stream state to the most recent date. - `state_checkpoint_interval` was checkpointing state after every 1000 emitted records. - `stream_slices` was checkpointing state after each "slice" was completed. Couple this tangled web of state management with an incorrect iteration order, and the connector ended up missing massive quantities of records. Essentially, the connector was using the most recent date from the previous resource as the start date for the next resource instead of starting each resource from the config's start date / most recent cursor. I fixed this by stripping down the state management to only use the newer `state` property to have finer control of when state is updated (`stream_slices` is still used to manage date & field slicing, but we only rely on the `state` property when determining what slices to create).
Snapshots are identical. This only changes the snapshot names to align with what's generated by `pytest` automatically, making development easier.
Alex-Bair
force-pushed
the
bair/linkedin-ads-v2-missing-data
branch
from
6643942
to
b001315
Compare
williamhbaker
approved these changes
Nov 1, 2024
2 tasks
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.
Description:
We received reports of missing data for the
AdCampaignAnalytics,AdCreativeAnalytics, andCreativesstreams.Changes in this PR include:
LinkedInAdsAnalyticsStreamState management issues were causing the most recent date from the previous resource to be used as the start date for the next resource. Almost every state management tool exposed by the Airbyte CDK was being used, causing the stream's state to update when it shouldn't. I converted the stream to use the newer
stateproperty to have finer control of when state is updated.These
LinkedInAdsAnalyticsStreamchanges address the missing data issues forAdCampaignAnalyticsandAdCreativeAnalytics.CreativesThe logic for adding a
creative_nameto each record was causing the connector to not emit a record if an exception occurred trying to fetch thecreative_name. That logic has been refactored and improved. Improvements include:creative_name./rest/posts/{encoded_share_urn}to fetchcreative_names since LinkedIn is sunsetting the legacy/v2endpoints.Snapshot tests
pytestautomatically generates.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:
Tested on a local stack. Confirmed:
CreativesandAdCampaignAnalyticsmatches up with the expected number of records.AdCreativeAnalyticsis significantly more than the connector was previously receiving.AdCreativeAnalyticscan take 24+ hours to backfill, so I only ran a partial backfill for it. Since bothCreativesandAdCampaignAnalyticsare now getting the expected number of records &AdCreativeAnalyticsis getting significantly more records than before, I feel fairly confident that the missing data issues are resolved forAdCreativeAnalytics.creative_nameretrieved from the newer/rest/posts/{encoded_share_urn}is the same as thecreative_nameretrieved from the legacy/v2endpoint.All
Ad${resource]Analyticsbindings for existing tasks will need backfilled since how they manage state has changed.This change is