diff --git a/.buildconfig.yml b/.buildconfig.yml index e3799ec33e..0ac516e4a7 100644 --- a/.buildconfig.yml +++ b/.buildconfig.yml @@ -1,4 +1,4 @@ -libraryVersion: 30.0.0 +libraryVersion: 30.1.0 groupId: org.mozilla.telemetry projects: glean: diff --git a/.circleci/config.yml b/.circleci/config.yml index 6bb458dce3..7746bdda13 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -232,7 +232,7 @@ jobs: - run: name: Check vendored schema for upstream updates command: | - bin/update-schema.sh latest + bin/update-schema.sh master if ! git diff --exit-code HEAD -- glean-core/preview/tests/glean.1.schema.json; then echo "====================================" echo "Latest schema from upstream changed." @@ -346,10 +346,10 @@ jobs: - run: name: Install mdbook-dtmo command: | - MDBOOK_VERSION=0.5.0 + MDBOOK_VERSION=0.6.1 MDBOOK="mdbook-dtmo-${MDBOOK_VERSION}-x86_64-unknown-linux-gnu.tar.gz" - MDBOOK_SHA256=ce96727a7d066dc69b9148db46b737f45f79bec738446177806e645673ed8a4e - curl -sfSL --retry 5 --retry-delay 10 -O "https://github.com/badboy/mdbook-dtmo/releases/download/${MDBOOK_VERSION}/${MDBOOK}" + MDBOOK_SHA256=775124f302e633db91696ff955509e8567305949c79a76ef51649b9871fdd590 + curl -sfSL --retry 5 -O "https://github.com/badboy/mdbook-dtmo/releases/download/${MDBOOK_VERSION}/${MDBOOK}" echo "${MDBOOK_SHA256} *${MDBOOK}" | shasum -a 256 -c - tar -xvf "${MDBOOK}" # We rename it to mdbook here, so other tools keep working as expected @@ -571,7 +571,7 @@ jobs: rustup target add aarch64-apple-ios x86_64-apple-ios bin/bootstrap.sh # See https://circleci.com/docs/2.0/testing-ios/#pre-starting-the-simulator - xcrun instruments -w "iPhone 11 (13.4) [" || true + xcrun instruments -w "iPhone 11 (13" || true # Store build type for use in cache key if [ -z "${CIRCLE_TAG}" ]; then echo "release" > buildtype.txt @@ -663,7 +663,7 @@ jobs: command: | rustup target add aarch64-apple-ios x86_64-apple-ios # See https://circleci.com/docs/2.0/testing-ios/#pre-starting-the-simulator - xcrun instruments -w "iPhone 11 (13.4) [" || true + xcrun instruments -w "iPhone 11 (13" || true - attach_workspace: at: . - run: @@ -683,12 +683,15 @@ jobs: sed -i.bak "/mozilla\/glean/s#.*#binary \"$JSON_URL\" ~> 0.0.1-SNAPSHOT#" "$CARTFILE_PATH" cat "${CARTFILE_PATH}" - run: - name: Build sample app + name: Bootstrap dependencies command: | - # Build in Debug mode to speed it all up + # We need release mode to correctly ship bitcode for dependencies pushd samples/ios/app - carthage bootstrap --platform iOS --cache-builds --configuration Debug --verbose + carthage bootstrap --platform iOS --cache-builds --configuration Release --verbose popd + - run: + name: Build sample app + command: | bash bin/run-ios-sample-app-build.sh - store_artifacts: path: raw_sample_xcodebuild.log diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000000..c6ce05be34 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,2 @@ +[run] +source = glean \ No newline at end of file diff --git a/.dictionary b/.dictionary index f675227462..4ce0173000 100644 --- a/.dictionary +++ b/.dictionary @@ -1,4 +1,4 @@ -personal_ws-1.1 en 172 utf-8 +personal_ws-1.1 en 173 utf-8 AAR AARs APIs @@ -105,6 +105,7 @@ gfritzsche glinter gradle grcov +gzip hotfix html init diff --git a/.gitignore b/.gitignore index 88c38abbdc..b2536640e6 100644 --- a/.gitignore +++ b/.gitignore @@ -20,9 +20,6 @@ raw_xcode*log /glean-core/ios/Pipfile.lock /glean-core/ios/.venv -# Due to a bug in Carthage, we ignore this file. -# See `tests-only-Cartfile` for more info. -/Cartfile # Carthage build artifacts Cartfile.resolved Carthage diff --git a/CHANGELOG.md b/CHANGELOG.md index 898dfc13f4..afc5967830 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,21 @@ # Unreleased changes -[Full changelog](https://github.com/mozilla/glean/compare/v30.0.0...master) +[Full changelog](https://github.com/mozilla/glean/compare/v30.1.0...master) + +# v30.1.0 (2020-05-22) + +[Full changelog](https://github.com/mozilla/glean/compare/v30.0.0...v30.1.0) + +* Android & iOS + * Ping payloads are now compressed using gzip. +* iOS + * `Glean.initialize` is now a no-op if called from an embedded extension. This means that Glean will only run in the base application process in order to prevent extensions from behaving like separate applications with different client ids from the base application. Applications are responsible for ensuring that extension metrics are only collected within the base application. +* Python + * `lifetime: application` metrics are now cleared after the Glean-owned pings are sent, + after the product starts. + * Glean Python bindings now build in a native Windows environment. + * BUGFIX: `MemoryDistributionMetric` now parses correctly in `metrics.yaml` files. + * BUGFIX: Glean will no longer crash if run as part of another library's coverage testing. # v30.0.0 (2020-05-13) @@ -18,6 +33,13 @@ * iOS: * Refactor the ping uploader to use the new upload mechanism. +# v29.1.1 (2020-05-22) + +[Full changelog](https://github.com/mozilla/glean/compare/v29.1.0...v29.1.1) + +* Android + * BUGFIX: Fix a race condition that leads to a `ConcurrentModificationException`. [Bug 1635865](https://bugzilla.mozilla.org/1635865) + # v29.1.0 (2020-05-11) [Full changelog](https://github.com/mozilla/glean/compare/v29.0.0...v29.1.0) @@ -109,7 +131,7 @@ were unintentionally public, have been made private. * Most Glean work and I/O is now done on its own worker thread. This brings the parallelism Python in line with the other platforms. * The timing distribution, memory distribution, string list, labeled boolean and labeled string metric types are now supported in Python ([#762](https://github.com/mozilla/glean/pull/762), [#763](https://github.com/mozilla/glean/pull/763), [#765](https://github.com/mozilla/glean/pull/765), [#766](https://github.com/mozilla/glean/pull/766)) - + # v25.1.0 (2020-02-26) [Full changelog](https://github.com/mozilla/glean/compare/v25.0.0...v25.1.0) diff --git a/Cargo.lock b/Cargo.lock index 0a5a2538b8..00b14a8e26 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,10 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +[[package]] +name = "adler32" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "android_log-sys" version = "0.1.2" @@ -41,7 +46,7 @@ name = "benchmark" version = "0.1.0" dependencies = [ "criterion 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", - "glean-core 30.0.0", + "glean-core 30.1.0", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -127,6 +132,14 @@ dependencies = [ "unicode-width 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "crc32fast" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "criterion" version = "0.3.1" @@ -276,6 +289,17 @@ dependencies = [ "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "flate2" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", + "crc32fast 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.67 (registry+https://github.com/rust-lang/crates.io-index)", + "miniz_oxide 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "getrandom" version = "0.1.14" @@ -288,13 +312,14 @@ dependencies = [ [[package]] name = "glean-core" -version = "30.0.0" +version = "30.1.0" dependencies = [ "bincode 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "chrono 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)", "ctor 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "ffi-support 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "flate2 1.0.14 (registry+https://github.com/rust-lang/crates.io-index)", "iso8601 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "once_cell 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -308,12 +333,12 @@ dependencies = [ [[package]] name = "glean-ffi" -version = "30.0.0" +version = "30.1.0" dependencies = [ "android_logger 0.8.6 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "ffi-support 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "glean-core 30.0.0", + "glean-core 30.1.0", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "once_cell 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.48 (registry+https://github.com/rust-lang/crates.io-index)", @@ -325,7 +350,7 @@ name = "glean-preview" version = "0.0.5" dependencies = [ "env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", - "glean-core 30.0.0", + "glean-core 30.1.0", "jsonschema-valid 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "once_cell 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -483,6 +508,14 @@ dependencies = [ "rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "miniz_oxide" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "adler32 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "nom" version = "5.1.1" @@ -986,6 +1019,7 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" [metadata] +"checksum adler32 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "5d2e7343e7fc9de883d1b0341e0b13970f764c14101234857d2ddafa1cb1cac2" "checksum android_log-sys 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "b8052e2d8aabbb8d556d6abbcce2a22b9590996c5f849b9c7ce4544a2e3b984e" "checksum android_logger 0.8.6 (registry+https://github.com/rust-lang/crates.io-index)" = "8cbd542dd180566fad88fd2729a53a62a734843c626638006a9d63ec0688484e" "checksum arrayref 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "a4c527152e37cf757a3f78aae5a06fbeefdb07ccc535c980a3208ee3060dd544" @@ -1002,6 +1036,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" "checksum chrono 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)" = "31850b4a4d6bae316f7a09e691c944c28299298837edc0a03f755618c23cbc01" "checksum clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5067f5bb2d80ef5d68b4c87db81601f0b75bca627bc2ef76b141d7b846a3c6d9" +"checksum crc32fast 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ba125de2af0df55319f41944744ad91c71113bf74a4646efff39afe1f6842db1" "checksum criterion 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "1fc755679c12bda8e5523a71e4d654b6bf2e14bd838dfc48cde6559a05caf7d1" "checksum criterion-plot 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a01e15e0ea58e8234f96146b1f91fa9d0e4dd7a38da93ff7a75d42c0b9d3a545" "checksum crossbeam-deque 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)" = "9f02af974daeee82218205558e51ec8768b48cf524bd01d550abe5573a608285" @@ -1016,6 +1051,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum failure 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "f8273f13c977665c5db7eb2b99ae520952fe5ac831ae4cd09d80c4c7042b5ed9" "checksum failure_derive 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "0bc225b78e0391e4b8683440bf2e63c2deeeb2ce5189eab46e2b68c6d3725d08" "checksum ffi-support 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "087be066eb6e85d7150f0c5400018a32802f99d688b2d3868c526f7bbfe17960" +"checksum flate2 1.0.14 (registry+https://github.com/rust-lang/crates.io-index)" = "2cfff41391129e0a856d6d822600b8d71179d46879e310417eb9c762eb178b42" "checksum getrandom 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)" = "7abc8dd8451921606d809ba32e95b6111925cd2906060d2dcc29c070220503eb" "checksum hermit-abi 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "1010591b26bbfe835e9faeabeb11866061cc7dcebffd56ad7d0942d0e61aefd8" "checksum humantime 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "df004cfca50ef23c36850aaaa59ad52cc70d0e90243c3c7737a4dd32dc7a3c4f" @@ -1036,6 +1072,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum maybe-uninit 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "60302e4db3a61da70c0cb7991976248362f30319e88850c487b9b95bbf059e00" "checksum memchr 2.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "3728d817d99e5ac407411fa471ff9800a778d88a24685968b36824eaf4bee400" "checksum memoffset 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)" = "75189eb85871ea5c2e2c15abbdd541185f63b408415e5051f5cac122d8c774b9" +"checksum miniz_oxide 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "aa679ff6578b1cddee93d7e82e263b94a575e0bfced07284eb0c037c1d2416a5" "checksum nom 5.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "0b471253da97532da4b61552249c521e01e736071f71c1a4f7ebbfbf0a06aad6" "checksum num-integer 0.1.42 (registry+https://github.com/rust-lang/crates.io-index)" = "3f6ea62e9d81a77cd3ee9a2a5b9b609447857f3d358704331e4ef39eb247fcba" "checksum num-traits 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)" = "c62be47e61d1842b9170f0fdeec8eba98e60e90e5446449a0545e5152acd7096" diff --git a/Cartfile.private b/Cartfile.private index 4d37985a71..52f3f4d1c7 100644 --- a/Cartfile.private +++ b/Cartfile.private @@ -1 +1,2 @@ github "AliSoftware/OHHTTPStubs" "8.0.0" +github "1024jp/GzipSwift" "5.1.1" diff --git a/Makefile b/Makefile index 030904a8df..cee74ebe91 100644 --- a/Makefile +++ b/Makefile @@ -178,7 +178,7 @@ rust-coverage: ## Generate code coverage information for Rust code .PHONY: rust-coverage python-coverage: build-python ## Generate a code coverage report for Python - $(GLEAN_PYENV)/bin/python3 -m coverage run --parallel-mode -m pytest + GLEAN_COVERAGE=1 $(GLEAN_PYENV)/bin/python3 -m coverage run --parallel-mode -m pytest $(GLEAN_PYENV)/bin/python3 -m coverage combine $(GLEAN_PYENV)/bin/python3 -m coverage html .PHONY: python-coverage diff --git a/bin/build-rust-docs.sh b/bin/build-rust-docs.sh index f149815e63..a6029a5b50 100755 --- a/bin/build-rust-docs.sh +++ b/bin/build-rust-docs.sh @@ -7,7 +7,7 @@ # Build all docs with one command # Documentation will be placed in `build/docs`. -set -e +set -xe CRATE_NAME=glean_core diff --git a/bin/prepare-release.sh b/bin/prepare-release.sh index efa3a51fac..4173d16e1a 100755 --- a/bin/prepare-release.sh +++ b/bin/prepare-release.sh @@ -144,6 +144,11 @@ ${CHANGELOG} EOL fi +### Dependency summary ### + +echo "Updating dependency summary" +run "${WORKSPACE_ROOT}"/bin/dependency-summary.sh + echo "Everything prepared for v${NEW_VERSION}" echo echo "Changed files:" diff --git a/bin/run-ios-sample-app-test.sh b/bin/run-ios-sample-app-test.sh index ca331d219d..c419f510c3 100755 --- a/bin/run-ios-sample-app-test.sh +++ b/bin/run-ios-sample-app-test.sh @@ -9,7 +9,7 @@ set -euvx set -o pipefail && \ xcodebuild \ -workspace ./samples/ios/app/glean-sample-app.xcodeproj/project.xcworkspace \ - -scheme glean-sample-appUITests \ + -scheme glean-sample-app \ -sdk iphonesimulator \ -destination 'platform=iOS Simulator,name=iPhone 11' \ test | \ diff --git a/bin/update-schema.sh b/bin/update-schema.sh index 8caddc2429..fbd5f33da8 100755 --- a/bin/update-schema.sh +++ b/bin/update-schema.sh @@ -16,18 +16,13 @@ # # Environment: # -# DRY_RUN - Do not modify files or run destructive commands when set. # VERB - Log commands that are run when set. set -eo pipefail run() { [ "${VERB:-0}" != 0 ] && echo "+ $*" - if [ "$DOIT" = y ]; then - "$@" - else - true - fi + "$@" } update() { @@ -39,38 +34,15 @@ update() { run curl --silent --fail --show-error --location --retry 5 --retry-delay 10 "$FULL_URL" --output "$SCHEMA_PATH" } -get_latest() { - API_URL="https://api.github.com/repos/mozilla-services/mozilla-pipeline-schemas/commits?path=schemas%2Fglean%2Fglean%2Fglean.1.schema.json&page=1&per_page=1" - SHA="$(curl --silent --fail --show-error --location --retry 5 --retry-delay 10 "$API_URL" | grep --max-count=1 sha)" - - echo "$SHA" | $SED -E -e 's/.+: "([^"]+)".*/\1/' -} - -SED=sed -if command -v gsed >/dev/null; then - SED=gsed -fi - -DOIT=y -if [[ -n "$DRY_RUN" ]]; then - echo "Dry-run. Not modifying files." - DOIT=n -fi - WORKSPACE_ROOT="$( cd "$(dirname "$0")/.." ; pwd -P )" SCHEMA_URL="https://raw.githubusercontent.com/mozilla-services/mozilla-pipeline-schemas/%s/schemas/glean/glean/glean.1.schema.json" if [ -z "$1" ]; then - echo "Usage: $(basename $0) " + echo "Usage: $(basename $0) " echo echo "Update schema version to test" exit 1 fi COMMIT_HASH="$1" - -if [ "$COMMIT_HASH" = "latest" ]; then - COMMIT_HASH="$(get_latest)" -fi - update "$COMMIT_HASH" diff --git a/build.gradle b/build.gradle index d14e15acc1..e063007d84 100644 --- a/build.gradle +++ b/build.gradle @@ -12,7 +12,7 @@ buildscript { // versions below must match the ones in that repository. ext.versions = [ android_components: '15.0.0', - android_gradle_plugin: '3.4.1', + android_gradle_plugin: '3.5.3', android_maven_publish_plugin: '3.6.2', coroutines: '1.3.0', dokka: '0.9.17', diff --git a/deny.toml b/deny.toml index 53cbcf336c..1d93c43f39 100644 --- a/deny.toml +++ b/deny.toml @@ -6,4 +6,5 @@ allow = [ "Apache-2.0", "MIT", "BSD-2-Clause", + "Zlib", ] diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 476c0aa82f..f0ef2e3608 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -46,6 +46,9 @@ - [Developing documentation](dev/docs.md) - [Android bindings](dev/android/index.md) - [Setup Build Environment](dev/android/setup-android-build-environment.md) + - [Android SDK/NDK versions](dev/android/sdk-ndk-versions.md) + - [Development with android-components](dev/android/development-with-android-components.md) + - [Locally-published components in Fenix](dev/android/locally-published-components-in-fenix.md) - [iOS bindings](dev/ios/index.md) - [Setup Build Environment](dev/ios/setup-ios-build-environment.md) - [Debugging Different Versions of Glean](dev/ios/debug-glean-on-ios.md) @@ -71,10 +74,6 @@ - [Debug Pings](dev/core/internal/debug-pings.md) - [Upload mechanism](dev/core/internal/upload.md) - [Implementations](dev/core/internal/implementations.md) - - [Android SDK/NDK versions](dev/core/internal/sdk-ndk-versions.md) - - [Howtos](dev/howtos/index.md) - - [Development with android-components](dev/howtos/development-with-android-components.md) - - [Locally-published components in Fenix](dev/howtos/locally-published-components-in-fenix.md) - [API Documentation](api/index.md) - [Appendix](appendix/index.md) - [Glossary](appendix/glossary.md) diff --git a/docs/dev/howtos/development-with-android-components.md b/docs/dev/android/development-with-android-components.md similarity index 100% rename from docs/dev/howtos/development-with-android-components.md rename to docs/dev/android/development-with-android-components.md diff --git a/docs/dev/howtos/locally-published-components-in-fenix.md b/docs/dev/android/locally-published-components-in-fenix.md similarity index 98% rename from docs/dev/howtos/locally-published-components-in-fenix.md rename to docs/dev/android/locally-published-components-in-fenix.md index 585583f6f9..743144517c 100644 --- a/docs/dev/howtos/locally-published-components-in-fenix.md +++ b/docs/dev/android/locally-published-components-in-fenix.md @@ -81,7 +81,7 @@ You should now be able to build and run Fenix (assuming you could before all thi ## Caveats -1. This assumes you have followed the [android/rust build setup](../android/setup-android-build-environment.md) +1. This assumes you have followed the [Android/Rust build setup](setup-android-build-environment.md) 2. Make sure you're fully up to date in all repositories, unless you know you need to not be. 3. This omits the steps if changes needed because, e.g. Glean made a breaking change to an API used in android-components. These should be understandable to fix, you usually should be able to find a PR with the fixes somewhere in the android-component's list of pending PRs diff --git a/docs/dev/core/internal/sdk-ndk-versions.md b/docs/dev/android/sdk-ndk-versions.md similarity index 83% rename from docs/dev/core/internal/sdk-ndk-versions.md rename to docs/dev/android/sdk-ndk-versions.md index 09e267e5e2..b16e2144a6 100644 --- a/docs/dev/core/internal/sdk-ndk-versions.md +++ b/docs/dev/android/sdk-ndk-versions.md @@ -7,10 +7,10 @@ The Glean SDK implementation is currently build against the following versions: * or install with: `sdkmanager --verbose "platforms;android-28"` * Android build tools 28.0.3 * Download link: -* NDK r20 - * Download link: +* NDK r21 + * Download link: -For the full setup see [Setup the Android Build Environment](../../android/setup-android-build-environment.html). +For the full setup see [Setup the Android Build Environment](setup-android-build-environment.html). The versions are defined in the following files. All locations need to be updated on upgrades: @@ -26,4 +26,4 @@ All locations need to be updated on upgrades: * `ENV ANDROID_BUILD_TOOLS "28.0.3"` * `ENV ANDROID_SDK_VERSION "3859397"` * `ENV ANDROID_PLATFORM_VERSION "28"` - * `ENV ANDROID_NDK_VERSION "r20"` + * `ENV ANDROID_NDK_VERSION "r21"` diff --git a/docs/dev/android/setup-android-build-environment.md b/docs/dev/android/setup-android-build-environment.md index 5cf91836b1..16db7818a7 100644 --- a/docs/dev/android/setup-android-build-environment.md +++ b/docs/dev/android/setup-android-build-environment.md @@ -1,89 +1,103 @@ # Setup the Android Build Environment -## Doing a local build of the Android Components: +## Doing a local build of the Glean SDK: -This document describes how to make local builds of the Android bindings in this -repository. Most consumers of these bindings *do not* need to follow this -process, but will instead [use pre-built -bindings](../../user/adding-glean-to-your-project.html). +This document describes how to make local builds of the Android bindings in this repository. +Most consumers of these bindings *do not* need to follow this process, +but will instead [use pre-built bindings](../../user/adding-glean-to-your-project.html). ## Prepare your build environment Typically, this process only needs to be run once, although periodically you -may need to repeat some steps (e.g., rust updates should be done periodically) +may need to repeat some steps (e.g., Rust updates should be done periodically). ### Setting up Android dependencies -At the end of this process you should have the following environment variables set up. - -- `ANDROID_HOME` -- `ANDROID_NDK_ROOT` -- `JAVA_HOME` +#### With Android Studio The easiest way to install all the dependencies (and automatically handle updates), is by using [Android Studio](https://developer.android.com/studio/index.html). -Once this is installed, it must be run and the Glean project opened to complete initial setup. +Once this is installed, start Android Studio and open the Glean project. If Android Studio asks you to upgrade the version of Gradle, decline. The following dependencies can be installed in Android Studio through `Tools > SDK Manager > SDK Tools`: - Android SDK Tools (may already be selected) -- NDK r20 +- NDK r21 - CMake - LLDB -With the dependencies installed, note down the `Android SDK Location` in `Tools > SDK Manager`. +You should be set to build Glean from within Android Studio now. + +#### Manually + +Set `JAVA_HOME` to be the location of Android Studio's JDK which can be found in Android Studio's "Project Structure" menu (you may need to escape spaces in the path). + +Note down the location of your SDK. +If installed through Android Studio you will find the `Android SDK Location` in `Tools > SDK Manager`. + Set the `ANDROID_HOME` environment variable to that path. -The `ANDROID_NDK_ROOT` can be set to `ANDROID_NDK_ROOT=$ANDROID_HOME/ndk-bundle`. -Set `JAVA_HOME` to be the location of Android Studio's JDK which can be found in Glean's "Project Structure" menu. (You may need to escape spaces in the path). +Alternatively add the following line to the `local.properties` file in the root of the Glean checkout (create the file if it does not exist): + +``` +sdk.dir=/path/to/sdk +``` -If you want to install the NDK manually: +For the Android NDK: -1. Download NDK r20 from . -2. Extract it and put it somewhere (`$HOME/.android-ndk-r20` is a reasonable choice, but it doesn't matter). -3. Set `ANDROID_NDK_ROOT` to this path. - * Set `ANDROID_NDK_HOME` to match `ANDROID_NDK_ROOT`, for compatibility with some android Gradle plugins. +1. Download NDK r21 from . +2. Extract it and put it somewhere (`$HOME/.android-ndk-r21` is a reasonable choice, but it doesn't matter). +3. Add the following line to the `local.properties` file in the root of the Glean checkout (create the file if it does not exist): + ``` + ndk.dir=/path/to/.android-ndk-r21 + ``` ### Setting up Rust Rust can be installed using `rustup`, with the following commands: -- `curl https://sh.rustup.rs -sSf | sh` -- `rustup update` +``` +curl https://sh.rustup.rs -sSf | sh +rustup update +``` -Platform specific toolchains need to be installed for Rust. This can be -done using the `rustup target` command. In order to enable building to real -devices and Android emulators, the following targets need to be installed: +Platform specific toolchains need to be installed for Rust. +This can be done using the `rustup target` command. +In order to enable building for real devices and Android emulators, +the following targets need to be installed: -- `rustup target add aarch64-linux-android` -- `rustup target add armv7-linux-androideabi` -- `rustup target add i686-linux-android` -- `rustup target add x86_64-linux-android` +``` +rustup target add aarch64-linux-android +rustup target add armv7-linux-androideabi +rustup target add i686-linux-android +rustup target add x86_64-linux-android +``` ## Building -This should be relatively straightforward and painless: +Before building: -1. Ensure your repository is up-to-date. +* Ensure your repository is up-to-date. +* Ensure Rust is up-to-date by running `rustup update`. -2. Ensure Rust is up-to-date by running `rustup update`. +### With Android Studio -3. The builds are all performed by `./gradlew` and the general syntax used is - `./gradlew project:task` +After importing the Glean project into Android Studio it should be all set up and you can build the project using `Build > Make Project` - You can see a list of projects by executing `./gradlew projects` and a list - of tasks by executing `./gradlew tasks`. +### Manually -The above can be skipped if using `Android Studio`. The project directory can be imported -and all the build details can be left to the IDE. +The builds are all performed by `./gradlew` and the general syntax used is `./gradlew project:task` + +Build the whole project and run tests: + +``` +./gradlew build +``` + +You can see a list of projects by executing `./gradlew projects` and a list of tasks by executing `./gradlew tasks`. ## FAQ - **Q: Android Studio complains about Python not being found when building.** - A: Make sure that the `python` binary is on your `PATH`. On Windows, in addition to that, it might be required to add its full path to the `rust.pythonCommand` entry in `$project_root/local.properties`. - -- **Q: Android Studio complains about `Toolchain for X does not exist`.** -- A: On Windows, make sure that the `ndk.dir` property in `$project_root/local.properties` points to an existing -directory containing the Android NDK. - diff --git a/docs/dev/core/internal/index.md b/docs/dev/core/internal/index.md index 391a0a81c2..91a2deb54c 100644 --- a/docs/dev/core/internal/index.md +++ b/docs/dev/core/internal/index.md @@ -10,4 +10,3 @@ This includes: * [Directory structure](directory-structure.md) * [Debug Pings](debug-pings.md) * [Implementations](implementations.md) -* [Android SDK/NDK versions](sdk-ndk-versions.md) diff --git a/docs/dev/core/internal/upload.md b/docs/dev/core/internal/upload.md index e1624f607c..cebbd1a575 100644 --- a/docs/dev/core/internal/upload.md +++ b/docs/dev/core/internal/upload.md @@ -62,10 +62,10 @@ For FFI consumers (e.g. Kotlin/Swift/Python implementations) these functions are ```rust /// Gets the next task for an uploader. Which can be either: -extern "C" fn glean_get_upload_task() -> FfiPingUploadTask +extern "C" fn glean_get_upload_task(result: *mut FfiPingUploadTask) /// Processes the response from an attempt to upload a ping. -extern "C" fn glean_process_ping_upload_response(task: FfiPingUploadTask, status: u32) +extern "C" fn glean_process_ping_upload_response(task: *mut FfiPingUploadTask, status: u32) ``` See the documentation for additional information about the types: diff --git a/docs/dev/howtos/index.md b/docs/dev/howtos/index.md deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/docs/user/adding-glean-to-your-project.md b/docs/user/adding-glean-to-your-project.md index 8f859c9b58..8fe2638ea4 100644 --- a/docs/user/adding-glean-to-your-project.md +++ b/docs/user/adding-glean-to-your-project.md @@ -229,6 +229,8 @@ Follow these steps to automatically run the parser at build time: This will ignore files that are generated at build time by the `sdk_generator.sh` script. They don't need to be kept in version control, as they can be re-generated from your `metrics.yaml` and `pings.yaml` files. +> **Important information about Glean and embedded extensions:** Metric collection is a no-op in [application extensions](https://developer.apple.com/library/archive/documentation/General/Conceptual/ExtensibilityPG/ExtensionOverview.html#//apple_ref/doc/uid/TP40014214-CH2-SW2) and Glean will not run. Since extensions run in a separate sandbox and process from the application, Glean would run in an extension as if it were a completely separate application with different client ids and storage. This complicates things because Glean doesn’t know or care about other processes. Because of this, Glean is purposefully prevented from running in an application extension and if metrics need to be collected from extensions, it's up to the integrating application to pass the information to the base application to record in Glean. +
diff --git a/docs/user/general-api.md b/docs/user/general-api.md index 5caf309692..6fc3d7fe41 100644 --- a/docs/user/general-api.md +++ b/docs/user/general-api.md @@ -57,7 +57,7 @@ class SampleApplication : Application() { // Initialize the Glean library. Glean.initialize( - applicationContext, + applicationContext, // Here, `settings()` is a method to get user preferences, specific to // your application and not part of the Glean SDK API. uploadEnabled = settings().isTelemetryEnabled @@ -78,6 +78,25 @@ Library code should never call `Glean.initialize`, since it should be called exa Glean.initialize(applicationContext, Configuration(channel = "beta")) ``` +> **Note**: When the Glean SDK is consumed through Android Components, it is required to configure an HTTP client to be used for upload. +> For example: + +```Kotlin +// Requires `org.mozilla.components:concept-fetch` +import mozilla.components.concept.fetch.Client +// Requires `org.mozilla.components:lib-fetch-httpurlconnection`. +// This can be replaced by other implementations, e.g. `lib-fetch-okhttp` +// or an implementation from `browser-engine-gecko`. +import mozilla.components.lib.fetch.httpurlconnection.HttpURLConnectionClient + +import mozilla.components.service.glean.config.Configuration +import mozilla.components.service.glean.net.ConceptFetchHttpUploader + +val httpClient = ConceptFetchHttpUploader(lazy { HttpURLConnectionClient() as Client }) +val config = Configuration(httpClient = httpClient) +Glean.initialize(context, uploadEnabled = true, configuration = config) +``` +
diff --git a/docs/user/metrics/boolean.md b/docs/user/metrics/boolean.md index c1ed53e62e..69d4a92a2c 100644 --- a/docs/user/metrics/boolean.md +++ b/docs/user/metrics/boolean.md @@ -44,20 +44,20 @@ assertTrue(Flags.a11yEnabled.testGetValue())
```Java -import org.mozilla.yourApplication.GleanMetrics.Flags +import org.mozilla.yourApplication.GleanMetrics.Flags; -Flags.INSTANCE.a11yEnabled.set(System.isAccessibilityEnabled()) +Flags.INSTANCE.a11yEnabled.set(System.isAccessibilityEnabled()); ``` There are test APIs available too: ```Java -import org.mozilla.yourApplication.GleanMetrics.Flags +import org.mozilla.yourApplication.GleanMetrics.Flags; // Was anything recorded? -assertTrue(Flags.INSTANCE.a11yEnabled.testHasValue()) +assertTrue(Flags.INSTANCE.a11yEnabled.testHasValue()); // Does it have the expected value? -assertTrue(Flags.INSTANCE.a11yEnabled.testGetValue()) +assertTrue(Flags.INSTANCE.a11yEnabled.testGetValue()); ```
diff --git a/docs/user/metrics/counter.md b/docs/user/metrics/counter.md index 491372e973..de8566a3d4 100644 --- a/docs/user/metrics/counter.md +++ b/docs/user/metrics/counter.md @@ -50,25 +50,25 @@ assertEquals(
```Java -import org.mozilla.yourApplication.GleanMetrics.Controls +import org.mozilla.yourApplication.GleanMetrics.Controls; -Controls.INSTANCE.refreshPressed.add() // Adds 1 to the counter. -Controls.INSTANCE.refreshPressed.add(5) // Adds 5 to the counter. +Controls.INSTANCE.refreshPressed.add(); // Adds 1 to the counter. +Controls.INSTANCE.refreshPressed.add(5); // Adds 5 to the counter. ``` There are test APIs available too: ```Java -import org.mozilla.yourApplication.GleanMetrics.Controls +import org.mozilla.yourApplication.GleanMetrics.Controls; // Was anything recorded? -assertTrue(Controls.INSTANCE.refreshPressed.testHasValue()) +assertTrue(Controls.INSTANCE.refreshPressed.testHasValue()); // Does the counter have the expected value? -assertEquals(6, Controls.INSTANCE.refreshPressed.testGetValue()) +assertEquals(6, Controls.INSTANCE.refreshPressed.testGetValue()); // Did the counter record an negative value? assertEquals( 1, Controls.INSTANCE.refreshPressed.testGetNumRecordedErrors(ErrorType.InvalidValue) -) +); ```
diff --git a/docs/user/metrics/custom_distribution.md b/docs/user/metrics/custom_distribution.md index 864390ad2b..173bbf343f 100644 --- a/docs/user/metrics/custom_distribution.md +++ b/docs/user/metrics/custom_distribution.md @@ -8,7 +8,7 @@ Otherwise, look at the standard distribution metric types: * [Timing Distributions](timing_distribution.md) * [Memory Distributions](memory_distribution.md) -> Note: Custom distributions are currently only allowed for GeckoView metrics (the `gecko_datapoint` parameter is present). +> **Note**: Custom distributions are currently only allowed for GeckoView metrics (the `gecko_datapoint` parameter is present) and thus have only a Kotlin API. ## Configuration diff --git a/docs/user/metrics/datetime.md b/docs/user/metrics/datetime.md index d72df604e7..b8845c8d59 100644 --- a/docs/user/metrics/datetime.md +++ b/docs/user/metrics/datetime.md @@ -66,25 +66,25 @@ assertEquals(1, Install.firstRun.testGetNumRecordedErrors(ErrorType.InvalidValue
```Java -import org.mozilla.yourApplication.GleanMetrics.Install +import org.mozilla.yourApplication.GleanMetrics.Install; -Install.INSTANCE.firstRun.set() // Records "now" -Install.INSTANCE.firstRun.set(Calendar(2019, 3, 25)) // Records a custom datetime +Install.INSTANCE.firstRun.set(); // Records "now" +Install.INSTANCE.firstRun.set(Calendar(2019, 3, 25)); // Records a custom datetime ``` There are test APIs available too: ```Java -import org.mozilla.yourApplication.GleanMetrics.Install +import org.mozilla.yourApplication.GleanMetrics.Install; // Was anything recorded? -assertTrue(Install.INSTANCE.firstRun.testHasValue()) +assertTrue(Install.INSTANCE.firstRun.testHasValue()); // Was it the expected value? // NOTE: Datetimes always include a timezone offset from UTC, hence the // "-05:00" suffix. -assertEquals("2019-03-25-05:00", Install.INSTANCE.firstRun.testGetValueAsString()) +assertEquals("2019-03-25-05:00", Install.INSTANCE.firstRun.testGetValueAsString()); // Was the value invalid? -assertEquals(1, Install.INSTANCE.firstRun.testGetNumRecordedErrors(ErrorType.InvalidValue)) +assertEquals(1, Install.INSTANCE.firstRun.testGetNumRecordedErrors(ErrorType.InvalidValue)); ```
diff --git a/docs/user/metrics/event.md b/docs/user/metrics/event.md index 79e31365dc..2fba68098a 100644 --- a/docs/user/metrics/event.md +++ b/docs/user/metrics/event.md @@ -119,6 +119,8 @@ assert 0 == metrics.views.login_opened.test_get_num_recorded_errors( * When 500 events are queued on the client an events ping is immediately sent. +* The `extra_keys` allows for a maximum of 10 keys. + * The keys in the `extra_keys` list must be in dotted snake case, with a maximum length of 40 bytes in UTF-8. * The values in the `extras` object have a maximum length of 50 in UTF-8. diff --git a/docs/user/metrics/quantity.md b/docs/user/metrics/quantity.md index d7cdec6027..0e59edc99d 100644 --- a/docs/user/metrics/quantity.md +++ b/docs/user/metrics/quantity.md @@ -3,7 +3,7 @@ Used to record a single non-negative integer value. For example, the width of the display in pixels. -> Quantities are only available for metrics that come from Gecko. +> **Note**: Quantities are currently only allowed for GeckoView metrics (the `gecko_datapoint` parameter is present) and thus have only a Kotlin API. ## Configuration diff --git a/docs/user/metrics/string.md b/docs/user/metrics/string.md index d9080629da..3ae5e29e63 100644 --- a/docs/user/metrics/string.md +++ b/docs/user/metrics/string.md @@ -54,12 +54,12 @@ assertEquals(1, SearchDefault.name.testGetNumRecordedErrors(ErrorType.InvalidVal
```Java -import org.mozilla.yourApplication.GleanMetrics.SearchDefault +import org.mozilla.yourApplication.GleanMetrics.SearchDefault; // Record a value into the metric. -SearchDefault.INSTANCE.name.set("duck duck go") +SearchDefault.INSTANCE.name.set("duck duck go"); // If it changed later, you can record the new value: -SearchDefault.INSTANCE.name.set("wikipedia") +SearchDefault.INSTANCE.name.set("wikipedia"); ``` There are test APIs available too: @@ -68,17 +68,17 @@ There are test APIs available too: import org.mozilla.yourApplication.GleanMetrics.SearchDefault // Was anything recorded? -assertTrue(SearchDefault.INSTANCE.name.testHasValue()) +assertTrue(SearchDefault.INSTANCE.name.testHasValue()); // Does the string metric have the expected value? // IMPORTANT: It may have been truncated -- see "Limits" below -assertEquals("wikipedia", SearchDefault.INSTANCE.name.testGetValue()) +assertEquals("wikipedia", SearchDefault.INSTANCE.name.testGetValue()); // Was the string truncated, and an error reported? assertEquals( - 1, + 1, SearchDefault.INSTANCE.name.testGetNumRecordedErrors( ErrorType.InvalidValue ) -) +); ```
diff --git a/docs/user/metrics/timespan.md b/docs/user/metrics/timespan.md index aef360df0f..b268c5a2a9 100644 --- a/docs/user/metrics/timespan.md +++ b/docs/user/metrics/timespan.md @@ -77,20 +77,20 @@ assertEquals(1, Auth.loginTime.testGetNumRecordedErrors(ErrorType.InvalidValue))
```Java -import org.mozilla.yourApplication.GleanMetrics.Auth +import org.mozilla.yourApplication.GleanMetrics.Auth; -fun onShowLogin() { - Auth.INSTANCE.loginTime.start() +void onShowLogin() { + Auth.INSTANCE.loginTime.start(); // ... } -fun onLogin() { - Auth.INSTANCE.loginTime.stop() +void onLogin() { + Auth.INSTANCE.loginTime.stop(); // ... } -fun onLoginCancel() { - Auth.INSTANCE.loginTime.cancel() +void onLoginCancel() { + Auth.INSTANCE.loginTime.cancel(); // ... } ``` @@ -100,19 +100,19 @@ The time reported in the telemetry ping will be timespan recorded during the lif There are test APIs available too: ```Java -import org.mozilla.yourApplication.GleanMetrics.Auth +import org.mozilla.yourApplication.GleanMetrics.Auth; // Was anything recorded? -assertTrue(Auth.INSTANCE.loginTime.testHasValue()) +assertTrue(Auth.INSTANCE.loginTime.testHasValue()); // Does the timer have the expected value -assertTrue(Auth.INSTANCE.loginTime.testGetValue() > 0) +assertTrue(Auth.INSTANCE.loginTime.testGetValue() > 0); // Was the timing recorded incorrectly? assertEquals( 1, Auth.INSTANCE.loginTime.testGetNumRecordedErrors( ErrorType.InvalidValue ) -) +); ```
@@ -236,7 +236,15 @@ HistorySync.setRawNanos(duration) ## Limits -* None. +* Timings are recorded in nanoseconds. + + * On Android, the [`SystemClock.elapsedRealtimeNanos()`](https://developer.android.com/reference/android/os/SystemClock.html#elapsedRealtimeNanos()) function is used, so it is limited by the accuracy and performance of that timer. The time measurement includes time spent in sleep. + + * On iOS, the [`mach_absolute_time`](https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/KernelProgramming/services/services.html) function is used, + so it is limited by the accuracy and performance of that timer. + The time measurement does not include time spent in sleep. + + * On Python 3.7 and later, [`time.monotonic_ns()`](https://docs.python.org/3/library/time.html#time.monotonic_ns) is used. On earlier versions of Python, [`time.monotonics()`](https://docs.python.org/3/library/time.html#time.monotonic) is used, which is not guaranteed to have nanosecond resolution. ## Examples diff --git a/docs/user/metrics/timing_distribution.md b/docs/user/metrics/timing_distribution.md index b6a7a57f19..c9ba5e45c1 100644 --- a/docs/user/metrics/timing_distribution.md +++ b/docs/user/metrics/timing_distribution.md @@ -95,17 +95,17 @@ assertEquals(1, pages.pageLoad.testGetNumRecordedErrors(ErrorType.InvalidValue))
```Java -import mozilla.components.service.glean.GleanTimerId -import org.mozilla.yourApplication.GleanMetrics.Pages +import mozilla.components.service.glean.GleanTimerId; +import org.mozilla.yourApplication.GleanMetrics.Pages; -val timerId : GleanTimerId +GleanTimerId timerId; -fun onPageStart(e: Event) { - timerId = Pages.INSTANCE.pageLoad.start() +void onPageStart(Event e) { + timerId = Pages.INSTANCE.pageLoad.start(); } -fun onPageLoaded(e: Event) { - Pages.INSTANCE.pageLoad.stopAndAccumulate(timerId) +void onPageLoaded(Event e) { + Pages.INSTANCE.pageLoad.stopAndAccumulate(timerId); } ``` @@ -114,19 +114,19 @@ There are test APIs available too. For convenience, properties `sum` and `count Continuing the `pageLoad` example above, at this point the metric should have a `sum == 11` and a `count == 2`: ```Java -import org.mozilla.yourApplication.GleanMetrics.Pages +import org.mozilla.yourApplication.GleanMetrics.Pages; // Was anything recorded? -assertTrue(pages.INSTANCE.pageLoad.testHasValue()) +assertTrue(pages.INSTANCE.pageLoad.testHasValue()); // Get snapshot. -val snapshot = pages.INSTANCE.pageLoad.testGetValue() +DistributionData snapshot = pages.INSTANCE.pageLoad.testGetValue(); // Does the sum have the expected value? -assertEquals(11, snapshot.getSum) +assertEquals(11, snapshot.getSum); // Usually you don't know the exact timing values, but how many should have been recorded. -assertEquals(2L, snapshot.getCount) +assertEquals(2L, snapshot.getCount); // Was an error recorded? assertEquals( @@ -134,7 +134,7 @@ assertEquals( pages.INSTANCE.pageLoad.testGetNumRecordedErrors( ErrorType.InvalidValue ) -) +); ```
@@ -231,7 +231,11 @@ assert 1 == metrics.pages.page_load.test_get_num_recorded_errors( * Timings are recorded in nanoseconds. - * On Android, the [`SystemClock.elapsedRealtimeNanos()`](https://developer.android.com/reference/android/os/SystemClock.html#elapsedRealtimeNanos()) function is used, so it is limited by the accuracy and performance of that timer. + * On Android, the [`SystemClock.elapsedRealtimeNanos()`](https://developer.android.com/reference/android/os/SystemClock.html#elapsedRealtimeNanos()) function is used, so it is limited by the accuracy and performance of that timer. The time measurement includes time spent in sleep. + + * On iOS, the [`mach_absolute_time`](https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/KernelProgramming/services/services.html) function is used, + so it is limited by the accuracy and performance of that timer. + The time measurement does not include time spent in sleep. * On Python 3.7 and later, [`time.monotonic_ns()`](https://docs.python.org/3/library/time.html#time.monotonic_ns) is used. On earlier versions of Python, [`time.monotonics()`](https://docs.python.org/3/library/time.html#time.monotonic) is used, which is not guaranteed to have nanosecond resolution. diff --git a/docs/user/metrics/uuid.md b/docs/user/metrics/uuid.md index baa3483ad1..1725b3e8a2 100644 --- a/docs/user/metrics/uuid.md +++ b/docs/user/metrics/uuid.md @@ -47,21 +47,21 @@ assertEquals(uuid, User.clientId.testGetValue())
```Java -import org.mozilla.yourApplication.GleanMetrics.User +import org.mozilla.yourApplication.GleanMetrics.User; -User.INSTANCE.clientId.generateAndSet() // Generate a new UUID and record it -User.INSTANCE.clientId.set(UUID.randomUUID()) // Set a UUID explicitly +User.INSTANCE.clientId.generateAndSet(); // Generate a new UUID and record it +User.INSTANCE.clientId.set(UUID.randomUUID()); // Set a UUID explicitly ``` There are test APIs available too: ```Java -import org.mozilla.yourApplication.GleanMetrics.User +import org.mozilla.yourApplication.GleanMetrics.User; // Was anything recorded? -assertTrue(User.INSTANCE.clientId.testHasValue()) +assertTrue(User.INSTANCE.clientId.testHasValue()); // Was it the expected value? -assertEquals(uuid, User.INSTANCE.clientId.testGetValue()) +assertEquals(uuid, User.INSTANCE.clientId.testGetValue()); ```
diff --git a/docs/user/pings/index.md b/docs/user/pings/index.md index 1d127441a4..e6094778d2 100644 --- a/docs/user/pings/index.md +++ b/docs/user/pings/index.md @@ -28,7 +28,6 @@ Optional fields are marked accordingly. | Field name | Type | Description | |---|---|---| -| `ping_type` | String | The name of the ping type (e.g. "baseline", "metrics") | | `seq` | Counter | A running counter of the number of times pings of this type have been sent | | `experiments` | Object | *Optional*. A dictionary of [active experiments](#the-experiments-object) | | `start_time` | Datetime | The time of the start of collection of the data in the ping, in local time and with minute precision, including timezone information. | diff --git a/docs/user/testing-metrics.md b/docs/user/testing-metrics.md index b81dd2f0e2..a9dc3c8e4a 100644 --- a/docs/user/testing-metrics.md +++ b/docs/user/testing-metrics.md @@ -117,7 +117,7 @@ from glean import testing @pytest.fixture(name="reset_glean", scope="function", autouse=True) def fixture_reset_glean(): - testing.reset_glean("my-app-id", "0.1.0") + testing.reset_glean(application_id="my-app-id", application_version="0.1.0") ``` To check if a value exists (i.e. it has been recorded), there is a `test_has_value()` function on each of the metric instances: diff --git a/glean-core/Cargo.toml b/glean-core/Cargo.toml index 2e327a4eab..948b370bfd 100644 --- a/glean-core/Cargo.toml +++ b/glean-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "glean-core" -version = "30.0.0" +version = "30.1.0" authors = ["Jan-Erik Rediger ", "The Glean Team "] description = "A modern Telemetry library" repository = "https://github.com/mozilla/glean" @@ -32,6 +32,7 @@ ffi-support = "0.4.0" regex = { version = "1.3.3", default-features = false, features = ["std"] } chrono = { version = "0.4.10", features = ["serde"] } once_cell = "1.2.0" +flate2 = {version = "1.0.11", default-features = false, features = ["miniz_oxide"] } [dev-dependencies] env_logger = { version = "0.7.1", default-features = false, features = ["termcolor", "atty", "humantime"] } diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt index fb4d58de81..fcb81f8b5b 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt @@ -79,7 +79,7 @@ open class GleanInternalAPI internal constructor () { internal lateinit var metricsPingScheduler: MetricsPingScheduler // Keep track of ping types that have been registered before Glean is initialized. - private val pingTypeQueue: MutableSet = mutableSetOf() + internal val pingTypeQueue: MutableSet = mutableSetOf() // This is used to cache the process state and is used by the function `isMainProcess()` @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @@ -166,7 +166,9 @@ open class GleanInternalAPI internal constructor () { // If any pings were registered before initializing, do so now. // We're not clearing this queue in case Glean is reset by tests. - pingTypeQueue.forEach { registerPingType(it) } + synchronized(this@GleanInternalAPI) { + pingTypeQueue.forEach { registerPingType(it) } + } // If this is the first time ever the Glean SDK runs, make sure to set // some initial core metrics in case we need to generate early pings. @@ -666,13 +668,7 @@ open class GleanInternalAPI internal constructor () { // but not the whole process, meaning globals, such as the ping types, still exist from the old run. // It's a set and keeping them around forever should not have much of an impact. - // This function is called from `Glean.initialize` while iterating over pingTypeQueue. - // Only add if it's not already there to avoid a - // ConcurrentModificationException on Android 5. - // See https://bugzilla.mozilla.org/show_bug.cgi?id=1635865 - if (!pingTypeQueue.contains(pingType)) { - pingTypeQueue.add(pingType) - } + pingTypeQueue.add(pingType) } /** diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/net/BaseUploader.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/net/BaseUploader.kt index 016ffa395b..ca43aff84c 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/net/BaseUploader.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/net/BaseUploader.kt @@ -7,9 +7,9 @@ package mozilla.telemetry.glean.net import android.util.Log import androidx.annotation.VisibleForTesting import mozilla.telemetry.glean.config.Configuration +import mozilla.telemetry.glean.utils.decompressGZIP import org.json.JSONException import org.json.JSONObject -import java.util.Calendar /** * The logic for uploading pings: this leaves the actual upload implementation @@ -74,12 +74,6 @@ class BaseUploader(d: PingUploader) : PingUploader by d { } } - /** - * TEST-ONLY. Allows to set specific dates for testing. - */ - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - internal fun getCalendarInstance(): Calendar { return Calendar.getInstance() } - /** * This function triggers the actual upload: logs the ping and calls the implementation * specific upload function. @@ -91,9 +85,10 @@ class BaseUploader(d: PingUploader) : PingUploader by d { * * @return return the status code of the upload response or null in case unable to upload. */ - internal fun doUpload(path: String, data: String, headers: HeadersList, config: Configuration): UploadResult { + internal fun doUpload(path: String, data: ByteArray, headers: HeadersList, config: Configuration): UploadResult { + val isGzip = !headers.none { (it.first == "Content-Encoding") && (it.second == "gzip") } if (config.logPings) { - logPing(path, data) + logPing(path, if (isGzip) decompressGZIP(data) else data.toString(Charsets.UTF_8)) } return upload( diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/net/HttpURLConnectionUploader.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/net/HttpURLConnectionUploader.kt index 015904653b..d9a0d3b412 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/net/HttpURLConnectionUploader.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/net/HttpURLConnectionUploader.kt @@ -6,6 +6,7 @@ package mozilla.telemetry.glean.net import android.util.Log import androidx.annotation.VisibleForTesting +import java.io.ByteArrayOutputStream import java.io.IOException import java.net.CookieHandler import java.net.CookieManager @@ -37,12 +38,13 @@ class HttpURLConnectionUploader : PingUploader { * @param data the serialized text data to send * @param headers a [HeadersList] containing String to String [Pair] with * the first entry being the header name and the second its value. + * @param isGzipped whether or not the payload is gzipped * * @return return the status code of the upload response, * or null in case unable to upload. */ @Suppress("ReturnCount", "MagicNumber") - override fun upload(url: String, data: String, headers: HeadersList): UploadResult { + override fun upload(url: String, data: ByteArray, headers: HeadersList): UploadResult { var connection: HttpURLConnection? = null try { connection = openConnection(url) @@ -61,7 +63,7 @@ class HttpURLConnectionUploader : PingUploader { removeCookies(url) // Finally upload. - var statusCode = doUpload(connection, data) + val statusCode = doUpload(connection, data) return HttpResponse(statusCode) } catch (e: MalformedURLException) { // There's nothing we can do to recover from this here. So let's just log an error and @@ -104,9 +106,11 @@ class HttpURLConnectionUploader : PingUploader { } @Throws(IOException::class) - internal fun doUpload(connection: HttpURLConnection, data: String): Int { - connection.outputStream.bufferedWriter().use { - it.write(data) + internal fun doUpload(connection: HttpURLConnection, data: ByteArray): Int { + connection.outputStream.use { + val byteOutputStream = ByteArrayOutputStream() + byteOutputStream.write(data) + byteOutputStream.writeTo(it) it.flush() } diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/net/PingUploader.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/net/PingUploader.kt index 1ef54db183..f99fd8c6fd 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/net/PingUploader.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/net/PingUploader.kt @@ -73,5 +73,5 @@ interface PingUploader { * @return return the status code of the upload response, * or null in case upload could not be attempted at all. */ - fun upload(url: String, data: String, headers: HeadersList): UploadResult + fun upload(url: String, data: ByteArray, headers: HeadersList): UploadResult } diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/net/Upload.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/net/Upload.kt index 0f20eec750..e4d5ee4b8f 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/net/Upload.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/net/Upload.kt @@ -24,20 +24,25 @@ enum class UploadTaskTag { Done } -@Structure.FieldOrder("tag", "documentId", "path", "body", "headers") +@Structure.FieldOrder("tag", "documentId", "path", "bodyLen", "body", "headers") internal class UploadBody( // NOTE: We need to provide defaults here, so that JNA can create this object. @JvmField val tag: Byte = UploadTaskTag.Done.ordinal.toByte(), @JvmField val documentId: Pointer? = null, @JvmField val path: Pointer? = null, - @JvmField val body: Pointer? = null, + // Note that the next two fields (`bodyLen` and `body`) are defined as a single + // structure, on the Rust side, called `ByteBuffer`. While the ideal would be to + // use something like `RustBuffer` (as provided by application-services), this + // does not work in the context of JNA unions out of the box. + @JvmField var bodyLen: Long = 0, + @JvmField var body: Pointer? = null, @JvmField val headers: Pointer? = null ) : Structure() { fun toPingRequest(): PingRequest { return PingRequest( this.documentId!!.getRustString(), this.path!!.getRustString(), - this.body!!.getRustString(), + this.body!!.getByteArray(0, this.bodyLen.toInt()), this.headers!!.getRustString() ) } @@ -48,7 +53,7 @@ internal open class FfiPingUploadTask( @JvmField var tag: Byte = UploadTaskTag.Done.ordinal.toByte(), @JvmField var upload: UploadBody = UploadBody() ) : Union() { - class ByValue : FfiPingUploadTask(), Structure.ByValue + class ByReference : FfiPingUploadTask(), Structure.ByReference init { // Initialize to be the `tag`-only variant @@ -73,7 +78,7 @@ internal open class FfiPingUploadTask( internal class PingRequest( private val documentId: String, val path: String, - val body: String, + val body: ByteArray, headers: String ) { val headers: HeadersList = headersFromJSONString(headers) diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt index b236ec78da..91c7842686 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt @@ -556,9 +556,9 @@ internal interface LibGleanFFI : Library { storage_name: String ): Int - fun glean_get_upload_task(): FfiPingUploadTask.ByValue + fun glean_get_upload_task(task: FfiPingUploadTask.ByReference) - fun glean_process_ping_upload_response(task: FfiPingUploadTask.ByValue, status: Int) + fun glean_process_ping_upload_response(task: FfiPingUploadTask.ByReference, status: Int) // Misc diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/PingUploadWorker.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/PingUploadWorker.kt index 8f47716be4..db85552b63 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/PingUploadWorker.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/PingUploadWorker.kt @@ -16,6 +16,7 @@ import androidx.work.Worker import androidx.work.WorkerParameters import mozilla.telemetry.glean.rust.LibGleanFFI import mozilla.telemetry.glean.Glean +import mozilla.telemetry.glean.net.FfiPingUploadTask import mozilla.telemetry.glean.utils.testFlushWorkManagerJob import mozilla.telemetry.glean.net.PingUploadTask @@ -55,11 +56,6 @@ class PingUploadWorker(context: Context, params: WorkerParameters) : Worker(cont @VisibleForTesting internal const val PINGS_DIR = "pending_pings" - // For this error, the ping will be retried later - internal const val RECOVERABLE_ERROR_STATUS_CODE = 500 - // For this error, the ping data will be deleted and no retry happens - internal const val UNRECOVERABLE_ERROR_STATUS_CODE = 400 - /** * Function to aid in properly enqueuing the worker in [WorkManager] * @@ -103,7 +99,10 @@ class PingUploadWorker(context: Context, params: WorkerParameters) : Worker(cont */ override fun doWork(): Result { do { - val incomingTask = LibGleanFFI.INSTANCE.glean_get_upload_task() + // Create a slot of memory for the task: glean-core will write data into + // the allocated memory. + val incomingTask = FfiPingUploadTask.ByReference() + LibGleanFFI.INSTANCE.glean_get_upload_task(incomingTask) when (val action = incomingTask.toPingUploadTask()) { is PingUploadTask.Upload -> { // Upload the ping request. diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/utils/GzipUtils.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/utils/GzipUtils.kt new file mode 100644 index 0000000000..0efc8464c3 --- /dev/null +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/utils/GzipUtils.kt @@ -0,0 +1,19 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.telemetry.glean.utils + +import java.io.BufferedReader +import java.io.ByteArrayInputStream +import java.util.zip.GZIPInputStream + +/** + * Decompress the GZIP returned by the glean-core layer. + * + * @param data the gzipped [ByteArray] to decompress + * @return a [String] containing the uncompressed data. + */ +fun decompressGZIP(data: ByteArray): String { + return GZIPInputStream(ByteArrayInputStream(data)).bufferedReader().use(BufferedReader::readText) +} diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt index ee5008be48..8918b9c309 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt @@ -5,15 +5,17 @@ package mozilla.telemetry.glean import android.content.Context -import android.os.Build import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LifecycleRegistry import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlinx.coroutines.Dispatchers as KotlinDispatchers +import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.launch import kotlinx.coroutines.ObsoleteCoroutinesApi import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout import mozilla.telemetry.glean.GleanMetrics.GleanError import mozilla.telemetry.glean.GleanMetrics.GleanInternalMetrics import mozilla.telemetry.glean.GleanMetrics.Pings @@ -47,7 +49,6 @@ import org.junit.runner.RunWith import org.mockito.Mockito.mock import org.mockito.Mockito.spy import org.robolectric.shadows.ShadowProcess -import org.robolectric.annotation.Config import java.io.BufferedReader import java.io.File import java.io.FileReader @@ -239,7 +240,7 @@ class GleanTest { for (i in 0..3) { val request = server.takeRequest(20L, TimeUnit.SECONDS) val docType = request.path.split("/")[3] - val json = JSONObject(request.body.readUtf8()) + val json = JSONObject(request.getPlainBody()) checkPingSchema(json) if (docType == "events") { assertEquals(1, json.getJSONArray("events").length()) @@ -294,7 +295,7 @@ class GleanTest { val docType = request.path.split("/")[3] assertEquals("The received ping must be a 'baseline' ping", "baseline", docType) - val baselineJson = JSONObject(request.body.readUtf8()) + val baselineJson = JSONObject(request.getPlainBody()) assertEquals("dirty_startup", baselineJson.getJSONObject("ping_info")["reason"]) checkPingSchema(baselineJson) @@ -517,7 +518,7 @@ class GleanTest { val docType = request.path.split("/")[3] assertEquals(pingName, docType) - val pingJson = JSONObject(request.body.readUtf8()) + val pingJson = JSONObject(request.getPlainBody()) checkPingSchema(pingJson) val pingMetricsObject = pingJson.getJSONObject("metrics") @@ -647,7 +648,7 @@ class GleanTest { triggerWorkManager(context) val request = server.takeRequest(20L, TimeUnit.SECONDS) - val jsonContent = JSONObject(request.body.readUtf8()) + val jsonContent = JSONObject(request.getPlainBody()) assertEquals( 110, jsonContent @@ -744,7 +745,7 @@ class GleanTest { val docType = request.path.split("/")[3] assertEquals("The received ping must be a 'baseline' ping", "baseline", docType) - val baselineJson = JSONObject(request.body.readUtf8()) + val baselineJson = JSONObject(request.getPlainBody()) assertEquals("dirty_startup", baselineJson.getJSONObject("ping_info")["reason"]) checkPingSchema(baselineJson) @@ -757,13 +758,79 @@ class GleanTest { } @Test - @Config(sdk = [Build.VERSION_CODES.O]) - fun `Initialize registering pings shouldn't crash with Oreo`() { - // Can't use resetGlean directly + fun `Initializing while registering pings isn't a race condition`() { + // See bug 1635865 + Glean.testDestroyGleanHandle() - val config = Configuration() + // We need a signal for when "initialization is done", and one doesn't + // really exist. For that, this sets a StringMetric on the pre-init task queue and + // then waits for the task queue to be empty. + + Dispatchers.API.setTaskQueueing(true) + Dispatchers.API.setTestingMode(false) + + val stringMetric = StringMetricType( + disabled = false, + category = "telemetry", + lifetime = Lifetime.Application, + name = "string_metric", + sendInPings = listOf("store1") + ) + stringMetric.set("foo") + // Add a bunch of ping types to the pingTypeQueue. We need many in here so + // that registering pings during initialization is slow enough that we can + // get other pings to be registered at the same time from another thread. + + // However, we don't want to add them to the queue and leave them there for + // other tests (that makes the whole testing suite slower), so we first take + // a copy of the current state, and restore it at the end of this test. + + val pingTypeQueueInitialState = HashSet(Glean.pingTypeQueue) + + for (i in 1..1000) { + val ping = PingType( + name = "race-condition-ping$i", + includeClientId = true, + sendIfEmpty = false, + reasonCodes = listOf() + ) + Glean.registerPingType(ping) + } + + // Initialize Glean. This will do most of the Glean initialization in the main + // Glean coroutine in Dispatchers.API. + val config = Configuration() Glean.initialize(context, true, config) + + // From another coroutine, just register pings as fast as we can to simulate the + // ping registration race condition. Do this until any queued tasks in Glean are + // complete (which signals the end of initialization). After that, restore the + // pingTypeQueue state. + runBlocking { + GlobalScope.launch { + val ping = PingType( + name = "race-condition-ping", + includeClientId = true, + sendIfEmpty = false, + reasonCodes = listOf() + ) + + // This timeout will fail and thereby fail the unit test if the + // Glean.initialize coroutine crashes. + withTimeout(500) { + while (Dispatchers.API.taskQueue.size > 0) { + Glean.registerPingType(ping) + } + } + }.join() + } + + // Restore the initial state of the pingTypeQueue + Glean.pingTypeQueue.clear() + for (it in pingTypeQueueInitialState) { + Glean.pingTypeQueue.add(it) + } } } diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt index f0caa70394..65a7c9c85d 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt @@ -22,12 +22,14 @@ import org.mockito.Mockito import mozilla.telemetry.glean.config.Configuration import mozilla.telemetry.glean.scheduler.PingUploadWorker import mozilla.telemetry.glean.private.PingTypeBase +import mozilla.telemetry.glean.utils.decompressGZIP import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.RecordedRequest import org.junit.Assert import org.robolectric.shadows.ShadowLog +import java.io.ByteArrayInputStream import java.util.UUID import java.util.concurrent.ExecutionException @@ -236,3 +238,19 @@ internal fun getMockWebServer(): MockWebServer { }) return server } + +/** + * Convenience method to get the body of a request as a String. + * The UTF8 representation of the request body will be returned. + * If the request body is gzipped, it will be decompressed first. + * + * @return a [String] containing the body of the request. + */ +fun RecordedRequest.getPlainBody(): String { + return if (this.getHeader("Content-Encoding") == "gzip") { + val bodyInBytes = ByteArrayInputStream(this.body.readByteArray()).readBytes() + decompressGZIP(bodyInBytes) + } else { + this.body.readUtf8() + } +} diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/debug/GleanDebugActivityTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/debug/GleanDebugActivityTest.kt index 9868948958..206019e52a 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/debug/GleanDebugActivityTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/debug/GleanDebugActivityTest.kt @@ -41,7 +41,7 @@ private class TestPingTagClient( private val responseUrl: String = Configuration.DEFAULT_TELEMETRY_ENDPOINT, private val debugHeaderValue: String? = null ) : PingUploader { - override fun upload(url: String, data: String, headers: HeadersList): UploadResult { + override fun upload(url: String, data: ByteArray, headers: HeadersList): UploadResult { assertTrue("URL must be redirected for tagged pings", url.startsWith(responseUrl)) assertEquals("Debug headers must match what the ping tag was set to", diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/net/BaseUploaderTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/net/BaseUploaderTest.kt index c0a0964d55..d7f6a57701 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/net/BaseUploaderTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/net/BaseUploaderTest.kt @@ -28,7 +28,7 @@ class BaseUploaderTest { * A stub uploader class that does not upload anything. */ private class TestUploader : PingUploader { - override fun upload(url: String, data: String, headers: HeadersList): UploadResult { + override fun upload(url: String, data: ByteArray, headers: HeadersList): UploadResult { return UnrecoverableFailure } } @@ -38,7 +38,7 @@ class BaseUploaderTest { val uploader = spy(BaseUploader(TestUploader())) val expectedUrl = testDefaultConfig.serverEndpoint + testPath - uploader.doUpload(testPath, testPing, testHeaders, testDefaultConfig) + uploader.doUpload(testPath, testPing.toByteArray(Charsets.UTF_8), testHeaders, testDefaultConfig) verify(uploader).upload(eq(expectedUrl), any(), any()) } diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/net/HttpURLConnectionUploaderTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/net/HttpURLConnectionUploaderTest.kt index 588867bc7d..dd8881d949 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/net/HttpURLConnectionUploaderTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/net/HttpURLConnectionUploaderTest.kt @@ -7,6 +7,7 @@ package mozilla.telemetry.glean.net import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.components.support.test.argumentCaptor import mozilla.telemetry.glean.config.Configuration +import mozilla.telemetry.glean.getPlainBody import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import org.junit.Assert.assertEquals @@ -45,9 +46,9 @@ class HttpURLConnectionUploaderTest { val client = spy(HttpURLConnectionUploader()) doReturn(connection).`when`(client).openConnection(anyString()) - doReturn(200).`when`(client).doUpload(connection, testPing) + doReturn(200).`when`(client).doUpload(connection, testPing.toByteArray(Charsets.UTF_8)) - client.upload(testPath, testPing, emptyList()) + client.upload(testPath, testPing.toByteArray(Charsets.UTF_8), emptyList()) verify(connection).readTimeout = HttpURLConnectionUploader.DEFAULT_READ_TIMEOUT verify(connection).connectTimeout = HttpURLConnectionUploader.DEFAULT_CONNECTION_TIMEOUT @@ -59,14 +60,14 @@ class HttpURLConnectionUploaderTest { val uploader = spy(HttpURLConnectionUploader()) val connection = mock(HttpURLConnection::class.java) doReturn(connection).`when`(uploader).openConnection(anyString()) - doReturn(200).`when`(uploader).doUpload(connection, testPing) + doReturn(200).`when`(uploader).doUpload(connection, testPing.toByteArray(Charsets.UTF_8)) val expectedHeaders = mapOf( "Content-Type" to "application/json; charset=utf-8", "Test-header" to "SomeValue", "OtherHeader" to "Glean/Test 25.0.2" ) - uploader.upload(testPath, testPing, expectedHeaders.toList()) + uploader.upload(testPath, testPing.toByteArray(Charsets.UTF_8), expectedHeaders.toList()) val headerNameCaptor = argumentCaptor() val headerValueCaptor = argumentCaptor() @@ -100,7 +101,10 @@ class HttpURLConnectionUploaderTest { val client = spy(HttpURLConnectionUploader()) doReturn(connection).`when`(client).openConnection(anyString()) - assertEquals(client.upload(testPath, testPing, emptyList()), HttpResponse(200)) + assertEquals( + client.upload(testPath, testPing.toByteArray(Charsets.UTF_8), emptyList()), + HttpResponse(200) + ) verify(connection, times(1)).disconnect() } @@ -116,12 +120,12 @@ class HttpURLConnectionUploaderTest { val client = HttpURLConnectionUploader() val url = testConfig.serverEndpoint + testPath - assertNotNull(client.upload(url, testPing, emptyList())) + assertNotNull(client.upload(url, testPing.toByteArray(Charsets.UTF_8), emptyList())) val request = server.takeRequest() assertEquals(testPath, request.path) assertEquals("POST", request.method) - assertEquals(testPing, request.body.readUtf8()) + assertEquals(testPing, request.getPlainBody()) server.shutdown() } @@ -172,12 +176,12 @@ class HttpURLConnectionUploaderTest { // Trigger the connection. val url = testConfig.serverEndpoint + testPath val client = HttpURLConnectionUploader() - assertNotNull(client.upload(url, testPing, emptyList())) + assertNotNull(client.upload(url, testPing.toByteArray(Charsets.UTF_8), emptyList())) val request = server.takeRequest() assertEquals(testPath, request.path) assertEquals("POST", request.method) - assertEquals(testPing, request.body.readUtf8()) + assertEquals(testPing, request.getPlainBody()) assertTrue(request.headers.values("Cookie").isEmpty()) // Check that we still have a cookie. @@ -191,6 +195,6 @@ class HttpURLConnectionUploaderTest { fun `upload() discards pings on malformed URLs`() { val client = spy(HttpURLConnectionUploader()) doThrow(MalformedURLException()).`when`(client).openConnection(anyString()) - assertEquals(UnrecoverableFailure, client.upload("path", "ping", emptyList())) + assertEquals(UnrecoverableFailure, client.upload("path", "ping".toByteArray(Charsets.UTF_8), emptyList())) } } diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/pings/CustomPingTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/pings/CustomPingTest.kt index f70b584860..791b1f1839 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/pings/CustomPingTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/pings/CustomPingTest.kt @@ -6,6 +6,7 @@ package mozilla.telemetry.glean.scheduler import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.telemetry.glean.Glean +import mozilla.telemetry.glean.getPlainBody import mozilla.telemetry.glean.getContextWithMockedInfo import mozilla.telemetry.glean.getMockWebServer import mozilla.telemetry.glean.private.NoReasonCodes @@ -96,7 +97,7 @@ class CustomPingTest { // Not much data in these pings, // but order should be preserved, so we can check the sequence number. - var pingJson = JSONObject(request.body.readUtf8()) + var pingJson = JSONObject(request.getPlainBody()) var pingInfo = pingJson.getJSONObject("ping_info") assertEquals(0L, pingInfo.tryGetLong("seq")) @@ -105,7 +106,7 @@ class CustomPingTest { docType = request.path.split("/")[3] assertEquals("custom-ping", docType) - pingJson = JSONObject(request.body.readUtf8()) + pingJson = JSONObject(request.getPlainBody()) pingInfo = pingJson.getJSONObject("ping_info") assertEquals(1L, pingInfo.tryGetLong("seq")!!) } diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt index 46e1578d89..66e28993b8 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt @@ -13,14 +13,15 @@ package mozilla.telemetry.glean.private import android.os.SystemClock import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 -import java.lang.NullPointerException -import java.util.concurrent.TimeUnit -import mozilla.telemetry.glean.Dispatchers import mozilla.telemetry.glean.Glean +import mozilla.telemetry.glean.getPlainBody import mozilla.telemetry.glean.checkPingSchema +import mozilla.telemetry.glean.Dispatchers import mozilla.telemetry.glean.getContextWithMockedInfo import mozilla.telemetry.glean.getMockWebServer import mozilla.telemetry.glean.resetGlean +import java.lang.NullPointerException +import java.util.concurrent.TimeUnit import mozilla.telemetry.glean.testing.ErrorType import mozilla.telemetry.glean.testing.GleanTestRule import mozilla.telemetry.glean.triggerWorkManager @@ -273,7 +274,7 @@ class EventMetricTypeTest { assert( request.path.startsWith("/submit/$applicationId/events/") ) - val pingJsonData = request.body.readUtf8() + val pingJsonData = request.getPlainBody() val pingJson = JSONObject(pingJsonData) checkPingSchema(pingJson) assertNotNull(pingJson.opt("events")) @@ -334,7 +335,7 @@ class EventMetricTypeTest { triggerWorkManager(context) var request = server.takeRequest(20L, TimeUnit.SECONDS) - var pingJsonData = request.body.readUtf8() + var pingJsonData = request.getPlainBody() var pingJson = JSONObject(pingJsonData) checkPingSchema(pingJson) assertNotNull(pingJson.opt("events")) @@ -359,7 +360,7 @@ class EventMetricTypeTest { triggerWorkManager(context) request = server.takeRequest(20L, TimeUnit.SECONDS) - pingJsonData = request.body.readUtf8() + pingJsonData = request.getPlainBody() pingJson = JSONObject(pingJsonData) checkPingSchema(pingJson) assertNotNull(pingJson.opt("events")) diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt index 34fc7f73fb..07441ea174 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt @@ -7,10 +7,11 @@ package mozilla.telemetry.glean.private import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.telemetry.glean.Glean -import mozilla.telemetry.glean.getWorkerStatus import mozilla.telemetry.glean.checkPingSchema +import mozilla.telemetry.glean.getPlainBody import mozilla.telemetry.glean.getContextWithMockedInfo import mozilla.telemetry.glean.getMockWebServer +import mozilla.telemetry.glean.getWorkerStatus import mozilla.telemetry.glean.resetGlean import mozilla.telemetry.glean.scheduler.PingUploadWorker import mozilla.telemetry.glean.testing.GleanTestRule @@ -68,7 +69,7 @@ class PingTypeTest { val docType = request.path.split("/")[3] assertEquals("custom", docType) - val pingJson = JSONObject(request.body.readUtf8()) + val pingJson = JSONObject(request.getPlainBody()) assertNotNull(pingJson.getJSONObject("client_info")["client_id"]) checkPingSchema(pingJson) } @@ -109,7 +110,7 @@ class PingTypeTest { val docType = request.path.split("/")[3] assertEquals("custom_ping", docType) - val pingJson = JSONObject(request.body.readUtf8()) + val pingJson = JSONObject(request.getPlainBody()) assertNotNull(pingJson.getJSONObject("client_info")["client_id"]) checkPingSchema(pingJson) } @@ -150,7 +151,7 @@ class PingTypeTest { val docType = request.path.split("/")[3] assertEquals("custom-ping", docType) - val pingJson = JSONObject(request.body.readUtf8()) + val pingJson = JSONObject(request.getPlainBody()) assertNotNull(pingJson.getJSONObject("client_info")["client_id"]) checkPingSchema(pingJson) } @@ -191,7 +192,7 @@ class PingTypeTest { val docType = request.path.split("/")[3] assertEquals("custom", docType) - val pingJson = JSONObject(request.body.readUtf8()) + val pingJson = JSONObject(request.getPlainBody()) assertNull(pingJson.getJSONObject("client_info").opt("client_id")) checkPingSchema(pingJson) } diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt index 7f9b92bfb6..028aa75dbb 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt @@ -11,18 +11,19 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.work.testing.WorkManagerTestInitHelper import mozilla.components.support.test.any import mozilla.components.support.test.eq -import mozilla.telemetry.glean.Dispatchers -import mozilla.telemetry.glean.getContextWithMockedInfo import mozilla.telemetry.glean.Glean import mozilla.telemetry.glean.GleanMetrics.Pings +import mozilla.telemetry.glean.checkPingSchema import mozilla.telemetry.glean.private.Lifetime -import mozilla.telemetry.glean.resetGlean import mozilla.telemetry.glean.private.StringMetricType import mozilla.telemetry.glean.private.TimeUnit -import mozilla.telemetry.glean.checkPingSchema -import mozilla.telemetry.glean.triggerWorkManager import mozilla.telemetry.glean.config.Configuration +import mozilla.telemetry.glean.getContextWithMockedInfo import mozilla.telemetry.glean.getMockWebServer +import mozilla.telemetry.glean.getPlainBody +import mozilla.telemetry.glean.Dispatchers +import mozilla.telemetry.glean.resetGlean +import mozilla.telemetry.glean.triggerWorkManager import mozilla.telemetry.glean.utils.getISOTimeString import mozilla.telemetry.glean.utils.parseISOTimeString import org.json.JSONObject @@ -42,7 +43,6 @@ import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.times import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` import java.util.Calendar import java.util.concurrent.TimeUnit as AndroidTimeUnit @@ -291,7 +291,7 @@ class MetricsPingSchedulerTest { val docType = request.path.split("/")[3] assertEquals("The received ping must be a 'metrics' ping", "metrics", docType) - val metricsJsonData = request.body.readUtf8() + val metricsJsonData = request.getPlainBody() val metricsJson = JSONObject(metricsJsonData) // Validate the received data. @@ -532,7 +532,7 @@ class MetricsPingSchedulerTest { val docType = request.path.split("/")[3] assertEquals("The received ping must be a 'metrics' ping", "metrics", docType) - val metricsJsonData = request.body.readUtf8() + val metricsJsonData = request.getPlainBody() val pingJson = JSONObject(metricsJsonData) assertEquals("The received ping must contain the old version", @@ -709,7 +709,7 @@ class MetricsPingSchedulerTest { val docType = request.path.split("/")[3] assertEquals("The received ping must be a 'metrics' ping", "metrics", docType) - val metricsJsonData = request.body.readUtf8() + val metricsJsonData = request.getPlainBody() assertFalse("The canary metric must not be present in this ping", metricsJsonData.contains("must-not-be-in-the-first-ping")) @@ -788,7 +788,7 @@ class MetricsPingSchedulerTest { val docType = request.path.split("/")[3] assertEquals("The received ping must be a 'metrics' ping", "metrics", docType) - val metricsJsonData = request.body.readUtf8() + val metricsJsonData = request.getPlainBody() assertTrue("The expected metric must be in this ping", metricsJsonData.contains(expectedString)) @@ -875,7 +875,7 @@ class MetricsPingSchedulerTest { // } // // Parse the received ping payload to a JSON object. - // val metricsJsonData = request.body.readUtf8() + // val metricsJsonData = request.getPlainBody() // val metricsJson = JSONObject(metricsJsonData) // // Validate the received data. diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/utils/DateUtilsTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/utils/DateUtilsTest.kt index 29da908067..4662fa2ece 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/utils/DateUtilsTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/utils/DateUtilsTest.kt @@ -3,10 +3,10 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +package mozilla.telemetry.glean.utils + import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.telemetry.glean.private.TimeUnit -import mozilla.telemetry.glean.utils.getISOTimeString -import mozilla.telemetry.glean.utils.parseISOTimeString import org.junit.Test import org.junit.runner.RunWith import org.junit.Assert.assertEquals diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/utils/GzipUtilsTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/utils/GzipUtilsTest.kt new file mode 100644 index 0000000000..46bfc87886 --- /dev/null +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/utils/GzipUtilsTest.kt @@ -0,0 +1,27 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +package mozilla.telemetry.glean.utils + +import org.junit.Test +import org.junit.Assert.assertEquals +import java.io.ByteArrayOutputStream +import java.util.zip.GZIPOutputStream + +class GzipUtilsTest { + private val testPing: String = "{ 'ping': 'test' }" + + @Test + fun `gzip must be correctly decompressed`() { + // Compress the test ping. + val byteOutputStream = ByteArrayOutputStream() + GZIPOutputStream(byteOutputStream).bufferedWriter(Charsets.UTF_8).use { it.write(testPing) } + val compressedTestPing = byteOutputStream.toByteArray() + + // Decompress the result and check if it's valid. + val decompressedPing = decompressGZIP(compressedTestPing) + assertEquals(testPing, decompressedPing) + } +} diff --git a/glean-core/ffi/Cargo.toml b/glean-core/ffi/Cargo.toml index 463cdc2b3e..c40e97b357 100644 --- a/glean-core/ffi/Cargo.toml +++ b/glean-core/ffi/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "glean-ffi" -version = "30.0.0" +version = "30.1.0" authors = ["Jan-Erik Rediger ", "The Glean Team "] description = "FFI layer for Glean, a modern Telemetry library" repository = "https://github.com/mozilla/glean" @@ -35,7 +35,7 @@ once_cell = "1.2.0" [dependencies.glean-core] path = ".." -version = "30.0.0" +version = "30.1.0" [target.'cfg(target_os = "android")'.dependencies] android_logger = { version = "0.8.6", default-features = false } diff --git a/glean-core/ffi/examples/glean_app.c b/glean-core/ffi/examples/glean_app.c index 8bffd2f00f..e0699ea18b 100644 --- a/glean-core/ffi/examples/glean_app.c +++ b/glean-core/ffi/examples/glean_app.c @@ -41,18 +41,21 @@ int main(void) // 1 - "upload", there is a new ping to upload and the task will also include the request data; // 2 - "done", there are no more pings to upload. // - // NOTE: If, there are other ping files inside tmp/glean_data directory + // NOTE: If, there are other ping files inside tmp/glean_data directory // they will also be consumed here by `glean_process_ping_upload_response`. - FfiPingUploadTask task = glean_get_upload_task(); - while (task.tag == 1) { - printf("tag: %d\n", task.tag); - printf("path: %s\n", task.upload.path); - printf("body: %s\n", task.upload.body); - - glean_process_ping_upload_response(task, 200); - task = glean_get_upload_task(); + FfiPingUploadTask task; + glean_get_upload_task(&task); + + while (task.tag != FfiPingUploadTask_Done) { + printf("tag: %d\n", task.tag); + + if (task.tag == FfiPingUploadTask_Upload) { + printf("path: %s\n", task.upload.path); + printf("body length: %lld\n", task.upload.body.len); + + glean_process_ping_upload_response(&task, UPLOAD_RESULT_HTTP_STATUS | 200); + } } - printf("tag: %d\n", task.tag); glean_destroy_counter_metric(metric); diff --git a/glean-core/ffi/glean.h b/glean-core/ffi/glean.h index 142b288c9a..3a94f15f72 100644 --- a/glean-core/ffi/glean.h +++ b/glean-core/ffi/glean.h @@ -86,6 +86,64 @@ typedef const int32_t *RawIntArray; typedef const char *const *RawStringArray; +/** + * ByteBuffer is a struct that represents an array of bytes to be sent over the FFI boundaries. + * There are several cases when you might want to use this, but the primary one for us + * is for returning protobuf-encoded data to Swift and Java. The type is currently rather + * limited (implementing almost no functionality), however in the future it may be + * more expanded. + * + * ## Caveats + * + * Note that the order of the fields is `len` (an i64) then `data` (a `*mut u8`), getting + * this wrong on the other side of the FFI will cause memory corruption and crashes. + * `i64` is used for the length instead of `u64` and `usize` because JNA has interop + * issues with both these types. + * + * ByteBuffer does not implement Drop. This is intentional. Memory passed into it will + * be leaked if it is not explicitly destroyed by calling [`ByteBuffer::destroy`]. This + * is because in the future, we may allow it's use for passing data into Rust code. + * ByteBuffer assuming ownership of the data would make this a problem. + * + * Note that alling `destroy` manually is not typically needed or recommended, + * and instead you should use [`define_bytebuffer_destructor!`]. + * + * ## Layout/fields + * + * This struct's field are not `pub` (mostly so that we can soundly implement `Send`, but also so + * that we can verify rust users are constructing them appropriately), the fields, their types, and + * their order are *very much* a part of the public API of this type. Consumers on the other side + * of the FFI will need to know its layout. + * + * If this were a C struct, it would look like + * + * ```c,no_run + * struct ByteBuffer { + * int64_t len; + * uint8_t *data; // note: nullable + * }; + * ``` + * + * In rust, there are two fields, in this order: `len: i64`, and `data: *mut u8`. + * + * ### Description of fields + * + * `data` is a pointer to an array of `len` bytes. Not that data can be a null pointer and therefore + * should be checked. + * + * The bytes array is allocated on the heap and must be freed on it as well. Critically, if there + * are multiple rust packages using being used in the same application, it *must be freed on the + * same heap that allocated it*, or you will corrupt both heaps. + * + * Typically, this object is managed on the other side of the FFI (on the "FFI consumer"), which + * means you must expose a function to release the resources of `data` which can be done easily + * using the [`define_bytebuffer_destructor!`] macro provided by this crate. + */ +typedef struct { + int64_t len; + uint8_t *data; +} ByteBuffer; + /** * A FFI-compatible representation for the PingUploadTask. * @@ -143,7 +201,7 @@ typedef struct { FfiPingUploadTask_Tag tag; char *document_id; char *path; - char *body; + ByteBuffer body; char *headers; } FfiPingUploadTask_Upload_Body; @@ -278,7 +336,7 @@ char *glean_experiment_test_get_data(FfiStr experiment_id); uint8_t glean_experiment_test_is_active(FfiStr experiment_id); -FfiPingUploadTask glean_get_upload_task(void); +void glean_get_upload_task(FfiPingUploadTask *result); /** * # Safety @@ -475,7 +533,19 @@ uint8_t glean_on_ready_to_submit_pings(void); char *glean_ping_collect(uint64_t ping_type_handle, FfiStr reason); -void glean_process_ping_upload_response(FfiPingUploadTask task, uint32_t status); +/** + * Process and free a `FfiPingUploadTask`. + * + * We need to pass the whole task instead of only the document id, + * so that we can free the strings properly on Drop. + * + * After return the `task` should not be used further by the caller. + * + * # Safety + * + * A valid and non-null upload task object is required for this function. + */ +void glean_process_ping_upload_response(FfiPingUploadTask *task, uint32_t status); void glean_quantity_set(uint64_t metric_id, int64_t value); diff --git a/glean-core/ffi/src/lib.rs b/glean-core/ffi/src/lib.rs index f1b81aec31..d0e855d399 100644 --- a/glean-core/ffi/src/lib.rs +++ b/glean-core/ffi/src/lib.rs @@ -342,23 +342,58 @@ pub extern "C" fn glean_is_first_run() -> u8 { with_glean_value(|glean| glean.is_first_run()) } +// Unfortunately, the way we use CFFI in Python ("out-of-line", "ABI mode") does not +// allow return values to be `union`s, so we need to use an output parameter instead of +// a return value to get the task. The output data will be consumed and freed by the +// `glean_process_ping_upload_response` below. +// +// Arguments: +// +// * `result`: the object the output task will be written to. #[no_mangle] -pub extern "C" fn glean_get_upload_task() -> FfiPingUploadTask { - with_glean_value(|glean| FfiPingUploadTask::from(glean.get_upload_task())) +pub extern "C" fn glean_get_upload_task(result: *mut FfiPingUploadTask) { + with_glean_value(|glean| { + let ffi_task = FfiPingUploadTask::from(glean.get_upload_task()); + unsafe { + std::ptr::write(result, ffi_task); + } + }); } -// We need to pass the whole task instead of only the document id, -// so that we can free the strings properly on Drop. +/// Process and free a `FfiPingUploadTask`. +/// +/// We need to pass the whole task instead of only the document id, +/// so that we can free the strings properly on Drop. +/// +/// After return the `task` should not be used further by the caller. +/// +/// # Safety +/// +/// A valid and non-null upload task object is required for this function. #[no_mangle] -pub extern "C" fn glean_process_ping_upload_response(task: FfiPingUploadTask, status: u32) { +pub unsafe extern "C" fn glean_process_ping_upload_response( + task: *mut FfiPingUploadTask, + status: u32, +) { + // Safety: + // * We null-check the passed task before dereferencing. + // * We replace data behind the pointer with another valid variant. + // * We gracefully handle invalid data in strings. + if task.is_null() { + return; + } + + // Take out task and replace with valid value. + // This value should never be read again on the FFI side, + // but as it controls the memory, we put something valid in place, just in case. + let task = std::ptr::replace(task, FfiPingUploadTask::Done); + with_glean(|glean| { if let FfiPingUploadTask::Upload { document_id, .. } = task { assert!(!document_id.is_null()); - let document_id_str = unsafe { - CStr::from_ptr(document_id) - .to_str() - .map_err(|_| glean_core::Error::utf8_error()) - }?; + let document_id_str = CStr::from_ptr(document_id) + .to_str() + .map_err(|_| glean_core::Error::utf8_error())?; glean.process_ping_upload_response(document_id_str, status.into()); }; Ok(()) diff --git a/glean-core/ffi/src/upload.rs b/glean-core/ffi/src/upload.rs index 527079755c..2640fbbd08 100644 --- a/glean-core/ffi/src/upload.rs +++ b/glean-core/ffi/src/upload.rs @@ -10,7 +10,7 @@ use std::ffi::CString; use std::os::raw::c_char; -use ffi_support::IntoFfi; +use ffi_support::{ByteBuffer, IntoFfi}; use crate::glean_str_free; use glean_core::upload::PingUploadTask; @@ -101,7 +101,7 @@ pub enum FfiPingUploadTask { Upload { document_id: *mut c_char, path: *mut c_char, - body: *mut c_char, + body: ByteBuffer, headers: *mut c_char, }, Wait, @@ -114,17 +114,15 @@ impl From for FfiPingUploadTask { PingUploadTask::Upload(request) => { // Safe unwraps: // 1. CString::new(..) should not fail as we are the ones that created the strings being transformed; - // 2. serde_json::to_string(&request.body) should not fail as request.body is a JsonValue; - // 3. serde_json::to_string(&request.headers) should not fail as request.headers is a HashMap of Strings. + // 2. serde_json::to_string(&request.headers) should not fail as request.headers is a HashMap of Strings. let document_id = CString::new(request.document_id.to_owned()).unwrap(); let path = CString::new(request.path.to_owned()).unwrap(); - let body = CString::new(serde_json::to_string(&request.body).unwrap()).unwrap(); let headers = CString::new(serde_json::to_string(&request.headers).unwrap()).unwrap(); FfiPingUploadTask::Upload { document_id: document_id.into_raw(), path: path.into_raw(), - body: body.into_raw(), + body: ByteBuffer::from_vec(request.body), headers: headers.into_raw(), } } @@ -147,9 +145,14 @@ impl Drop for FfiPingUploadTask { unsafe { glean_str_free(*document_id); glean_str_free(*path); - glean_str_free(*body); glean_str_free(*headers); } + // Unfortunately, we cannot directly call `body.destroy();` as + // we're behind a mutable reference, so we have to manually take the + // ownership and drop. Moreover, `ByteBuffer::new_with_size(0)` + // does not allocate, so we are not leaking memory. + let body = std::mem::replace(body, ByteBuffer::new_with_size(0)); + body.destroy(); } } } diff --git a/glean-core/ios/Glean.xcodeproj/project.pbxproj b/glean-core/ios/Glean.xcodeproj/project.pbxproj index cba332511e..c4801a9dff 100644 --- a/glean-core/ios/Glean.xcodeproj/project.pbxproj +++ b/glean-core/ios/Glean.xcodeproj/project.pbxproj @@ -59,6 +59,7 @@ BFB59A9D2342A24000F40CA8 /* GleanBaseline.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFB59A992342A24000F40CA8 /* GleanBaseline.swift */; }; BFB59A9E2342A24000F40CA8 /* Pings.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFB59A9A2342A24000F40CA8 /* Pings.swift */; }; BFB59A9F2342A24000F40CA8 /* GleanInternalMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFB59A9B2342A24000F40CA8 /* GleanInternalMetrics.swift */; }; + BFCBD6AB246D55CC0032096D /* TestUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFCBD6AA246D55CC0032096D /* TestUtils.swift */; }; BFE1CDC4233B63A70019EE47 /* LabeledMetric.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFE1CDC3233B63A70019EE47 /* LabeledMetric.swift */; }; BFE1CDC6233B6B6D0019EE47 /* LabeledMetricTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFE1CDC5233B6B6D0019EE47 /* LabeledMetricTests.swift */; }; BFE1CDC8233B73B30019EE47 /* Unreachable.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFE1CDC7233B73B30019EE47 /* Unreachable.swift */; }; @@ -113,6 +114,7 @@ BF30FDC3233260B500840607 /* TimespanMetric.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimespanMetric.swift; sourceTree = ""; }; BF30FDC5233260C400840607 /* TimespanMetricTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimespanMetricTests.swift; sourceTree = ""; }; BF30FDC72332640400840607 /* TimeUnit.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimeUnit.swift; sourceTree = ""; }; + BF3A3055246E936E00E4A18E /* Gzip.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Gzip.framework; path = ../../Carthage/Build/iOS/Gzip.framework; sourceTree = ""; }; BF3DE3912243A2F20018E23F /* Glean.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Glean.framework; sourceTree = BUILT_PRODUCTS_DIR; }; BF3DE3942243A2F20018E23F /* Glean.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Glean.h; sourceTree = ""; }; BF3DE3952243A2F20018E23F /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; @@ -139,6 +141,7 @@ BFB59A992342A24000F40CA8 /* GleanBaseline.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GleanBaseline.swift; sourceTree = ""; }; BFB59A9A2342A24000F40CA8 /* Pings.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Pings.swift; sourceTree = ""; }; BFB59A9B2342A24000F40CA8 /* GleanInternalMetrics.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GleanInternalMetrics.swift; sourceTree = ""; }; + BFCBD6AA246D55CC0032096D /* TestUtils.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestUtils.swift; sourceTree = ""; }; BFE1CDC3233B63A70019EE47 /* LabeledMetric.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LabeledMetric.swift; sourceTree = ""; }; BFE1CDC5233B6B6D0019EE47 /* LabeledMetricTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LabeledMetricTests.swift; sourceTree = ""; }; BFE1CDC7233B73B30019EE47 /* Unreachable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Unreachable.swift; sourceTree = ""; }; @@ -316,6 +319,7 @@ 1F70B787232A81A4007395FB /* DispatchersTest.swift */, BF3DE39F2243A2F20018E23F /* GleanTests.swift */, BF3DE3A12243A2F20018E23F /* Info.plist */, + BFCBD6AA246D55CC0032096D /* TestUtils.swift */, ); path = GleanTests; sourceTree = ""; @@ -366,6 +370,7 @@ BF6F2DA6224BF8F000394062 /* Frameworks */ = { isa = PBXGroup; children = ( + BF3A3055246E936E00E4A18E /* Gzip.framework */, BF1BF3F72333787E0036F4CC /* OHHTTPStubs.framework */, BF6F2DA7224BF8F100394062 /* libglean_ffi.a */, ); @@ -410,7 +415,6 @@ buildPhases = ( BF6F2DA5224BF2E000394062 /* Run Script */, BFB59A9723429FC000F40CA8 /* Run Glean SDK generator */, - AC0C2B5E2328090D00A50CC8 /* Copy Carthage Dependencies */, BF3DE38C2243A2F20018E23F /* Headers */, BF3DE38D2243A2F20018E23F /* Sources */, BF3DE38E2243A2F20018E23F /* Frameworks */, @@ -510,29 +514,14 @@ ); inputPaths = ( ../../Carthage/Build/iOS/OHHTTPStubs.framework, + ../../Carthage/Build/iOS/Gzip.framework, ); name = "Copy Carthage Dependencies"; outputFileListPaths = ( ); outputPaths = ( - ); - runOnlyForDeploymentPostprocessing = 0; - shellPath = /bin/sh; - shellScript = "/usr/local/bin/carthage copy-frameworks\n"; - }; - AC0C2B5E2328090D00A50CC8 /* Copy Carthage Dependencies */ = { - isa = PBXShellScriptBuildPhase; - buildActionMask = 2147483647; - files = ( - ); - inputFileListPaths = ( - ); - inputPaths = ( - ); - name = "Copy Carthage Dependencies"; - outputFileListPaths = ( - ); - outputPaths = ( + "$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/OHHTTPStubs.framework", + "$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Gzip.framework", ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/sh; @@ -634,6 +623,7 @@ 1F39E7B3239F0777009B13B3 /* GleanDebugUtilityTests.swift in Sources */, BFAED50A2369752400DF293D /* StringListMetricTests.swift in Sources */, BF890561232BC227003CA2BA /* StringMetricTests.swift in Sources */, + BFCBD6AB246D55CC0032096D /* TestUtils.swift in Sources */, 1F58921223C923C4007D2D80 /* MetricsPingSchedulerTests.swift in Sources */, 1FD4527723395EEB00F4C7E8 /* UuidMetricTests.swift in Sources */, BF80AA5B2399301300A8B172 /* HttpPingUploaderTests.swift in Sources */, diff --git a/glean-core/ios/Glean/Glean.swift b/glean-core/ios/Glean/Glean.swift index f2708639fc..b606cc22ef 100644 --- a/glean-core/ios/Glean/Glean.swift +++ b/glean-core/ios/Glean/Glean.swift @@ -39,6 +39,10 @@ public class Glean { private let logger = Logger(tag: Constants.logTag) + // Cache variable for checking if running in main process. Also used to override for tests in + // order to simulate not running in the main process. DO NOT SET EXCEPT IN TESTS! + var isMainProcess: Bool? + private init() { // intentionally left private, no external user can instantiate a new global object. @@ -50,6 +54,7 @@ public class Glean { self.initialized = false } + // swiftlint:disable function_body_length /// Initialize the Glean SDK. /// /// This should only be initialized once by the application, and not by @@ -67,6 +72,14 @@ public class Glean { /// * configuration: A Glean `Configuration` object with global settings. public func initialize(uploadEnabled: Bool, configuration: Configuration = Configuration()) { + // In certain situations Glean.initialize may be called from a process other than the main + // process such as an embedded extension. In this case we want to just return. + // See https://bugzilla.mozilla.org/show_bug.cgi?id=1625157 for more information. + if !checkIsMainProcess() { + logger.error("Attempted to initialize Glean on a process other than the main process") + return + } + if self.isInitialized() { logger.error("Glean should not be initialized multiple times") return @@ -156,6 +169,8 @@ public class Glean { } } + // swiftlint:enable function_body_length + /// Initialize the core metrics internally managed by Glean (e.g. client id). private func initializeCoreMetrics() { // Set a few more metrics that will be sent as part of every ping. @@ -452,6 +467,31 @@ public class Glean { GleanDebugUtility.handleCustomUrl(url: url) } + /// Returns true if running in the base application process, otherwise returns false + private func checkIsMainProcess() -> Bool { + if isMainProcess == nil { + if let packageType = Bundle.main.object(forInfoDictionaryKey: "CFBundlePackageType") as? String { + // This is the bundle type reported by embedded application extensions and so we test for it to + // make sure that we are not running in an extension. + // + // For more info on XPC services see: + // https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPSystemStartup/Chapters/CreatingXPCServices.html + // + // For more info on CFBundlePackageType see: + // https://developer.apple.com/documentation/bundleresources/information_property_list/cfbundlepackagetype + // and + // https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html#//apple_ref/doc/uid/20001431-111321 + isMainProcess = packageType != "XPC!" + } else { + // The `CFBundlePackageType` doesn't get set in tests so we return true to indicate that we are + // running in the main process. + isMainProcess = true + } + } + + return isMainProcess! + } + /// PUBLIC TEST ONLY FUNCTION. /// /// Returns true if a ping by this name is in the ping registry. diff --git a/glean-core/ios/Glean/GleanFfi.h b/glean-core/ios/Glean/GleanFfi.h index 142b288c9a..3a94f15f72 100644 --- a/glean-core/ios/Glean/GleanFfi.h +++ b/glean-core/ios/Glean/GleanFfi.h @@ -86,6 +86,64 @@ typedef const int32_t *RawIntArray; typedef const char *const *RawStringArray; +/** + * ByteBuffer is a struct that represents an array of bytes to be sent over the FFI boundaries. + * There are several cases when you might want to use this, but the primary one for us + * is for returning protobuf-encoded data to Swift and Java. The type is currently rather + * limited (implementing almost no functionality), however in the future it may be + * more expanded. + * + * ## Caveats + * + * Note that the order of the fields is `len` (an i64) then `data` (a `*mut u8`), getting + * this wrong on the other side of the FFI will cause memory corruption and crashes. + * `i64` is used for the length instead of `u64` and `usize` because JNA has interop + * issues with both these types. + * + * ByteBuffer does not implement Drop. This is intentional. Memory passed into it will + * be leaked if it is not explicitly destroyed by calling [`ByteBuffer::destroy`]. This + * is because in the future, we may allow it's use for passing data into Rust code. + * ByteBuffer assuming ownership of the data would make this a problem. + * + * Note that alling `destroy` manually is not typically needed or recommended, + * and instead you should use [`define_bytebuffer_destructor!`]. + * + * ## Layout/fields + * + * This struct's field are not `pub` (mostly so that we can soundly implement `Send`, but also so + * that we can verify rust users are constructing them appropriately), the fields, their types, and + * their order are *very much* a part of the public API of this type. Consumers on the other side + * of the FFI will need to know its layout. + * + * If this were a C struct, it would look like + * + * ```c,no_run + * struct ByteBuffer { + * int64_t len; + * uint8_t *data; // note: nullable + * }; + * ``` + * + * In rust, there are two fields, in this order: `len: i64`, and `data: *mut u8`. + * + * ### Description of fields + * + * `data` is a pointer to an array of `len` bytes. Not that data can be a null pointer and therefore + * should be checked. + * + * The bytes array is allocated on the heap and must be freed on it as well. Critically, if there + * are multiple rust packages using being used in the same application, it *must be freed on the + * same heap that allocated it*, or you will corrupt both heaps. + * + * Typically, this object is managed on the other side of the FFI (on the "FFI consumer"), which + * means you must expose a function to release the resources of `data` which can be done easily + * using the [`define_bytebuffer_destructor!`] macro provided by this crate. + */ +typedef struct { + int64_t len; + uint8_t *data; +} ByteBuffer; + /** * A FFI-compatible representation for the PingUploadTask. * @@ -143,7 +201,7 @@ typedef struct { FfiPingUploadTask_Tag tag; char *document_id; char *path; - char *body; + ByteBuffer body; char *headers; } FfiPingUploadTask_Upload_Body; @@ -278,7 +336,7 @@ char *glean_experiment_test_get_data(FfiStr experiment_id); uint8_t glean_experiment_test_is_active(FfiStr experiment_id); -FfiPingUploadTask glean_get_upload_task(void); +void glean_get_upload_task(FfiPingUploadTask *result); /** * # Safety @@ -475,7 +533,19 @@ uint8_t glean_on_ready_to_submit_pings(void); char *glean_ping_collect(uint64_t ping_type_handle, FfiStr reason); -void glean_process_ping_upload_response(FfiPingUploadTask task, uint32_t status); +/** + * Process and free a `FfiPingUploadTask`. + * + * We need to pass the whole task instead of only the document id, + * so that we can free the strings properly on Drop. + * + * After return the `task` should not be used further by the caller. + * + * # Safety + * + * A valid and non-null upload task object is required for this function. + */ +void glean_process_ping_upload_response(FfiPingUploadTask *task, uint32_t status); void glean_quantity_set(uint64_t metric_id, int64_t value); diff --git a/glean-core/ios/Glean/Net/HttpPingUploader.swift b/glean-core/ios/Glean/Net/HttpPingUploader.swift index bffaf32db0..e113baaae1 100644 --- a/glean-core/ios/Glean/Net/HttpPingUploader.swift +++ b/glean-core/ios/Glean/Net/HttpPingUploader.swift @@ -52,15 +52,23 @@ public class HttpPingUploader { /// Note that the `X-Client-Type`: `Glean` and `X-Client-Version`: /// headers are added to the HTTP request in addition to the UserAgent. This allows /// us to easily handle pings coming from Glean on the legacy Mozilla pipeline. - func upload(path: String, data: String, headers: [String: String], callback: @escaping (UploadResult) -> Void) { + func upload(path: String, data: Data, headers: [String: String], callback: @escaping (UploadResult) -> Void) { if config.logPings { - logPing(path: path, data: data) + // FIXME(bug 1637914): Logging should happen inside glean-core (Rust). + // For now we don't ship Gzip decompression in the Glean SDK for iOS + // due to difficulties delivering dependencies, so we skip logging them. + if headers["Content-Encoding"] == "gzip" { + logPing(path: path, data: "") + } else { + let payload = String(data: data, encoding: .utf8) ?? "" + logPing(path: path, data: payload) + } } // Build the request and create an async upload operation and launch it through the // Dispatchers if let request = buildRequest(path: path, data: data, headers: headers) { - let uploadOperation = PingUploadOperation(request: request, data: Data(data.utf8), callback: callback) + let uploadOperation = PingUploadOperation(request: request, data: data, callback: callback) Dispatchers.shared.launchConcurrent(operation: uploadOperation) } } @@ -73,7 +81,7 @@ public class HttpPingUploader { /// * callback: A callback to return the success/failure of the upload /// /// - returns: Optional `URLRequest` object with the configured headings set. - func buildRequest(path: String, data: String, headers: [String: String]) -> URLRequest? { + func buildRequest(path: String, data: Data, headers: [String: String]) -> URLRequest? { if let url = URL(string: config.serverEndpoint + path) { var request = URLRequest(url: url) for (field, value) in headers { @@ -81,9 +89,22 @@ public class HttpPingUploader { } request.timeoutInterval = TimeInterval(Constants.connectionTimeout) request.httpMethod = "POST" - request.httpBody = Data(data.utf8) request.httpShouldHandleCookies = false + // NOTE: We're using `URLSession.uploadTask` in `PingUploadOperation`, + // which ignores the `httpBody` and instead takes the body payload as a parameter + // to add to the request. + // However in tests we're using OHHTTPStubs to stub out the HTTP upload. + // It has the known limitation that it doesn't simulate data upload, + // because the underlying protocol doesn't expose a hook for that. + // By setting `httpBody` here the data is still attached to the request, + // so OHHTTPStubs sees it. + // It shouldn't be too bad memory-wise and not duplicate the data in memory. + // This should only be a reference and Swift keeps track of all the places it's needed. + // + // See https://github.com/AliSoftware/OHHTTPStubs#known-limitations. + request.httpBody = data + if let tag = config.pingTag { request.addValue(tag, forHTTPHeaderField: "X-Debug-ID") } @@ -100,13 +121,14 @@ public class HttpPingUploader { /// It will continue upload as long as it can fetch new tasks. func process() { while true { - let incomingTask = glean_get_upload_task() + var incomingTask = FfiPingUploadTask() + glean_get_upload_task(&incomingTask) let task = incomingTask.toPingUploadTask() switch task { case let .upload(request): self.upload(path: request.path, data: request.body, headers: request.headers) { result in - glean_process_ping_upload_response(incomingTask, result.toFfi()) + glean_process_ping_upload_response(&incomingTask, result.toFfi()) } case .wait: continue diff --git a/glean-core/ios/Glean/Net/Upload.swift b/glean-core/ios/Glean/Net/Upload.swift index db164009bd..3fd06393de 100644 --- a/glean-core/ios/Glean/Net/Upload.swift +++ b/glean-core/ios/Glean/Net/Upload.swift @@ -47,12 +47,15 @@ struct PingRequest { let documentId: String /// The path to upload this ping to. let path: String - /// The JSON-encoded payload of the ping. - let body: String + /// The body of the request. + /// + /// If gzip encoded, then the `headers` list will + /// contain a `Content-Encoding` header with the value `gzip`. + let body: Data /// A map of headers for the HTTP request to send. let headers: [String: String] - init(documentId: String, path: String, body: String, headers: [String: String]) { + init(documentId: String, path: String, body: Data, headers: [String: String]) { self.documentId = documentId self.path = path self.body = body @@ -117,7 +120,7 @@ extension FfiPingUploadTask_Upload_Body { func toPingRequest() -> PingRequest { let documentId = String(cString: self.document_id) let path = String(cString: self.path) - let body = String(cString: self.body) + let body = Data(bytes: self.body.data, count: Int(self.body.len)) // Decode the header object from JSON let json = String(cString: self.headers) diff --git a/glean-core/ios/GleanTests/Debug/GleanDebugUtilityTests.swift b/glean-core/ios/GleanTests/Debug/GleanDebugUtilityTests.swift index 8649548ca4..c8cba19972 100644 --- a/glean-core/ios/GleanTests/Debug/GleanDebugUtilityTests.swift +++ b/glean-core/ios/GleanTests/Debug/GleanDebugUtilityTests.swift @@ -18,38 +18,6 @@ class GleanDebugUtilityTests: XCTestCase { Glean.shared.setUploadEnabled(true) } - private func setupHttpResponseStub(statusCode: Int32 = 200) { - expectation = expectation(description: "Ping sent") - // This function will be handling one each of baseline, events, and metrics pings - // so we set the expected count to 3 and set it to assert for overfulfill in order - // to test that unknown pings aren't being sent. - expectation!.expectedFulfillmentCount = 3 - expectation!.assertForOverFulfill = true - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { request in - let request = request as NSURLRequest - let body = request.ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] - XCTAssert(json != nil) - let pingType = request.url?.path.split(separator: "/")[2] - XCTAssertTrue( - Glean.shared.testHasPingType(String(pingType!)), - "\(String(pingType!)) should be registered, but is missing" - ) - - DispatchQueue.main.async { - // Let the response get processed before we mark the expectation fulfilled - self.expectation?.fulfill() - } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: statusCode, - headers: ["Content-Type": "application/json"] - ) - } - } - func testHandleCustomUrlTagPings() { // Check invalid tags: This should have the configuration keep the // default value of nil because they don't match the accepted @@ -105,7 +73,23 @@ class GleanDebugUtilityTests: XCTestCase { } func testHandleCustomUrlSendPing() { - setupHttpResponseStub() + expectation = expectation(description: "Ping sent") + // This test will be sending one each of baseline, events, and metrics pings + // so we set the expected count to 3 and set it to assert for overfulfill in order + // to test that unknown pings aren't being sent. + expectation!.expectedFulfillmentCount = 3 + expectation!.assertForOverFulfill = true + stubServerReceive { pingType, _ in + XCTAssertTrue( + Glean.shared.testHasPingType(pingType), + "\(pingType) should be registered, but is missing" + ) + + DispatchQueue.main.async { + // Let the response get processed before we mark the expectation fulfilled + self.expectation?.fulfill() + } + } // Create a dummy event and a dummy metric so that the // respective pings will be sent diff --git a/glean-core/ios/GleanTests/GleanTests.swift b/glean-core/ios/GleanTests/GleanTests.swift index 89b7c7147f..1ceed83155 100644 --- a/glean-core/ios/GleanTests/GleanTests.swift +++ b/glean-core/ios/GleanTests/GleanTests.swift @@ -102,13 +102,7 @@ class GleanTests: XCTestCase { } func testSendingOfForegroundBaselinePing() { - // Set up the test stub based on the default telemetry endpoint - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let body = (data as NSURLRequest).ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] - XCTAssert(json != nil) - + stubServerReceive { _, json in // Check for the "dirty_startup" flag let pingInfo = json?["ping_info"] as? [String: Any] XCTAssertEqual("foreground", pingInfo?["reason"] as? String) @@ -127,12 +121,6 @@ class GleanTests: XCTestCase { // let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } // Set up the expectation that will be fulfilled by the stub above @@ -157,10 +145,7 @@ class GleanTests: XCTestCase { glean_set_dirty_flag(true.toByte()) // Set up the test stub based on the default telemetry endpoint - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let body = (data as NSURLRequest).ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] + stubServerReceive { _, json in XCTAssert(json != nil) // Check for the "dirty_startup" flag @@ -181,12 +166,6 @@ class GleanTests: XCTestCase { // let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } // Set up the expectation that will be fulfilled by the stub above @@ -205,25 +184,13 @@ class GleanTests: XCTestCase { } func testSendingDeletionPingIfDisabledOutsideOfRun() { - // Set up the test stub based on the default telemetry endpoint - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let path = (data as NSURLRequest).url! - - let parts = path.absoluteString.split(separator: "/") - - XCTAssertEqual("deletion-request", parts[4]) + stubServerReceive { pingType, _ in + XCTAssertEqual("deletion-request", pingType) DispatchQueue.main.async { // let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } // Set up the expectation that will be fulfilled by the stub above @@ -242,20 +209,8 @@ class GleanTests: XCTestCase { func testNotSendingDeletionRequestIfUnchangedOutsideOfRun() { // Set up the test stub based on the default telemetry endpoint - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { _ in + stubServerReceive { _, _ in XCTFail("Should not have recieved any ping") - - DispatchQueue.main.async { - // let the response get processed before we mark the expectation fulfilled - self.expectation?.fulfill() - } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } // Set up the expectation that will NOT be fulfilled by the stub above. If it is @@ -293,14 +248,8 @@ class GleanTests: XCTestCase { stringMetric.set("HELLOOOOO!") // Set up the test stub based on the default telemetry endpoint - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let path = (data as NSURLRequest).url! - let parts = path.absoluteString.split(separator: "/") - XCTAssertEqual("baseline", parts[4]) - - let body = (data as NSURLRequest).ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] + stubServerReceive { pingType, json in + XCTAssertEqual("baseline", pingType) XCTAssert(json != nil) // Check for the "dirty_startup" flag @@ -317,12 +266,6 @@ class GleanTests: XCTestCase { // let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } expectation = expectation(description: "baseline ping received") @@ -334,4 +277,22 @@ class GleanTests: XCTestCase { XCTAssertNil(error, "Test timed out waiting for upload: \(error!)") } } + + func testGleanIsNotInitializedFromOtherProcesses() { + // Check to see if Glean is initialized + XCTAssert(Glean.shared.isInitialized()) + + // Set the control variable to false to simulate that we are not running + // in the main process + Glean.shared.isMainProcess = false + + // Restart glean + Glean.shared.resetGlean(clearStores: false) + + // Check to see if Glean is initialized + XCTAssertFalse(Glean.shared.isInitialized()) + + // Reset variable so as to not interfere with other tests. + Glean.shared.isMainProcess = true + } } diff --git a/glean-core/ios/GleanTests/Metrics/EventMetricTests.swift b/glean-core/ios/GleanTests/Metrics/EventMetricTests.swift index ecc0210fed..700cbb14d3 100644 --- a/glean-core/ios/GleanTests/Metrics/EventMetricTests.swift +++ b/glean-core/ios/GleanTests/Metrics/EventMetricTests.swift @@ -37,11 +37,8 @@ class EventMetricTypeTests: XCTestCase { var expectation: XCTestExpectation? var lastPingJson: [String: Any]? - private func setupHttpResponseStub(statusCode: Int32 = 200) { - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let body = (data as NSURLRequest).ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] + private func setupHttpResponseStub() { + stubServerReceive { _, json in XCTAssert(json != nil) self.lastPingJson = json @@ -50,13 +47,6 @@ class EventMetricTypeTests: XCTestCase { // Let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - // Ensure a response so that the uploader does its job. - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: statusCode, - headers: ["Content-Type": "application/json"] - ) } } diff --git a/glean-core/ios/GleanTests/Metrics/PingTests.swift b/glean-core/ios/GleanTests/Metrics/PingTests.swift index 636e5fbbb5..f9cb4fe34b 100644 --- a/glean-core/ios/GleanTests/Metrics/PingTests.swift +++ b/glean-core/ios/GleanTests/Metrics/PingTests.swift @@ -20,15 +20,10 @@ class PingTests: XCTestCase { } private func setupHttpResponseStub(_ expectedPingType: String) { - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { request in - let request = request as NSURLRequest - let pingType = request.url?.path.split(separator: "/")[2] - XCTAssertEqual(String(pingType!), expectedPingType, "Wrong ping type received") - - let body = request.ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] + stubServerReceive { pingType, json in + XCTAssertEqual(pingType, expectedPingType, "Wrong ping type received") XCTAssert(json != nil) + self.lastPingJson = json // Fulfill test's expectation once we parsed the incoming data. @@ -36,13 +31,6 @@ class PingTests: XCTestCase { // Let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - // Ensure a response so that the uploader does its job. - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } } diff --git a/glean-core/ios/GleanTests/Net/DeletionRequestPingTests.swift b/glean-core/ios/GleanTests/Net/DeletionRequestPingTests.swift index 4c1d6cad73..7a9df71806 100644 --- a/glean-core/ios/GleanTests/Net/DeletionRequestPingTests.swift +++ b/glean-core/ios/GleanTests/Net/DeletionRequestPingTests.swift @@ -13,14 +13,9 @@ class DeletionRequestPingTests: XCTestCase { var lastPingJson: [String: Any]? private func setupHttpResponseStub(_ expectedPingType: String) { - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { request in - let request = request as NSURLRequest - let pingType = request.url?.path.split(separator: "/")[2] - XCTAssertEqual(String(pingType!), expectedPingType, "Wrong ping type received") - - let body = request.ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] + stubServerReceive { pingType, json in + XCTAssertEqual(pingType, expectedPingType, "Wrong ping type received") + XCTAssert(json != nil) self.lastPingJson = json @@ -29,13 +24,6 @@ class DeletionRequestPingTests: XCTestCase { // Let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - // Ensure a response so that the uploader does its job. - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } } diff --git a/glean-core/ios/GleanTests/Net/HttpPingUploaderTests.swift b/glean-core/ios/GleanTests/Net/HttpPingUploaderTests.swift index 3c7013a60f..e777a4a0d5 100644 --- a/glean-core/ios/GleanTests/Net/HttpPingUploaderTests.swift +++ b/glean-core/ios/GleanTests/Net/HttpPingUploaderTests.swift @@ -22,22 +22,6 @@ class HttpPingUploaderTests: XCTestCase { expectation = nil } - private func setupHttpResponseStub(statusCode: Int32 = 200) { - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let body = (data as NSURLRequest).ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] - XCTAssert(json != nil) - XCTAssertEqual(json?["ping"] as? String, "test") - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: statusCode, - headers: ["Content-Type": "application/json"] - ) - } - } - func getOrCreatePingDirectory(_ pingDirectory: String) -> URL { let dataPath = getGleanDirectory().appendingPathComponent(pingDirectory) @@ -83,12 +67,15 @@ class HttpPingUploaderTests: XCTestCase { func testHTTPStatusCode() { var testValue: UploadResult? - setupHttpResponseStub(statusCode: 200) + stubServerReceive { _, json in + XCTAssert(json != nil) + XCTAssertEqual(json?["ping"] as? String, "test") + } expectation = expectation(description: "Completed upload") let httpPingUploader = HttpPingUploader(configuration: testConfig) - httpPingUploader.upload(path: testPath, data: testPing, headers: [:]) { result in + httpPingUploader.upload(path: testPath, data: Data(testPing.utf8), headers: [:]) { result in testValue = result self.expectation?.fulfill() } @@ -109,7 +96,7 @@ class HttpPingUploaderTests: XCTestCase { // We specify a single additional header here. // In usual code they are generated on the Rust side. let request = HttpPingUploader(configuration: testConfig) - .buildRequest(path: testPath, data: testPing, headers: ["X-Client-Type": "Glean"]) + .buildRequest(path: testPath, data: Data(testPing.utf8), headers: ["X-Client-Type": "Glean"]) XCTAssertEqual( request?.url?.path, @@ -171,10 +158,7 @@ class HttpPingUploaderTests: XCTestCase { // Now set up our test server var countFilesUploaded = 0 - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let body = (data as NSURLRequest).ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] + stubServerReceive { _, json in XCTAssert(json != nil) XCTAssertEqual(json?["ping"] as? String, "test") @@ -185,12 +169,6 @@ class HttpPingUploaderTests: XCTestCase { self.expectation?.fulfill() } } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } // Set our expectation that will be fulfilled by the stub above @@ -235,10 +213,7 @@ class HttpPingUploaderTests: XCTestCase { ) // Now set up our test server - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let body = (data as NSURLRequest).ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] + stubServerReceive { _, json in XCTAssert(json != nil) XCTAssertEqual(json?["ping"] as? String, "test") @@ -246,12 +221,6 @@ class HttpPingUploaderTests: XCTestCase { // let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } // Set our expectation that will be fulfilled by the stub above diff --git a/glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift b/glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift index 8b20ac90e4..1fb7ffd04f 100644 --- a/glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift +++ b/glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift @@ -135,8 +135,6 @@ class MetricsPingSchedulerTests: XCTestCase { ) } - // swiftlint:disable function_body_length - // REASON: Used in a test func testQueuedDataNotInOverdueMetricsPings() { // Reset Glean and do not start it right away Glean.shared.testDestroyGleanHandle() @@ -182,10 +180,7 @@ class MetricsPingSchedulerTests: XCTestCase { let yesterday = Calendar.current.date(byAdding: Calendar.Component.day, value: -1, to: now) Glean.shared.metricsPingScheduler.updateSentDate(yesterday!) - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let body = (data as NSURLRequest).ohhttpStubs_HTTPBody() - let json = try! JSONSerialization.jsonObject(with: body!, options: []) as? [String: Any] + stubServerReceive { _, json in XCTAssert(json != nil) let metrics = json?["metrics"] as? [String: Any] let strings = metrics?["string"] as? [String: Any] @@ -201,12 +196,6 @@ class MetricsPingSchedulerTests: XCTestCase { // let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } // Set our expectation that will be fulfilled by the stub above @@ -257,24 +246,18 @@ class MetricsPingSchedulerTests: XCTestCase { Glean.shared.metricsPingScheduler.updateSentDate(yesterday!) // Set up the interception of the ping for inspection - let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! - stub(condition: isHost(host)) { data in - let docType = (data as NSURLRequest).url!.path.split(separator: "/")[2] - XCTAssertEqual(docType, "metrics", "Must be a metrics ping") + stubServerReceive { pingType, json in + XCTAssertEqual(pingType, "metrics", "Must be a metrics ping") - let body = String(decoding: (data as NSURLRequest).ohhttpStubs_HTTPBody(), as: UTF8.self) - XCTAssertTrue(body.contains(expectedValue), "Must contain expected value") + let metrics = json?["metrics"] as? [String: Any] + let strings = metrics?["string"] as? [String: Any] + let testMetric = strings?["telemetry.test_applifetime_metric"] as? String + XCTAssertEqual(expectedValue, testMetric, "Must contain expected value") DispatchQueue.main.async { // let the response get processed before we mark the expectation fulfilled self.expectation?.fulfill() } - - return OHHTTPStubsResponse( - jsonObject: [], - statusCode: 200, - headers: ["Content-Type": "application/json"] - ) } // Set our expectation that will be fulfilled by the stub above diff --git a/glean-core/ios/GleanTests/TestUtils.swift b/glean-core/ios/GleanTests/TestUtils.swift new file mode 100644 index 0000000000..f17348c8c5 --- /dev/null +++ b/glean-core/ios/GleanTests/TestUtils.swift @@ -0,0 +1,43 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +@testable import Glean +import Gzip +import OHHTTPStubs + +/// Stub out receiving a request on Glean's default Telemetry endpoint. +/// +/// When receiving a request, it extracts the ping type from the URL +/// according to the endpoint URL format. +/// +/// It assumes the request body is a JSON document and tries to decode it. +/// If necessary, it decompresses the request body first. +/// +/// - parameters +/// * callback: A callback to validate the incoming request. +/// It receives a `pingType` and the ping's JSON-decoded `payload`. +func stubServerReceive(callback: @escaping (String, [String: Any]?) -> Void) { + let host = URL(string: Configuration.Constants.defaultTelemetryEndpoint)!.host! + stub(condition: isHost(host)) { data in + let request = data as NSURLRequest + let url = request.url! + let parts = url.absoluteString.split(separator: "/") + let pingType = String(parts[4]) + + var body = request.ohhttpStubs_HTTPBody()! + if request.value(forHTTPHeaderField: "Content-Encoding") == "gzip" { + body = try! body.gunzipped() + } + + let payload = try! JSONSerialization.jsonObject(with: body, options: []) as? [String: Any] + + callback(pingType, payload) + + return OHHTTPStubsResponse( + jsonObject: [], + statusCode: 200, + headers: ["Content-Type": "application/json"] + ) + } +} diff --git a/glean-core/preview/Cargo.toml b/glean-core/preview/Cargo.toml index 211ab582a4..06711a8b5c 100644 --- a/glean-core/preview/Cargo.toml +++ b/glean-core/preview/Cargo.toml @@ -23,7 +23,7 @@ maintenance = { status = "actively-developed" } [dependencies.glean-core] path = ".." -version = "30.0.0" +version = "30.1.0" [dependencies] once_cell = "1.2.0" diff --git a/glean-core/python/glean/_loader.py b/glean-core/python/glean/_loader.py index 876bc3d65a..8825663925 100644 --- a/glean-core/python/glean/_loader.py +++ b/glean-core/python/glean/_loader.py @@ -35,7 +35,7 @@ "labeled_boolean": metrics.LabeledBooleanMetricType, "labeled_counter": metrics.LabeledCounterMetricType, "labeled_string": metrics.LabeledStringMetricType, - "memory_unit": metrics.MemoryDistributionMetricType, + "memory_distribution": metrics.MemoryDistributionMetricType, "ping": metrics.PingType, "string": metrics.StringMetricType, "string_list": metrics.StringListMetricType, diff --git a/glean-core/python/glean/_subprocess/_process_dispatcher_helper.py b/glean-core/python/glean/_subprocess/_process_dispatcher_helper.py index e199d9002e..144d537557 100644 --- a/glean-core/python/glean/_subprocess/_process_dispatcher_helper.py +++ b/glean-core/python/glean/_subprocess/_process_dispatcher_helper.py @@ -18,7 +18,7 @@ import sys # Run coverage in the subprocess if necessary - if "COVERAGE_PROCESS_START" in os.environ: + if "GLEAN_COVERAGE" in os.environ and "COVERAGE_PROCESS_START" in os.environ: import coverage # type: ignore config_path = os.environ.get("COVERAGE_PROCESS_START") diff --git a/glean-core/python/glean/glean.py b/glean-core/python/glean/glean.py index cd9c489668..b36c8f87e0 100644 --- a/glean-core/python/glean/glean.py +++ b/glean-core/python/glean/glean.py @@ -150,8 +150,12 @@ def initialize( for ping in cls._ping_type_queue: cls.register_ping_type(ping) - # Initialize the core metrics - cls._initialize_core_metrics() + # If this is the first time ever the Glean SDK runs, make sure to set + # some initial core metrics in case we need to generate early pings. + # The next times we start, we would have them around already. + is_first_run = _ffi.lib.glean_is_first_run() != 0 + if is_first_run: + cls._initialize_core_metrics() # Glean Android sets up the metrics ping scheduler here, but we don't # have one. @@ -162,6 +166,16 @@ def submit_pending_events(): if _ffi.lib.glean_on_ready_to_submit_pings(): PingUploadWorker.process() + # Glean Android checks for the "dirty bit" and sends the `baseline` ping + # with reason `dirty_startup`. + + # From the second time we run, after all startup pings are generated, + # make sure to clear `lifetime: application` metrics and set them again. + # Any new value will be sent in newly generated pings after startup. + if not is_first_run: + _ffi.lib.glean_clear_application_lifetime_metrics() + cls._initialize_core_metrics() + Dispatcher.flush_queued_initial_tasks() # Glean Android sets up the lifecycle observer here. We don't really diff --git a/glean-core/python/setup.py b/glean-core/python/setup.py index 8cdeb85d9f..e073830529 100644 --- a/glean-core/python/setup.py +++ b/glean-core/python/setup.py @@ -66,7 +66,9 @@ shared_object = "libglean_ffi.so" elif platform == "darwin": shared_object = "libglean_ffi.dylib" -elif platform == "windows": +elif platform.startswith("win"): + # `platform` can be both "windows" (if running within MinGW) or "win32" + # if running in a standard Python environment. Account for both. shared_object = "glean_ffi.dll" else: raise ValueError("The platform {} is not supported.".format(sys.platform)) diff --git a/glean-core/python/tests/test_glean.py b/glean-core/python/tests/test_glean.py index 0ffa28631b..86496cd2b1 100644 --- a/glean-core/python/tests/test_glean.py +++ b/glean-core/python/tests/test_glean.py @@ -594,3 +594,38 @@ def broken_process(*args, **kwargs): assert process.returncode == 0 assert 1 == len(safe_httpserver.requests) + + +def test_clear_application_lifetime_metrics(tmpdir): + Glean._reset() + + Glean.initialize( + application_id=GLEAN_APP_ID, + application_version=glean_version, + upload_enabled=True, + data_dir=Path(str(tmpdir)), + ) + + counter_metric = CounterMetricType( + disabled=False, + category="test.telemetry", + lifetime=Lifetime.APPLICATION, + name="lifetime_reset", + send_in_pings=["store1"], + ) + + counter_metric.add(10) + + assert counter_metric.test_has_value() + assert counter_metric.test_get_value() == 10 + + Glean._reset() + + Glean.initialize( + application_id=GLEAN_APP_ID, + application_version=glean_version, + upload_enabled=True, + data_dir=Path(str(tmpdir)), + ) + + assert not counter_metric.test_has_value() diff --git a/glean-core/src/error.rs b/glean-core/src/error.rs index bf7b20b1e6..e545fdf3c7 100644 --- a/glean-core/src/error.rs +++ b/glean-core/src/error.rs @@ -102,7 +102,7 @@ impl Display for Error { MemoryUnit(m) => write!(f, "MemoryUnit conversion from {} failed", m), HistogramType(h) => write!(f, "HistogramType conversion from {} failed", h), OsString(s) => write!(f, "OsString conversion from {:?} failed", s), - Utf8Error => write!(f, "Invalid UTF-8 byte sequence in string"), + Utf8Error => write!(f, "Invalid UTF-8 byte sequence in string"), NotInitialized => write!(f, "Global Glean object missing"), __NonExhaustive => write!(f, "Unknown error"), } diff --git a/glean-core/src/upload/request.rs b/glean-core/src/upload/request.rs index 998c82c150..e745e2cd17 100644 --- a/glean-core/src/upload/request.rs +++ b/glean-core/src/upload/request.rs @@ -7,7 +7,10 @@ use std::collections::HashMap; use chrono::prelude::{DateTime, Utc}; +use flate2::write::GzEncoder; +use flate2::Compression; use serde_json::{self, Value as JsonValue}; +use std::io::prelude::*; /// Represents a request to upload a ping. #[derive(PartialEq, Debug, Clone)] @@ -17,8 +20,10 @@ pub struct PingRequest { pub document_id: String, /// The path for the server to upload the ping to. pub path: String, - /// The body of the request. - pub body: JsonValue, + /// The body of the request, as a byte array. If gzip encoded, then + /// the `headers` list will contain a `Content-Encoding` header with + /// the value `gzip`. + pub body: Vec, /// A map with all the headers to be sent with the request. pub headers: HashMap<&'static str, String>, } @@ -29,11 +34,19 @@ impl PingRequest { /// Automatically creates the default request headers. /// Clients may add more headers such as `userAgent` to this list. pub fn new(document_id: &str, path: &str, body: JsonValue) -> Self { + // We want uploads to be gzip'd. Instead of doing this for each platform + // we have language bindings for, apply compression here. + let original_as_string = body.to_string(); + let gzipped_content = Self::gzip_content(path, original_as_string.as_bytes()); + let add_gzip_header = gzipped_content.is_some(); + let body = gzipped_content.unwrap_or_else(|| original_as_string.into_bytes()); + let body_len = body.len(); + Self { document_id: document_id.into(), path: path.into(), body, - headers: Self::create_request_headers(), + headers: Self::create_request_headers(add_gzip_header, body_len), } } @@ -47,8 +60,21 @@ impl PingRequest { .unwrap_or(false) } + /// Attempt to gzip the provided ping content. + fn gzip_content(path: &str, content: &[u8]) -> Option> { + let mut gzipper = GzEncoder::new(Vec::new(), Compression::default()); + + // Attempt to add the content to the gzipper. + if let Err(e) = gzipper.write_all(content) { + log::error!("Failed to write to the gzipper: {} - {:?}", path, e); + return None; + } + + gzipper.finish().ok() + } + /// Creates the default request headers. - fn create_request_headers() -> HashMap<&'static str, String> { + fn create_request_headers(is_gzipped: bool, body_len: usize) -> HashMap<&'static str, String> { let mut headers = HashMap::new(); let date: DateTime = Utc::now(); headers.insert("Date", date.to_string()); @@ -57,6 +83,10 @@ impl PingRequest { "Content-Type", "application/json; charset=utf-8".to_string(), ); + headers.insert("Content-Length", body_len.to_string()); + if is_gzipped { + headers.insert("Content-Encoding", "gzip".to_string()); + } headers.insert("X-Client-Version", crate::GLEAN_VERSION.to_string()); headers } diff --git a/gradle-plugin/src/main/groovy/mozilla/telemetry/glean-gradle-plugin/GleanGradlePlugin.groovy b/gradle-plugin/src/main/groovy/mozilla/telemetry/glean-gradle-plugin/GleanGradlePlugin.groovy index 80ccf25c4a..e06d8b35b8 100644 --- a/gradle-plugin/src/main/groovy/mozilla/telemetry/glean-gradle-plugin/GleanGradlePlugin.groovy +++ b/gradle-plugin/src/main/groovy/mozilla/telemetry/glean-gradle-plugin/GleanGradlePlugin.groovy @@ -369,7 +369,7 @@ subprocess.check_call([ } void apply(Project project) { - project.ext.glean_version = "30.0.0" + project.ext.glean_version = "30.1.0" File condaDir = setupPythonEnvironmentTasks(project) project.ext.set("gleanCondaDir", condaDir) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index ea13fdfd19..5028f28f8e 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-5.3.1-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-5.6.4-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/samples/android/app/src/androidTest/java/org/mozilla/samples/gleancore/pings/SharedTestUtils.kt b/samples/android/app/src/androidTest/java/org/mozilla/samples/gleancore/pings/SharedTestUtils.kt index dd91fe2d15..7f42d36e47 100644 --- a/samples/android/app/src/androidTest/java/org/mozilla/samples/gleancore/pings/SharedTestUtils.kt +++ b/samples/android/app/src/androidTest/java/org/mozilla/samples/gleancore/pings/SharedTestUtils.kt @@ -4,11 +4,13 @@ package org.mozilla.samples.gleancore.pings +import mozilla.telemetry.glean.utils.decompressGZIP import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.RecordedRequest import org.json.JSONObject +import java.io.ByteArrayInputStream import java.util.concurrent.TimeUnit /** @@ -44,7 +46,12 @@ fun waitForPingContent( val request = server.takeRequest(20L, TimeUnit.SECONDS) val docType = request.path.split("/")[3] if (pingName == docType) { - return JSONObject(request.body.readUtf8()) + return if (request.getHeader("Content-Encoding") == "gzip") { + val bodyInBytes = ByteArrayInputStream(request.body.readByteArray()).readBytes() + JSONObject(decompressGZIP(bodyInBytes)) + } else { + JSONObject(request.body.readUtf8()) + } } } while (attempts < maxAttempts) diff --git a/samples/ios/app/Cartfile b/samples/ios/app/Cartfile index d7ee22595e..ee3d21f72d 100644 --- a/samples/ios/app/Cartfile +++ b/samples/ios/app/Cartfile @@ -1,2 +1,3 @@ github "httpswift/swifter" ~> 1.4.7 +github "1024jp/GzipSwift" "5.1.1" github "mozilla/glean" "master" diff --git a/samples/ios/app/glean-sample-app.xcodeproj/project.pbxproj b/samples/ios/app/glean-sample-app.xcodeproj/project.pbxproj index 6ed937e5c2..4b055f1fec 100644 --- a/samples/ios/app/glean-sample-app.xcodeproj/project.pbxproj +++ b/samples/ios/app/glean-sample-app.xcodeproj/project.pbxproj @@ -9,6 +9,7 @@ /* Begin PBXBuildFile section */ BF1BF3FF23337E6F0036F4CC /* Glean.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = BF1BF3FE23337E6F0036F4CC /* Glean.framework */; }; BF89665A2371C82E0097B7F2 /* pings.yaml in Resources */ = {isa = PBXBuildFile; fileRef = BF8966592371C82E0097B7F2 /* pings.yaml */; }; + BF8C3866246D749400274ACE /* Gzip.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = BF8C3865246D749400274ACE /* Gzip.framework */; }; BF90C913236B43A10045BD0E /* Swifter.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = BF90C911236B42F00045BD0E /* Swifter.framework */; }; BFD3AB2E224D475E00AD9255 /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFD3AB2D224D475E00AD9255 /* AppDelegate.swift */; }; BFD3AB30224D475E00AD9255 /* ViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = BFD3AB2F224D475E00AD9255 /* ViewController.swift */; }; @@ -48,6 +49,7 @@ BF1BF3FE23337E6F0036F4CC /* Glean.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Glean.framework; path = Carthage/Build/iOS/Glean.framework; sourceTree = ""; }; BF6F41C4234346A000E222A8 /* metrics.yaml */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.yaml; path = metrics.yaml; sourceTree = ""; }; BF8966592371C82E0097B7F2 /* pings.yaml */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.yaml; path = pings.yaml; sourceTree = ""; }; + BF8C3865246D749400274ACE /* Gzip.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Gzip.framework; path = Carthage/Build/iOS/Gzip.framework; sourceTree = ""; }; BF90C911236B42F00045BD0E /* Swifter.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Swifter.framework; path = Carthage/Build/iOS/Swifter.framework; sourceTree = ""; }; BFD3AB2A224D475E00AD9255 /* glean-sample-app.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = "glean-sample-app.app"; sourceTree = BUILT_PRODUCTS_DIR; }; BFD3AB2D224D475E00AD9255 /* AppDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppDelegate.swift; sourceTree = ""; }; @@ -96,6 +98,7 @@ isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; files = ( + BF8C3866246D749400274ACE /* Gzip.framework in Frameworks */, BF90C913236B43A10045BD0E /* Swifter.framework in Frameworks */, ); runOnlyForDeploymentPostprocessing = 0; @@ -180,6 +183,7 @@ BFD3AB68224D4F9500AD9255 /* Frameworks */ = { isa = PBXGroup; children = ( + BF8C3865246D749400274ACE /* Gzip.framework */, BF90C911236B42F00045BD0E /* Swifter.framework */, BF1BF3FE23337E6F0036F4CC /* Glean.framework */, ); @@ -352,12 +356,14 @@ ); inputPaths = ( "$(SRCROOT)/Carthage/Build/iOS/Swifter.framework", + "$(SRCROOT)/Carthage/Build/iOS/Gzip.framework", ); name = "Copy Carthage frameworks"; outputFileListPaths = ( ); outputPaths = ( "$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Swifter.framework", + "$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Gzip.framework", ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/sh; @@ -372,12 +378,14 @@ ); inputPaths = ( "$(SRCROOT)/Carthage/Build/iOS/Glean.framework", + "$(SRCROOT)/Carthage/Build/iOS/Gzip.framework", ); name = "Copy Carthage frameworks"; outputFileListPaths = ( ); outputPaths = ( "$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Glean.framework", + "$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Gzip.framework", ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/sh; diff --git a/samples/ios/app/glean-sample-app.xcodeproj/xcshareddata/xcschemes/glean-sample-app.xcscheme b/samples/ios/app/glean-sample-app.xcodeproj/xcshareddata/xcschemes/glean-sample-app.xcscheme index 09f49931dc..83c2889c67 100644 --- a/samples/ios/app/glean-sample-app.xcodeproj/xcshareddata/xcschemes/glean-sample-app.xcscheme +++ b/samples/ios/app/glean-sample-app.xcodeproj/xcshareddata/xcschemes/glean-sample-app.xcscheme @@ -28,6 +28,16 @@ selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" shouldUseLaunchSchemeArgsEnv = "YES"> + + + +