Skip to content

Commit

Permalink
Glean upgrade (#1911)
Browse files Browse the repository at this point in the history
Fixes #1599, #1930 

* Adds new mvpn_debug flag
* Enables glean for iOS with fix for glean crash in debug mode QT_LIBRARY_SUFFIX="" (#1599)
* replaces all QT_DEBUG checks with MVPN_DEBUG
* removes QDebug imports in src code
* use requirements.txt everywhere (#1930)
* embed glean v0.23 (this PR has actually done multiple upgrades, I'll try and keep this comment accurate)
* initializes glean only after telemetry policy window has been shown
* ensures Glean.setUploadEnabled isn't called till after glean has been initialized
* sets Glean channels to distinguish between `staging` and `production` pings
* Does not gate glean calls, lets Glean handle its own business
* Uses Glean.shutdown method on quit
* Removes custom uploader and lets glean handle retries
  • Loading branch information
birdsarah authored Oct 18, 2021
1 parent be68bf0 commit fca8834
Show file tree
Hide file tree
Showing 70 changed files with 306 additions and 415 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/android.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ jobs:
- name: Setup Glean Dependency
shell: bash
run: |
pip3 install "glean_parser==3.5"
pip3 install pyyaml
pip3 install -r requirements.txt
- name: Compilation
run: |
Expand Down
5 changes: 2 additions & 3 deletions .github/workflows/functional_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ jobs:
run: |
export PATH=/opt/qt515/bin:$PATH
pip3 install glean_parser
pip3 install pyyaml
pip3 install -r requirements.txt
git submodule update --remote --depth 1 i18n
python3 scripts/importLanguages.py
Expand Down Expand Up @@ -120,7 +119,7 @@ jobs:
run: |
sudo dpkg -i build/archive/*.deb
sudo apt install --no-upgrade firefox xvfb -y
pip3 install flask
pip3 install -r requirements.txt
npm install dotenv
npm install selenium-webdriver
npm install mocha
Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/linters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,11 @@ jobs:

- name: Install Python dependencies
run: |
pip install lxml
pip3 install -r requirements.txt
- name: Generating glean samples
shell: bash
run: |
pip3 install glean_parser
pip3 install pyyaml
python3 scripts/generate_glean.py
- name: Importing translation files
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/linux.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ jobs:
shell: bash
run: |
sudo apt-get install golang debhelper -y
pip3 install "glean_parser==3.5"
pip3 install pyyaml
pip3 install -r requirements.txt
- name: Build source bundle
shell: bash
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/macos-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ jobs:
- name: Generating glean samples
shell: bash
run: |
pip3 install "glean_parser==3.5"
pip3 install pyyaml
pip3 install -r requirements.txt
python3 scripts/generate_glean.py
- name: Importing translation files
Expand Down Expand Up @@ -193,8 +192,7 @@ jobs:
- name: Generating glean samples
shell: bash
run: |
pip3 install "glean_parser==3.5"
pip3 install pyyaml
pip3 install -r requirements.txt
python3 scripts/generate_glean.py
- name: Importing translation files
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/screencapture.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ jobs:
- name: Generating glean samples
shell: bash
run: |
pip3 install glean_parser
pip3 install pyyaml
pip3 install -r requirements.txt
python3 scripts/generate_glean.py
- name: Importing translation files
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/test_coverage.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ jobs:
- name: Generating glean samples
shell: bash
run: |
pip3 install glean_parser
pip3 install pyyaml
pip3 install -r requirements.txt
python3 scripts/generate_glean.py
- name: Importing translation files
Expand Down
7 changes: 3 additions & 4 deletions .github/workflows/wasm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@ jobs:
python3 -m pip install aqtinstall
python3 -m aqt install --outputdir /opt 5.15.0 linux desktop wasm_32 -m qtcharts
- name: Install glean depedencies
- name: Install python dependencies
shell: bash
run: |
pip3 install "glean_parser==3.5"
pip3 install pyyaml
pip install -r requirements.txt
- name: Setup emsdk
uses: mymindstorm/setup-emsdk@v7
with:
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/windows-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ jobs:
- name: Install glean depedencies
shell: bash
run: |
pip3 install "glean_parser==3.5"
pip3 install pyyaml
pip3 install -r requirements.txt
- name: Adding msbuild to PATH
uses: ilammy/msvc-dev-cmd@v1
Expand Down
30 changes: 19 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,9 @@ following dependencies:
- wireguard-tools >=1.0.20200513
- resolvconf >= 1.82
- golang >= 1.13
- python >= 3.6

Python3 (pip) depedencies:

- glean_parser==3.5
- pyyaml
For Python depedencies see requirements.txt

#### QT5

Expand Down Expand Up @@ -152,8 +150,7 @@ This step needs to be updated each time XCode updates.
```
4. Install python3 dependencies:
```
$ pip3 install 'glean_parser==3.5'
$ pip3 install pyyaml
$ pip3 install -r requirements.txt --user
```
5. Copy `xcode.xconfig.template` to `xcode.xconfig`
```
Expand Down Expand Up @@ -212,8 +209,7 @@ Once Qt has been installed, the IOS procedure is similar to the macOS one:

3. Install python3 dependencies:
```
$ pip3 install 'glean_parser==3.5'
$ pip3 install pyyaml
$ pip3 install -r requirements.txt --user
```

4. Copy `xcode.xconfig.template` to `xcode.xconfig`
Expand Down Expand Up @@ -259,8 +255,7 @@ Add the Adjust SDK token with `-a | --adjust <adjust_token>`

5. Install python3 dependencies:
```
$ pip3 install 'glean_parser==3.5'
$ pip3 install pyyaml
$ pip3 install -r requirements.txt --user
```

6. Build the apk
Expand All @@ -286,7 +281,7 @@ The dependencies are:
2. nasm: https://www.nasm.us/
3. python3: https://www.python.org/downloads/windows/
4. visual studio 2019: https://visualstudio.microsoft.com/vs/
5. Install python3 dependencies (pip install "glean_parser==3.5" pyyaml)
5. Install python3 dependencies (pip3 install -r requirements.txt --user)

Openssl can be obtained from here: https://www.openssl.org/source/
Qt5.15 can be obtained from: https://download.qt.io/archive/qt/5.15/5.15.1/single/qt-everywhere-src-5.15.1.tar.xz
Expand All @@ -306,6 +301,19 @@ When running MozillaVPN, go to http://localhost:8766 to view the inspector.

From the inspector, type `help` to see the list of available commands.

## Glean

When the client is built in debug mode, pings will have the applicationId `MozillaVPN-debug`. Additionally, ping contents will be logged to the client logs and will also be sent to the
[glean debug viewer](https://debug-ping-preview.firebaseapp.com/pings/MozillaVPN) (login required) where they are retained for 3 weeks.

More info on debug view in [glean docs](https://mozilla.github.io/glean/book/user/debugging/index.html).

When the client is in staging mode, but not debug mode, pings will have the applicationId `MozillaVPN-staging` which allows for filtering between staging and production pings.

#### A note on glean embedding

Qt only accepts `major.minor` versions for importing. So if, for example, you're embedding glean v0.21.2 then it will still, for Qt's purpose, be v0.21.

## Bug report

Please file bugs here: https://github.com/mozilla-mobile/mozilla-vpn-client/issues
Expand Down
34 changes: 33 additions & 1 deletion glean/org/mozilla/Glean/glean.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,19 @@
// This file is the actual entry point file for Glean.js in QML, that users will interact with.
//
// I was not able to figure out a way to simply use the Webpack generated file to
// be the entry point in Qt, because of the unusual syntax allowed in Qt Javascript.
// be the entry point in Qt, because of the unusual syntax allowed in Qt JavaScript.
// Thus, we compile the Glean.js library normaly into the `glean.lib.js` file and then
// we have this file which interacts opaquely with the Webpack generated one.
//
// **All functions and variables defined here are public.**

.pragma library

.import QtQuick.LocalStorage 2.0 as LocalStorage
.import "glean.lib.js" as Glean

const ErrorType = Glean.Glean.default.ErrorType;

/**
* Initialize Glean. This method should only be called once, subsequent calls will be no-op.
*
Expand Down Expand Up @@ -90,4 +93,33 @@ function setSourceTags(value) {
Glean.Glean.default.setSourceTags(value);
}

/**
* Finishes executing any ongoing tasks and shuts down Glean.
*
* This will attempt to send pending pings before resolving.
*
* @returns A promise which resolves once shutdown is complete.
*/
function shutdown() {
return Glean.Glean.default.shutdown();
}

/**
* Test-only API**
*
* Resets the Glean singleton to its initial state and re-initializes it.
*
* TODO: Only allow this function to be called on test mode (depends on Bug 1682771).
*
* @param applicationId The application ID (will be sanitized during initialization).
* @param uploadEnabled Determines whether telemetry is enabled.
* If disabled, all persisted metrics, events and queued pings (except
* first_run_date) are cleared. Default to `true`.
* @param config Glean configuration options.
* @returns A promise that resolves when the initialization is complete.
*/
function testResetGlean(applicationId, uploadEnabled, config) {
return Glean.Glean.default.testResetGlean(applicationId, uploadEnabled, config);
}

const _private = Glean.Glean.default._private;
2 changes: 1 addition & 1 deletion glean/org/mozilla/Glean/glean.lib.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion glean/org/mozilla/Glean/qmldir
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
module org.mozilla.Glean
Glean 0.15 glean.js
Glean 0.23 glean.js
4 changes: 4 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
flask
glean_parser==4.1.1
lxml
PyYAML
4 changes: 2 additions & 2 deletions scripts/generate_glean.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ def camelize(string):
try:
subprocess.call(["glean_parser", "translate", "glean/metrics.yaml", "glean/pings.yaml",
"-f", "javascript", "-o", "glean/telemetry", "--option", "platform=qt",
"--option", "version=0.15"])
"--option", "version=0.23"])
except:
print("glean_parser failed. Is it installed? Try with:\n\tpip3 install glean_parser");
print("glean_parser failed. Is it installed? Try with:\n\tpip3 install -r requirements.txt --user");
exit(1)

2 changes: 2 additions & 0 deletions scripts/xcode_patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ def setup_target_main(shortVersion, fullVersion, platform, networkExtension, con
else
groupId = configHash['GROUP_ID_IOS']
config.build_settings['GROUP_ID_IOS'] ||= configHash['GROUP_ID_IOS']
# Force xcode to not set QT_LIBRARY_SUFFIX to "_debug", which causes crash
config.build_settings['QT_LIBRARY_SUFFIX'] = ""
end

config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] ||= [
Expand Down
2 changes: 1 addition & 1 deletion src/adjust/adjustfiltering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ QUrlQuery AdjustFiltering::filterParameters(QUrlQuery& parameters,
QStringList& unknownParameters) {
QUrlQuery newParameters;

#ifdef QT_DEBUG
#ifdef MVPN_DEBUG
// We use the binary-search algorithm. The arrays must be alphabetically
// sorted. Let's check this in debug builds.

Expand Down
6 changes: 3 additions & 3 deletions src/closeeventhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,21 @@ void CloseEventHandler::removeItem(QObject* item) {
logger.debug() << "Remove item";
Q_ASSERT(item);

#ifdef QT_DEBUG
#ifdef MVPN_DEBUG
bool found = false;
#endif

for (int i = 0; i < m_layers.length(); ++i) {
if (m_layers.at(i).m_layer == item) {
m_layers.removeAt(i);
#ifdef QT_DEBUG
#ifdef MVPN_DEBUG
found = true;
#endif
break;
}
}

#ifdef QT_DEBUG
#ifdef MVPN_DEBUG
Q_ASSERT(found);
#endif
}
2 changes: 1 addition & 1 deletion src/commandlineparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ int CommandLineParser::parse(int argc, char* argv[]) {

QStringList tokens;
for (int i = 0; i < argc; ++i) {
#ifdef QT_DEBUG
#ifdef MVPN_DEBUG
if (QString(argv[i]).startsWith("-qmljsdebugger")) {
continue;
}
Expand Down
20 changes: 0 additions & 20 deletions src/commands/commandui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,6 @@

#include <QApplication>

#ifdef QT_DEBUG
# include "gleantest.h"
# include <QLoggingCategory>
#endif

namespace {
Logger logger(LOG_MAIN, "CommandUI");
}
Expand Down Expand Up @@ -138,11 +133,6 @@ int CommandUI::run(QStringList& tokens) {
MozillaVPN vpn;
vpn.setStartMinimized(minimizedOption.m_set);

#ifdef QT_DEBUG
// This is a collector of glean HTTP requests to see if we leak something.
GleanTest gleanTest;
#endif

#if defined(MVPN_WINDOWS) || defined(MVPN_LINUX)
// If there is another instance, the execution terminates here.
if (!EventListener::checkOtherInstances()) {
Expand Down Expand Up @@ -379,16 +369,6 @@ int CommandUI::run(QStringList& tokens) {
});
}

#ifdef QT_DEBUG
qmlRegisterSingletonType<MozillaVPN>(
"Mozilla.VPN", 1, 0, "VPNGleanTest",
[](QQmlEngine*, QJSEngine*) -> QObject* {
QObject* obj = GleanTest::instance();
QQmlEngine::setObjectOwnership(obj, QQmlEngine::CppOwnership);
return obj;
});
#endif

qmlRegisterSingletonType<MozillaVPN>(
"Mozilla.VPN", 1, 0, "VPNAuthInApp",
[](QQmlEngine*, QJSEngine*) -> QObject* {
Expand Down
6 changes: 3 additions & 3 deletions src/connectionhealth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ void ConnectionHealth::setStability(ConnectionStability stability) {
if (stability == Unstable) {
MozillaVPN::instance()->silentSwitch();

emit MozillaVPN::instance()->triggerGleanSample(
emit MozillaVPN::instance()->recordGleanEvent(
GleanSample::connectionHealthUnstable);
} else if (stability == NoSignal) {
emit MozillaVPN::instance()->triggerGleanSample(
emit MozillaVPN::instance()->recordGleanEvent(
GleanSample::connectionHealthNoSignal);
}

Expand Down Expand Up @@ -107,7 +107,7 @@ void ConnectionHealth::connectionStateChanged() {
}

void ConnectionHealth::pingSentAndReceived(qint64 msec) {
#ifdef QT_DEBUG
#ifdef MVPN_DEBUG
logger.debug() << "Ping answer received in msec:" << msec;
#else
Q_UNUSED(msec);
Expand Down
2 changes: 1 addition & 1 deletion src/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ CONSTEXPR(uint32_t, captivePortalRequestTimeoutMsec, 10000, 4000, 0)
CONSTEXPR(uint32_t, statusIconAnimationMsec, 200, 200, 0)

// How often glean pings are sent
CONSTEXPR(uint32_t, gleanTimeoutMsec, 1200000, 1000, 0)
CONSTEXPR(uint32_t, gleanTimeoutMsec, 1200000, 4000, 0)

// How often we check the surveys to be executed (no network requests are done
// for this check)
Expand Down
Loading

0 comments on commit fca8834

Please sign in to comment.