1589743: Don't log pings twice

This commit is contained in:
Michael Droettboom 2019-10-23 16:33:45 -04:00
Родитель 345288b354
Коммит e22638e1b2
11 изменённых файлов: 20 добавлений и 33 удалений

Просмотреть файл

@ -524,8 +524,7 @@ open class GleanInternalAPI internal constructor () {
val sentPing = LibGleanFFI.INSTANCE.glean_send_pings_by_name(
handle,
pingArray,
pingArrayLen,
(configuration.logPings).toByte()
pingArrayLen
).toBoolean()
if (sentPing) {

Просмотреть файл

@ -93,8 +93,7 @@ internal interface LibGleanFFI : Library {
fun glean_send_pings_by_name(
glean_handle: Long,
ping_names: StringArray,
ping_names_len: Int,
log_ping: Byte
ping_names_len: Int
): Byte
fun glean_set_experiment_active(

Просмотреть файл

@ -332,8 +332,7 @@ void glean_register_ping_type(uint64_t glean_handle, uint64_t ping_type_handle);
uint8_t glean_send_pings_by_name(uint64_t glean_handle,
RawStringArray ping_names,
int32_t ping_names_len,
uint8_t log_ping);
int32_t ping_names_len);
void glean_set_experiment_active(uint64_t glean_handle,
FfiStr experiment_id,

Просмотреть файл

@ -189,13 +189,11 @@ pub extern "C" fn glean_send_pings_by_name(
glean_handle: u64,
ping_names: RawStringArray,
ping_names_len: i32,
log_ping: u8,
) -> u8 {
GLEAN.call_with_log(glean_handle, |glean| {
let log_ping = log_ping != 0;
let pings = from_raw_string_array(ping_names, ping_names_len)?;
Ok(glean.send_pings_by_name(&pings, log_ping))
Ok(glean.send_pings_by_name(&pings))
})
}

Просмотреть файл

@ -226,8 +226,7 @@ public class Glean {
let sentPing = glean_send_pings_by_name(
self.handle,
pingNames,
Int32(pingNames?.count ?? 0),
self.configuration!.logPings.toByte()
Int32(pingNames?.count ?? 0)
)
if sentPing != 0 {

Просмотреть файл

@ -332,8 +332,7 @@ void glean_register_ping_type(uint64_t glean_handle, uint64_t ping_type_handle);
uint8_t glean_send_pings_by_name(uint64_t glean_handle,
RawStringArray ping_names,
int32_t ping_names_len,
uint8_t log_ping);
int32_t ping_names_len);
void glean_set_experiment_active(uint64_t glean_handle,
FfiStr experiment_id,

Просмотреть файл

@ -140,7 +140,7 @@ impl EventDatabase {
let mut ping_sent = false;
for store_name in store_names {
if let Err(err) = glean.send_ping_by_name(&store_name, false) {
if let Err(err) = glean.send_ping_by_name(&store_name) {
log::error!(
"Error flushing existing events to the '{}' ping: {}",
store_name,
@ -199,7 +199,7 @@ impl EventDatabase {
// If any of the event stores reached maximum size, send the pings
// containing those events immediately.
for store_name in stores_to_send {
if let Err(err) = glean.send_ping_by_name(store_name, false) {
if let Err(err) = glean.send_ping_by_name(store_name) {
log::error!(
"Got more than {} events, but could not send {} ping: {}",
glean.get_max_events(),

Просмотреть файл

@ -94,7 +94,7 @@ pub struct Configuration {
///
/// call_counter.add(&glean, 1);
///
/// glean.send_ping(&ping, true).unwrap();
/// glean.send_ping(&ping).unwrap();
/// ```
///
/// ## Note
@ -401,7 +401,7 @@ impl Glean {
///
/// Returns true if a ping was assembled and queued, false otherwise.
/// Returns an error if collecting or writing the ping to disk failed.
pub fn send_ping(&self, ping: &PingType, log_ping: bool) -> Result<bool> {
pub fn send_ping(&self, ping: &PingType) -> Result<bool> {
let ping_maker = PingMaker::new();
let doc_id = Uuid::new_v4().to_string();
let url_path = self.make_path(&ping.name, &doc_id);
@ -414,11 +414,6 @@ impl Glean {
Ok(false)
}
Some(content) => {
if log_ping {
// Use pretty-printing for log
log::info!("{}", ::serde_json::to_string_pretty(&content)?);
}
if let Err(e) =
ping_maker.store_ping(&doc_id, &self.get_data_path(), &url_path, &content)
{
@ -440,14 +435,14 @@ impl Glean {
/// See `send_ping` for detailed information.
///
/// Returns true if at least one ping was assembled and queued, false otherwise.
pub fn send_pings_by_name(&self, ping_names: &[String], log_ping: bool) -> bool {
pub fn send_pings_by_name(&self, ping_names: &[String]) -> bool {
// TODO: 1553813: glean-ac collects and stores pings in parallel and then joins them all before queueing the worker.
// This here is writing them out sequentially.
let mut result = false;
for ping_name in ping_names {
if let Ok(true) = self.send_ping_by_name(ping_name, log_ping) {
if let Ok(true) = self.send_ping_by_name(ping_name) {
result = true;
}
}
@ -464,13 +459,13 @@ impl Glean {
///
/// Returns true if a ping was assembled and queued, false otherwise.
/// Returns an error if collecting or writing the ping to disk failed.
pub fn send_ping_by_name(&self, ping_name: &str, log_ping: bool) -> Result<bool> {
pub fn send_ping_by_name(&self, ping_name: &str) -> Result<bool> {
match self.get_ping_by_name(ping_name) {
None => {
log::error!("Attempted to send unknown ping '{}'", ping_name);
Ok(false)
}
Some(ping) => self.send_ping(ping, log_ping),
Some(ping) => self.send_ping(ping),
}
}

Просмотреть файл

@ -38,12 +38,11 @@ impl PingType {
/// ## Arguments
///
/// * `glean` - the Glean instance to use to send the ping.
/// * `log_ping` - whether to log the ping after assembly.
///
/// ## Return value
///
/// See [`Glean#send_ping`](../struct.Glean.html#method.send_ping) for details.
pub fn send(&self, glean: &Glean, log_ping: bool) -> Result<bool> {
glean.send_ping(self, log_ping)
pub fn send(&self, glean: &Glean) -> Result<bool> {
glean.send_ping(self)
}
}

Просмотреть файл

@ -24,7 +24,7 @@ fn write_ping_to_disk() {
});
counter.add(&glean, 1);
assert!(ping.send(&glean, true).unwrap());
assert!(ping.send(&glean).unwrap());
assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len());
}
@ -45,7 +45,7 @@ fn disabling_upload_clears_pending_pings() {
});
counter.add(&glean, 1);
assert!(ping.send(&glean, true).unwrap());
assert!(ping.send(&glean).unwrap());
assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len());
glean.set_upload_enabled(false);
@ -55,6 +55,6 @@ fn disabling_upload_clears_pending_pings() {
assert_eq!(0, get_queued_pings(glean.get_data_path()).unwrap().len());
counter.add(&glean, 1);
assert!(ping.send(&glean, true).unwrap());
assert!(ping.send(&glean).unwrap());
assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len());
}

Просмотреть файл

@ -171,7 +171,7 @@ fn test_clear_pending_pings() {
});
metric.set(&glean, true);
assert!(glean.send_ping(&ping_type, false).is_ok());
assert!(glean.send_ping(&ping_type).is_ok());
assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len());
assert!(ping_maker