Closed Bug 1757216 Opened 4 years ago Closed 4 years ago

Tag pings from "BrowserStack" ISP as from automation

Categories

(Data Platform and Tools :: General, enhancement, P1)

Product:
Data Platform and Tools
Component:
General
Type:
enhancement
Points:
3

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: mdroettboom, Assigned: klukas)

Details

(Whiteboard: [dataplatform])

Attachments

(3 files)

Link to GitHub pull-request: https://github.com/mozilla/bigquery-etl/pull/2919
49 bytes, text/x-github-pull-request
Details | Review
Link to GitHub pull-request: https://github.com/mozilla/bigquery-etl/pull/2928
49 bytes, text/x-github-pull-request
Details | Review
Link to GitHub pull-request: https://github.com/mozilla/bigquery-etl/pull/2936
49 bytes, text/x-github-pull-request
Details | Review

A large portion of pings on Fenix are coming from BrowserStack, a testing automation tool. The ISP for all of these is BrowserStack. We've seen these come and go in large numbers over time, and since they aren't "real" users, we'd prefer to exclude them from top-level KPIs such as DAO.

We should add the "automation" tag to all incoming pings from the ISP BrowserStack both for future data and retroactively to ~ Sep 2021 when we first starting seeing these in large numbers. IIUC from Bug 1657360, that will exclude these pings from the stable tables.

(This bug came out of the feb-2022-fenix-drop incident).

(In reply to Michael Droettboom [:mdroettboom] from comment #0)

We should add the "automation" tag to all incoming pings from the ISP BrowserStack both for future data

The concept of the X-Source-Tags header is that the source emitting pings should add the "automation" value. We have no concept at the moment of injecting source tags as part of pipeline processing.

:mdboom - Do we control the tests coming from BrowserStack? Is it possible to update that configuration to include X-Source-Tags when submitting telemetry?

and retroactively to ~ Sep 2021 when we first starting seeing these in large numbers. IIUC from Bug 1657360, that will exclude these pings from the stable tables.

These pings are already in the stable tables. If we want to rewrite the data, we'd do so by deleting existing rows from the stable tables with this ISP value.

Flags: needinfo?(mdroettboom)

We don't control the tests coming from BrowserStack -- they are presumably from all of BrowserStack's customers. We do have some contacts there so we could ask them to add the tag. Reasonable chance of success on that, but wouldn't solve it in the general case for other BrowserStack-like companies.

Flags: needinfo?(mdroettboom)

This seems like something we can reasonably handle in MessageScrubber, and this is important for KPI stability.

So the task here would be to add a new condition to MessageScrubber to check for "BrowserStack".equals(attributes.get(Attribute.ISP_NAME)) (we should double check in the data that it's name rather than organization that's set to this value) and if so inject "automation" into the "x_source_tags" attribute. We'll need to handle the case where there is no such attribute set (which is almost certainly the case for all of these pings) and also when there is a value; if "automation" is already in there, we shouldn't change the value; otherwise, we'll need to append ", automation" to the value.

Points: --- → 3
Priority: -- → P1
Priority: P1 → P2
Whiteboard: [data-platform-infra-wg]

PR has been merged. By EOW, this should be deployed and we should see the number of clients from BrowserStack drop to zero.

Plot to monitor: https://sql.telemetry.mozilla.org/queries/84895/source#210244

Assignee: nobody → jklukas

I announced this change in the incident channel, and we determined that more discovery is needed before we commit to dropping these pings going forward.

PR to revert the pipeline change: https://github.com/mozilla/gcp-ingestion/pull/2032

The pipeline change has been reverted. Holding on this pending further discussion of the desired way forward.

Hey Jeff,

Instead of marking them as "automation", given the unclear path forward, would it be beneficial to mark them with another source tag? e.g. "auto-browser-stack"?

Or could this be still done in a month or so?

Flags: needinfo?(jklukas)

Instead of marking them as "automation", given the unclear path forward, would it be beneficial to mark them with another source tag? e.g. "auto-browser-stack"?

No, I'm not a fan of the concept of injecting a source tag as part of the pipeline in the first place, since it's corrupting the intended use of having the data producer label what it's producing. Setting "automation" was a reasonable compromise to hook into the existing machinery for exempting automation pings from permanent storage.

It's already very easy to isolate pings from BrowserStack by filtering on metadata.isp.name = 'BrowserStack' so I don't think we get any particular benefit from additionally encoding this as a source tag.

Flags: needinfo?(jklukas)

(In reply to Jeff Klukas [:klukas] (UTC-4) from comment #8)

It's already very easy to isolate pings from BrowserStack by filtering on metadata.isp.name = 'BrowserStack' so I don't think we get any particular benefit from additionally encoding this as a source tag.

Got it, makes sense, thanks for explaining

I'm picking this up again.

The strategy I have in mind is this:

  • Update the logic for glean baseline_clients_daily and baseline_clients_last_seen to include a new isp field that takes the value of metadata.isp.name
  • Backfill these tables for org_mozilla_fenix (Fenix release channel) to populate isp
  • Update the logic for unified_metrics to check for isp = 'BrowserStack' and set app_name to Fenix BrowserStack rather than Fenix in that case, so that we can tell these apart
  • Backfill unified_metrics
  • Update all mobile dashboards to make sure we filter out app_name = 'Fenix BrowserStack'

There's an assumption here that all of our KPI reporting relies on unified_metrics, which I should validate.

There's also an assumption here that we want to make comparison of KPI numbers with/without BrowserStack easy, so we push the filter to the very end of the stack. The tradeoff here is that analyses looking at upstream datasets will need to know to add the filter isp != 'BrowserStack' if they want to match which clients are considered for KPIs.

Priority: P2 → P1

Note: we should create a spinoff bug about what to do at the end of 2022 when we probably can tag these as automation and let them not flow to stable tables at all.

The isp field has been added to the fenix_derived.clients_last_seen_joined_v1 table and backfilled so that field is populated for all release channel pings. That means we're prepped now to be able to incorporate new logic into unified_metrics and backfill that table. Once the backfill is done and we're ready to copy into prod, I'll need to reach out to affected folks to make sure there is awareness about how KPI numbers will change and how to interact with the updated data.

Here is a redash plot showing how BrowserStack can now be broken out in queries on top of fenix_derived.clients_last_seen_joined_v1:

https://sql.telemetry.mozilla.org/queries/85605/source#211993

It shows plots of DAU with BrowserStack broken out, CDOU, and finally the CDOU difference due to BrowserStack being included or not. These plots hopefully give a good sense of what change to expect when we remove BrowserStack clients from the KPI calculation.

I've staged the following PRs for the final steps of getting this integrated into unified_metrics and the KPI dashboard. I'm seeking reviews on those now. I will update here once I have a sense of sign-offs and timeline for pushing the changes to prod. PRs:

Update the logic for unified_metrics to check for isp = 'BrowserStack' and set app_name to Fenix BrowserStack rather than Fenix in that case, so that we can tell these apart

I'd suggest making this generic rather than specific to Fenix, and if isp = 'BrowserStack' then append BrowserStack to the app_name for any product.

Update all mobile dashboards to make sure we filter out app_name = 'Fenix BrowserStack'

If you take the previous suggestion, maybe filter with app_name NOT LIKE '%BrowserStack'.

Moving the backfill into place is awaiting finalization of the decision brief

The decision brief is now approved, so we can move forward with the backfill.

For some more context on what to expect from a backfill, the number of baseline pings from BrowserStack initially jumped up on 2021-09-06 and stayed around 200k to 300k pings per day until 2022-01-29 when it dropped down to 50k or fewer. As of 2022-05-06 it looks like we're consistently below 10k pings per day.

To make the data sufficiently consistent, we'd need to backfill at least from 2021-09-06 to present.

I'd suggest going back at least a year so any current year-over-year analyses aren't affected, and maybe all the way back to 2021-01-01 to make it easier to remember/describe (e.g. BrowserStack excluded starting in 2021).

The query size was less than I feared so I was indeed able to stage all the way back to 2021-01-01 fairly easily. This is now staged in a table. I'm going to do some validation and then hopefully move this into place today and communicate the changes out.

The previous backfill into clients_last_seen_joined_v1 only went back to 2022-01-01, so all earlier dates don't have ISP info. I need to do some additional backfilling of upstream tables.

Running the following:

bqetl query backfill org_mozilla_firefox_derived.baseline_clients_daily_v1 --project_id=moz-fx-data-shared-prod -s 2021-09-01 -e 2021-12-31 -n 0
bqetl query backfill org_mozilla_firefox_derived.baseline_clients_last_seen_v1 --project_id=moz-fx-data-shared-prod -s 2021-08-01 -e 2021-12-31 -n 0 --parallelism=1
bqetl query backfill fenix_derived.clients_last_seen_joined_v1 --project_id=moz-fx-data-shared-prod -s 2021-09-01 -e 2021-12-31 -n 0
bqetl query backfill tmp.klukas_unified_backfill --project_id=moz-fx-data-shared-prod -s 2021-09-01 -e 2021-12-31 -n 0

This backfill is now complete. Due to complexities of needing to process upstream tables, I have only backfilled from 2021-09-01 to present, since early September is when we first started seeing significant numbers of pings from BrowserStack.

The backfill has been copied into place and I will be pushing out information about what to expect for KPIs and what shows up in dashboards.

As far as I can tell right now, there is no change in visualized mobile KPI numbers in Looker right now, such as on the overall KPI dashboard: https://mozilla.cloud.looker.com/dashboards/519

I had assumed that the client_qualifies filter would be applied on all of these, but it looks like it's not consistently applied to mobile views. I don't want to piecemeal this, so I am going to defer to :loines to coordinate getting Looker consistent on showing the updated numbers.

For futher context, we have already merged a change to make client_qualifies ignore app names with "BrowserStack". I think we need to add a filter for client_qualifies = true to a variety of visualizations under the corporate KPIs folder.

Flags: needinfo?(loines)

The changes are now shown in Looker. Closing this.

Flags: needinfo?(loines)
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Component: Pipeline Ingestion → General
Whiteboard: [data-platform-infra-wg] → [dataplatform]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: