Merge branch 'master' into dispatchers_assert_tests

Makefile

@@ -1,7 +1,3 @@
-ifeq ($(ANDROID_HOME),)
-	ANDROID_HOME := ~/Library/Android/sdk
-endif
-
 build-apk:
 	./gradlew glean-core:build 
 	./gradlew glean-sample-app:build
@@ -15,7 +11,7 @@ emulator:
 	$(ANDROID_HOME)/emulator/emulator -avd Nexus_5X_API_P -netdelay none -netspeed full
 .PHONY: install
 
-lint:
+lint: fmt
 	cargo clippy --all
 	./gradlew ktlint detekt
 .PHONY: lint
@@ -25,7 +21,7 @@ fmt:
 .PHONY: fmt
 
 test:
-	RUST_TEST_THREADS=1 cargo test --all
+	cargo test --all
 	./gradlew test
 .PHONY: test
 

README.md

@@ -30,6 +30,14 @@ User-facing documentation can be found in [./docs/user](docs/user).
 
 The Rust documentation is available [online as well](https://mozilla.github.com/glean/docs).
 
+## Contact
+
+To contact us you can:
+- Find us on the Mozilla Slack in *#glean*, on [Mozilla IRC](https://wiki.mozilla.org/IRC) in *#telemetry*.
+- To report issues or request changes, file a bug in [Bugzilla in Data Platform & Tools :: Glean: SDK](https://bugzilla.mozilla.org/enter_bug.cgi?assigned_to=nobody%40mozilla.org&bug_ignored=0&bug_severity=normal&bug_status=NEW&cf_fission_milestone=---&cf_fx_iteration=---&cf_fx_points=---&cf_status_firefox65=---&cf_status_firefox66=---&cf_status_firefox67=---&cf_status_firefox_esr60=---&cf_status_thunderbird_esr60=---&cf_tracking_firefox65=---&cf_tracking_firefox66=---&cf_tracking_firefox67=---&cf_tracking_firefox_esr60=---&cf_tracking_firefox_relnote=---&cf_tracking_thunderbird_esr60=---&product=Data%20Platform%20and%20Tools&component=Glean%3A%20SDK&contenttypemethod=list&contenttypeselection=text%2Fplain&defined_groups=1&flag_type-203=X&flag_type-37=X&flag_type-41=X&flag_type-607=X&flag_type-721=X&flag_type-737=X&flag_type-787=X&flag_type-799=X&flag_type-800=X&flag_type-803=X&flag_type-835=X&flag_type-846=X&flag_type-855=X&flag_type-864=X&flag_type-916=X&flag_type-929=X&flag_type-930=X&flag_type-935=X&flag_type-936=X&flag_type-937=X&form_name=enter_bug&maketemplate=Remember%20values%20as%20bookmarkable%20template&op_sys=Unspecified&priority=P3&&rep_platform=Unspecified&status_whiteboard=%5Btelemetry%3Aglean-rs%3Am%3F%5D&target_milestone=---&version=unspecified).
+- Send an email to *glean-team@mozilla.com*.
+- The Glean Core team is: *:dexter*, *:janerik*, *:mdroettboom*, *:gfritzsche*
+
 ## License
 
     This Source Code Form is subject to the terms of the Mozilla Public

glean-core/android/src/main/java/mozilla/telemetry/glean/private/CounterMetricType.kt

@@ -64,7 +64,7 @@ class CounterMetricType(
             LibGleanFFI.INSTANCE.glean_counter_add(
                 Glean.handle,
                 this@CounterMetricType.handle,
-                amount.toLong())
+                amount)
         }
     }
 
@@ -102,10 +102,9 @@ class CounterMetricType(
         @Suppress("EXPERIMENTAL_API_USAGE")
         Dispatchers.API.assertInTestingMode()
 
-        // FIXME(#19): glean-core should give us an int to begin with
         if (!testHasValue(pingName)) {
             throw NullPointerException()
         }
-        return LibGleanFFI.INSTANCE.glean_counter_test_get_value(Glean.handle, this.handle, pingName).toInt()
+        return LibGleanFFI.INSTANCE.glean_counter_test_get_value(Glean.handle, this.handle, pingName)
     }
 }

glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt

@@ -60,9 +60,9 @@ internal interface LibGleanFFI : Library {
 
     fun glean_boolean_set(glean_handle: Long, metric_id: Long, value: Byte)
 
-    fun glean_counter_add(glean_handle: Long, metric_id: Long, amount: Long)
+    fun glean_counter_add(glean_handle: Long, metric_id: Long, amount: Int)
 
-    fun glean_counter_test_get_value(glean_handle: Long, metric_id: Long, storage_name: String): Long
+    fun glean_counter_test_get_value(glean_handle: Long, metric_id: Long, storage_name: String): Int
 
     fun glean_counter_test_has_value(glean_handle: Long, metric_id: Long, storage_name: String): Byte
 

glean-core/android/src/test/java/mozilla/telemetry/glean/private/CounterMetricTypeTest.kt

@@ -135,4 +135,28 @@ class CounterMetricTypeTest {
         assertTrue(counterMetric.testHasValue("store2"))
         assertEquals(11, counterMetric.testGetValue("store2"))
     }
+
+    @Test
+    fun `negative values are not counted`() {
+        // Define a 'counterMetric' counter metric, which will be stored in "store1"
+        val counterMetric = CounterMetricType(
+            disabled = false,
+            category = "telemetry",
+            lifetime = Lifetime.Application,
+            name = "counter_metric",
+            sendInPings = listOf("store1")
+        )
+
+        // Increment to 1 (initial value)
+        counterMetric.add()
+
+        // Check that the count was incremented
+        assertTrue(counterMetric.testHasValue("store1"))
+        assertEquals(1, counterMetric.testGetValue("store1"))
+
+        counterMetric.add(-10)
+        // Check that count was NOT incremented.
+        assertTrue(counterMetric.testHasValue("store1"))
+        assertEquals(1, counterMetric.testGetValue("store1"))
+    }
 }

glean-core/ffi/src/lib.rs

@@ -173,7 +173,7 @@ pub extern "C" fn glean_new_counter_metric(
 }
 
 #[no_mangle]
-pub extern "C" fn glean_counter_add(glean_handle: u64, metric_id: u64, amount: u64) {
+pub extern "C" fn glean_counter_add(glean_handle: u64, metric_id: u64, amount: i32) {
     GLEAN.call_infallible(glean_handle, |glean| {
         COUNTER_METRICS.call_infallible(metric_id, |metric| {
             metric.add(glean, amount);
@@ -201,7 +201,7 @@ pub extern "C" fn glean_counter_test_get_value(
     glean_handle: u64,
     metric_id: u64,
     storage_name: FfiStr,
-) -> u64 {
+) -> i32 {
     GLEAN.call_infallible(glean_handle, |glean| {
         COUNTER_METRICS.call_infallible(metric_id, |metric| {
             metric.test_get_value(glean, storage_name.as_str()).unwrap()

glean-core/src/metrics/counter.rs

@@ -17,11 +17,17 @@ impl CounterMetric {
         Self { meta }
     }
 
-    pub fn add(&self, glean: &Glean, amount: u64) {
+    pub fn add(&self, glean: &Glean, amount: i32) {
         if !self.meta.should_record() || !glean.is_upload_enabled() {
             return;
         }
 
+        if amount <= 0 {
+            // TODO: Turn this into logging an error
+            log::warn!("CounterMetric::add: got negative amount. Not recording.");
+            return;
+        }
+
         glean
             .storage()
             .record_with(&self.meta, |old_value| match old_value {
@@ -35,7 +41,7 @@ impl CounterMetric {
     /// Get the currently stored value as an integer.
     ///
     /// This doesn't clear the stored value.
-    pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<u64> {
+    pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<i32> {
         let snapshot = match StorageManager.snapshot_as_json(glean.storage(), storage_name, false) {
             Some(snapshot) => snapshot,
             None => return None,
@@ -45,6 +51,6 @@ impl CounterMetric {
             .and_then(|o| o.get("counter"))
             .and_then(|o| o.as_object())
             .and_then(|o| o.get(&self.meta.identifier()))
-            .and_then(|o| o.as_i64().map(|i| i as u64))
+            .and_then(|o| o.as_i64().map(|i| i as i32))
     }
 }

glean-core/src/metrics/mod.rs

@@ -21,7 +21,7 @@ pub use self::uuid::UuidMetric;
 pub enum Metric {
     String(String),
     Boolean(bool),
-    Counter(u64),
+    Counter(i32),
     Uuid(String),
     StringList(Vec<String>),
 }