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 Facebook Marketing: Allow user to set time_increment parameter for Insights API #6225

Closed
jd-sanders opened this issue Sep 17, 2021 · 11 comments · Fixed by #10531
Closed

Comments

@jd-sanders
Copy link

Tell us about the problem you're trying to solve

In the current behavior of this connector, the call to the Insights API returns data in single day increments.

However, the field reach doesn't sum correctly over days, so there is no way to correctly determine the reach of your ads over a greater time period. The problem also affects the parameter frequency. In order to have the correct reach over a greater time period, you would need to be able to ask for metrics aggregated over a greater "time slice".

Describe the solution you’d like

The Insights API supports a request for a broader aggregate of metrics over an extended time slice through the parameter time_increment,

https://developers.facebook.com/docs/marketing-api/insights/parameters/v12.0

There is currently no option for the user to set this parameter. Being able to set it to monthly, all_days or an integer for the source would solve the problem.

Describe the alternative you’ve considered or used

I have not found a workaround to correctly determine ad reach over a period of time.

Are you willing to submit a PR?

Not immediately

@jd-sanders jd-sanders added the type/enhancement New feature or request label Sep 17, 2021
@sherifnada
Copy link
Contributor

@jd-sanders could you clarify one point you made:

the field reach doesn't sum correctly over days

Is this an API level limitation or an issue with the connector itself? How is this issue manifesting?

@sherifnada sherifnada added the area/connectors Connector related issues label Sep 20, 2021
@darian-heede
Copy link
Contributor

@jd-sanders could you clarify one point you made:

the field reach doesn't sum correctly over days

Is this an API level limitation or an issue with the connector itself? How is this issue manifesting?

@sherifnada this is more of a topic about the metric itself:
The reach (sometimes more appropriately referred to as unique users) is a metric that loses it's meaning if it's aggregated (summed). Take the example of the difference between daily and monthly unique users: a user might be unique for a specific day, but he would also be considered unique for the following day and all other days of a given month. By summing up the daily unique users for a month, you would add the same user multiple times, making the aggregated users not unique anymore. In the monthly unique users, the same user would only appear once.

This implies that when analyzing reach or unique users you cannot aggregate the data without losing the meaning in the data. It is therefore important to be able to specify the time increment for which you would like to fetch the data, as the uniqueness of a user is only available to the data provider (FB in this case).

From my experience, knowing the number of unique users on a monthly base is essential for reporting purposes.

@jd-sanders
Copy link
Author

Thanks @darian-heede . That was a good explanation and is exactly my issue.

@zestyping
Copy link
Contributor

zestyping commented Oct 20, 2021

It occurred to me that this has potential implications for the Airbyte representation of streams, because this is a parameter at the stream level. It would be reasonable to want both daily reach and weekly reach, for example, so you'd need ads_insights with time_increment=1 and ads_insights with time_increment=7 — two instances of the same stream in the same connection, which the UI doesn't support at the moment.

I could imagine hacking around this by adding the parameter to the Facebook Marketing source definition, and then defining two separate connections, one with time_increment=1 and one with time_increment=7. That would work but it's not really the right abstraction, because time_increment only makes sense for some streams and not others.

@sherifnada
Copy link
Contributor

sherifnada commented Dec 10, 2021

It might be nice to make report configuration completely part of the configuration process of the connector. The idea is: by default, the connector doesn't sync any report streams; they would need to be added as part of the configuration page.

To add a report to the connector, a user clicks the "add" button on the UI and creates a list of reports, inputting their fields/breakdowns/action_breakdowns plus the following parameters:

  • start_date for that report
  • aggregation window (I believe this would just become the time_increment param)
  • report_name (the name of the output stream)

It might be nice to have a list of report presets so the user doesn't have to manually input fields/breakdowns/etc.. for all reports.

I'm not sure exactly if the UI can do this today, but this seems like a much more flexible way of doing this. This way you can combine reports of different aggregation windows, start dates, attribution window, etc...

@jd-sanders @zestyping @darian-heede any thoughts on this UX?

@sherifnada
Copy link
Contributor

probably going to pull this into the next sprint, but marking as blocked for now pending a decision on the right UX

@keu
Copy link
Contributor

keu commented Dec 14, 2021

Recently we add a possibility to add custom Insights, this allows us to configure any number of insight streams.

The plan is complicated by #8385, so it is better to wait until we release it:

  • make time_increment configurable for custom insights (in the same way as we have breakdowns, fields, ...)
  • JobManager.days_per_job should be derived from stream.time_increment value here

8h dev, 2h adding tests

@darian-heede
Copy link
Contributor

@sherifnada I don't really use the UI but your solution sounds good to me. Of course, the end_date should also be a parameter, the user should be able to input.

@Zirochkaa Zirochkaa assigned Zirochkaa and unassigned Zirochkaa Dec 22, 2021
@avida avida self-assigned this Dec 23, 2021
@avida
Copy link
Contributor

avida commented Dec 23, 2021

As @keu said we already have implemented custom insights stream with ability to specify user defined breakdowns and action breakdowns so we only need to pass time_increment as stream parameter.

Here is example how it could look like from UX side. I've added time_incremental field and breakdown/action breakdown enumerations to make it look better:

Adding new custom stream:
Screenshot from 2021-12-23 14-46-24

List of custom streams:
Screenshot from 2021-12-23 14-46-35

Adding extra time_incremental parameter to custom ads insight stream wont take a lot of time but there is concern about data aggregation for incremental stream cause current data update logic not taking into account aggregation window and consider it to be 1 day making time_incremental parameter useless.
For example, today is Jan 10. User wants weekly metrics to be gathered starting from the Jan 1. He set time_incremental parameter to 7, sync period to 1 day and start incremental sync job. For first run connector would return metrics for ads with start_date equal to Jan 1 and end_date equal to Jan 7 (first week). Next portion of data would be jan 8 - jan 10 (remaining period). On every next run it would return data for period of one day (data sync period) so we would get data with time ranges of 7, 3, 1, 1, 1... and so on days but obviously user wanted to see data with same 7 days ranges.

To meet user expectations I purpose to change incremental stream logic in this way:

  1. First of all we need to adjust days_per_job parameter to make it multiple of time_incremental value (for simplicity sake lets not take into account possible "monthly" and "all_days" time_incremental values).
  2. If sync period is not multiple of time_incremental parameter then save last range's end date into stream state and stop the sync.
  3. On next sync attemt load stream state and if new time range is equal to time_increment that perform the sync. Otherwise skip it.

So for case described above first it would get data for Jan1- Jan7 and save Jan7 to stream state and stop the sync. On next 3 syncs (Jan11-Jan13) it would do nothing and on Jan 14 sync it would make request for metrics within Jan7-Jan14 range.

@keu @sherifnada does it make sense? Should we make this changes i.e. skiping sync if there is not enough data to perform weekly, monthly and whatever period user wants metric collection? Or just change time_incremental parameter and thats it?

@misteryeo
Copy link
Contributor

@VasylLazebnyk can you make sure to re-assign this ticket to an engineer so that it can be reviewed and closed?

@keu
Copy link
Contributor

keu commented Jan 28, 2022

this ticket is blocked by #8282

@keu keu changed the title Source Facebook Marketing: Allow user to set <time_increment> parameter for Insights API Source Facebook Marketing: Allow user to set time_increment parameter for Insights API Feb 22, 2022
@keu keu removed the needs-scoping label Feb 23, 2022
@keu keu closed this as completed in #10531 Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment