Skip to content

Commit

Permalink
Ignore Telemetry from legacy versions in Remote Settings checks (#943)
Browse files Browse the repository at this point in the history
* Ignore Telemetry from legacy versions in error_rate check

* Filter out legacy from max_age and max_duration too
  • Loading branch information
leplatrem authored Jan 19, 2022
1 parent d161533 commit fccfd1b
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 9 deletions.
25 changes: 20 additions & 5 deletions checks/remotesettings/uptake_error_rate.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
from telescope.typings import CheckResult
from telescope.utils import csv_quoted, fetch_bigquery

from .utils import current_firefox_esr


EXPOSED_PARAMETERS = [
"max_error_percentage",
Expand Down Expand Up @@ -45,9 +47,7 @@
AND event_category = 'uptake.remotecontent.result'
AND event_object = 'remotesettings'
AND event_string_value <> 'up_to_date'
AND app_version != '69.0'
AND app_version != '69.0.1' -- 69.0.2, 69.0.3 seem fine.
AND (normalized_channel != 'aurora' OR app_version NOT LIKE '70%')
{version_condition}
{channel_condition}
)
SELECT
Expand All @@ -67,8 +67,16 @@


async def fetch_remotesettings_uptake(
channels: List[str], sources: List[str], period_hours: int
channels: List[str],
sources: List[str],
period_hours: int,
min_version: Optional[tuple],
):
version_condition = (
f"AND SAFE_CAST(SPLIT(app_version, '.')[OFFSET(0)] AS INTEGER) >= {min_version[0]}"
if min_version
else ""
)
channel_condition = (
f"AND LOWER(normalized_channel) IN ({csv_quoted(channels)})" if channels else ""
)
Expand All @@ -77,6 +85,7 @@ async def fetch_remotesettings_uptake(
EVENTS_TELEMETRY_QUERY.format(
period_hours=period_hours,
source_condition=source_condition,
version_condition=version_condition,
channel_condition=channel_condition,
)
)
Expand Down Expand Up @@ -106,9 +115,15 @@ async def run(
ignore_status: List[str] = [],
ignore_versions: List[int] = [],
period_hours: int = 4,
include_legacy_versions: bool = False,
) -> CheckResult:
min_version = await current_firefox_esr() if not include_legacy_versions else None

rows = await fetch_remotesettings_uptake(
sources=sources, channels=channels, period_hours=period_hours
sources=sources,
channels=channels,
period_hours=period_hours,
min_version=min_version,
)

min_timestamp = min(r["min_timestamp"] for r in rows)
Expand Down
14 changes: 11 additions & 3 deletions checks/remotesettings/uptake_max_age.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
from telescope.typings import CheckResult
from telescope.utils import csv_quoted, fetch_bigquery

from .utils import current_firefox_esr


EVENTS_TELEMETRY_QUERY = r"""
-- This query returns the percentiles for the sync duration and age of data, by source.
Expand All @@ -28,8 +30,7 @@
WHERE
timestamp > TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL {period_hours} HOUR)
{channel_condition}
-- 99% of broadcasts that took more than 10min with Firefox 67
AND SPLIT(app_version, '.')[OFFSET(0)] != '67'
{version_condition}
AND event_category = 'uptake.remotecontent.result'
AND event_object = 'remotesettings'
AND event_string_value = 'success'
Expand Down Expand Up @@ -64,13 +65,20 @@ async def run(
max_percentiles: Dict[str, int],
channels: List[str] = ["release"],
period_hours: int = 6,
include_legacy_versions: bool = False,
) -> CheckResult:
version_condition = ""
if not include_legacy_versions:
min_version = await current_firefox_esr()
version_condition = f"AND SAFE_CAST(SPLIT(app_version, '.')[OFFSET(0)] AS INTEGER) >= {min_version[0]}"
channel_condition = (
f"AND LOWER(normalized_channel) IN ({csv_quoted(channels)})" if channels else ""
)
rows = await fetch_bigquery(
EVENTS_TELEMETRY_QUERY.format(
period_hours=period_hours, channel_condition=channel_condition
period_hours=period_hours,
channel_condition=channel_condition,
version_condition=version_condition,
)
)

Expand Down
9 changes: 9 additions & 0 deletions checks/remotesettings/uptake_max_duration.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
from telescope.typings import CheckResult
from telescope.utils import csv_quoted, fetch_bigquery

from .utils import current_firefox_esr


EVENTS_TELEMETRY_QUERY = r"""
-- This query returns the percentiles for the sync duration, by source.
Expand All @@ -29,6 +31,7 @@
WHERE
timestamp > TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL {period_hours} HOUR)
{channel_condition}
{version_condition}
),
filtered_telemetry AS (
SELECT
Expand Down Expand Up @@ -61,14 +64,20 @@ async def run(
source: str = "settings-sync",
channels: List[str] = ["release"],
period_hours: int = 6,
include_legacy_versions: bool = False,
) -> CheckResult:
version_condition = ""
if not include_legacy_versions:
min_version = await current_firefox_esr()
version_condition = f"AND SAFE_CAST(SPLIT(app_version, '.')[OFFSET(0)] AS INTEGER) >= {min_version[0]}"
channel_condition = (
f"AND LOWER(normalized_channel) IN ({csv_quoted(channels)})" if channels else ""
)
rows = await fetch_bigquery(
EVENTS_TELEMETRY_QUERY.format(
source=source,
channel_condition=channel_condition,
version_condition=version_condition,
period_hours=period_hours,
)
)
Expand Down
12 changes: 11 additions & 1 deletion checks/remotesettings/utils.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import asyncio
import copy
import random
import re
from typing import Dict, List, Optional, Tuple

import backoff
import kinto_http
import requests
from kinto_http.session import USER_AGENT as KINTO_USER_AGENT

from telescope import config
from telescope import config, utils


USER_AGENT = f"telescope {KINTO_USER_AGENT}"
Expand Down Expand Up @@ -228,3 +229,12 @@ def ellipse(line):
f"{len(extras)} record{'s' if len(extras) > 1 else ''} present in {right} but missing in {left} ({ellipse(extras)})"
)
return ", ".join(details)


async def current_firefox_esr():
resp = await utils.fetch_json(
"https://product-details.mozilla.org/1.0/firefox_versions.json"
)
version = resp["FIREFOX_ESR"]
# "91.0.1esr" -> (91, 0, 1)
return tuple(int(re.sub(r"[^\d]+", "", n)) for n in version.split("."))
30 changes: 30 additions & 0 deletions tests/checks/remotesettings/test_uptake_error_rate.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from datetime import datetime
from unittest import mock

import pytest

Expand Down Expand Up @@ -241,6 +242,35 @@ async def test_min_total_events():
}


async def test_filter_on_legacy_versions_by_default(mock_aioresponses):
url_versions = "https://product-details.mozilla.org/1.0/firefox_versions.json"
mock_aioresponses.get(
url_versions,
payload={
"FIREFOX_DEVEDITION": "97.0b4",
"FIREFOX_ESR": "91.5.0esr",
"FIREFOX_NIGHTLY": "98.0a1",
},
)
with patch_async(
f"{MODULE}.fetch_remotesettings_uptake", return_value=FAKE_ROWS
) as mocked:
await run(max_error_percentage=0.1)
assert mocked.call_args_list == [
mock.call(sources=[], channels=[], period_hours=4, min_version=(91, 5, 0))
]


async def test_include_legacy_versions(mock_aioresponses):
with patch_async(
f"{MODULE}.fetch_remotesettings_uptake", return_value=FAKE_ROWS
) as mocked:
await run(max_error_percentage=0.1, include_legacy_versions=True)
assert mocked.call_args_list == [
mock.call(sources=[], channels=[], period_hours=4, min_version=None)
]


async def test_filter_sources():
fake_rows = [
{
Expand Down

0 comments on commit fccfd1b

Please sign in to comment.