Skip to content

Commit

Permalink
Add configuration to add a glean_timestamp extra to all events
Browse files Browse the repository at this point in the history
  • Loading branch information
badboy committed Aug 11, 2023
1 parent 9038cae commit 8a2b889
Show file tree
Hide file tree
Showing 19 changed files with 141 additions and 20 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

[Full changelog](https://github.com/mozilla/glean/compare/v53.2.0...main)

* General
* Experimental: Add configuration to add a wall clock timestamp to all events ([#2513](https://github.com/mozilla/glean/issues/2513))
* Python
* Switched the build system to maturin. This should not have any effect on consumers. ([#2345](https://github.com/mozilla/glean/pull/2345))
* BREAKING CHANGE: Dropped support for Python 3.6 ([#2345](https://github.com/mozilla/glean/pull/2345))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ open class GleanInternalAPI internal constructor() {
trimDataToRegisteredPings = false,
logLevel = configuration.logLevel,
rateLimit = null,
enableEventTimestamps = configuration.enableEventTimestamps,
)
val clientInfo = getClientInfo(configuration, buildInfo)
val callbacks = OnGleanEventsImpl(this@GleanInternalAPI)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import mozilla.telemetry.glean.net.PingUploader
* @property dataPath An optional [String] that specifies where to store data locally on the device.
* This should ONLY be used when setting up Glean on a non-main process.
* @property logLevel An optional [LevelFilter] that controls how verbose the internal logging is.
* @property enableEventTimestamps (Experimental) Whether to add a wallclock timestamp to all events.
*/
data class Configuration @JvmOverloads constructor(
val serverEndpoint: String = DEFAULT_TELEMETRY_ENDPOINT,
Expand All @@ -31,6 +32,7 @@ data class Configuration @JvmOverloads constructor(
val httpClient: PingUploader = HttpURLConnectionUploader(),
val dataPath: String? = null,
val logLevel: LevelFilter? = null,
val enableEventTimestamps: Boolean = false,
) {
companion object {
/**
Expand Down
6 changes: 5 additions & 1 deletion glean-core/ios/Glean/Config/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public struct Configuration {
let channel: String?
let dataPath: String?
let logLevel: LevelFilter?
let enableEventTimestamps: Bool

struct Constants {
static let defaultTelemetryEndpoint = "https://incoming.telemetry.mozilla.org"
Expand All @@ -25,17 +26,20 @@ public struct Configuration {
/// * dataPath an optional String that specifies where to store data locally on the device.
/// This should ONLY be used when setting up Glean on a non-main process.
/// * logLevel an optional log level that controls how verbose the internal logging is.
/// * enableEventTimestamps (Experimental) whether to add a wallclock timestamp to all events.
public init(
maxEvents: Int32? = nil,
channel: String? = nil,
serverEndpoint: String? = nil,
dataPath: String? = nil,
logLevel: LevelFilter? = nil
logLevel: LevelFilter? = nil,
enableEventTimestamps: Bool = false
) {
self.serverEndpoint = serverEndpoint ?? Constants.defaultTelemetryEndpoint
self.maxEvents = maxEvents
self.channel = channel
self.dataPath = dataPath
self.logLevel = logLevel
self.enableEventTimestamps = enableEventTimestamps
}
}
3 changes: 2 additions & 1 deletion glean-core/ios/Glean/Glean.swift
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ public class Glean {
useCoreMps: false,
trimDataToRegisteredPings: false,
logLevel: configuration.logLevel,
rateLimit: nil
rateLimit: nil,
enableEventTimestamps: configuration.enableEventTimestamps
)
let clientInfo = getClientInfo(configuration, buildInfo: buildInfo)
let callbacks = OnGleanEventsImpl(glean: self)
Expand Down
9 changes: 9 additions & 0 deletions glean-core/python/glean/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def __init__(
max_events: int = DEFAULT_MAX_EVENTS,
ping_uploader: Optional[net.BaseUploader] = None,
allow_multiprocessing: bool = True,
enable_event_timestamps: bool = False,
):
"""
Args:
Expand All @@ -46,6 +47,8 @@ def __init__(
implementation. Defaults to `glean.net.HttpClientUploader`.
allow_multiprocessing (bool): When True (default), use a subprocess
to offload some work (such as ping uploading).
enable_event_timestamps (bool): (Experimental) Whether to add a
wallclock timestamp to all events. Default: `false`.
"""
if server_endpoint is None:
server_endpoint = DEFAULT_TELEMETRY_ENDPOINT
Expand All @@ -56,6 +59,7 @@ def __init__(
ping_uploader = net.HttpClientUploader()
self._ping_uploader = ping_uploader
self._allow_multiprocessing = allow_multiprocessing
self._enable_event_timestamps = enable_event_timestamps

@property
def server_endpoint(self) -> str:
Expand Down Expand Up @@ -86,6 +90,11 @@ def max_events(self) -> int:

# max_events can't be changed after Glean is initialized

@property
def enable_event_timestamps(self) -> bool:
"""(Experimental) Whether to add a wallclock timestamp to all events."""
return self._enable_event_timestamps

@property
def ping_uploader(self) -> net.BaseUploader:
"""The ping uploader implementation."""
Expand Down
1 change: 1 addition & 0 deletions glean-core/python/glean/glean.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ def initialize(
trim_data_to_registered_pings=False,
log_level=None,
rate_limit=None,
enable_event_timestamps=configuration.enable_event_timestamps,
)

_uniffi.glean_initialize(cfg, client_info, callbacks)
Expand Down
1 change: 1 addition & 0 deletions glean-core/python/glean/net/ping_upload_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ def _process(data_dir: Path, application_id: str, configuration) -> bool:
trim_data_to_registered_pings=False,
log_level=None,
rate_limit=None,
enable_event_timestamps=False,
)
if not glean_initialize_for_subprocess(cfg):
log.error("Couldn't initialize Glean in subprocess")
Expand Down
12 changes: 12 additions & 0 deletions glean-core/rlb/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ pub struct Configuration {
pub log_level: Option<LevelFilter>,
/// The rate pings may be uploaded before they are throttled.
pub rate_limit: Option<crate::PingRateLimit>,
/// (Experimental) Whether to add a wallclock timestamp to all events.
pub enable_event_timestamps: bool,
}

/// Configuration builder.
Expand Down Expand Up @@ -80,6 +82,8 @@ pub struct Builder {
/// Optional: The internal ping upload rate limit.
/// Default: `None`
pub rate_limit: Option<crate::PingRateLimit>,
/// (Experimental) Whether to add a wallclock timestamp to all events.
pub enable_event_timestamps: bool,
}

impl Builder {
Expand All @@ -101,6 +105,7 @@ impl Builder {
trim_data_to_registered_pings: false,
log_level: None,
rate_limit: None,
enable_event_timestamps: false,
}
}

Expand All @@ -118,6 +123,7 @@ impl Builder {
trim_data_to_registered_pings: self.trim_data_to_registered_pings,
log_level: self.log_level,
rate_limit: self.rate_limit,
enable_event_timestamps: self.enable_event_timestamps,
}
}

Expand Down Expand Up @@ -156,4 +162,10 @@ impl Builder {
self.trim_data_to_registered_pings = value;
self
}

/// Set whether to add a wallclock timestamp to all events (experimental).
pub fn with_event_timestamps(mut self, value: bool) -> Self {
self.enable_event_timestamps = value;
self
}
}
1 change: 1 addition & 0 deletions glean-core/rlb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ fn initialize_internal(cfg: Configuration, client_info: ClientInfoMetrics) -> Op
trim_data_to_registered_pings: cfg.trim_data_to_registered_pings,
log_level: cfg.log_level,
rate_limit: cfg.rate_limit,
enable_event_timestamps: cfg.enable_event_timestamps,
};

glean_core::glean_initialize(core_cfg, client_info.into(), callbacks);
Expand Down
8 changes: 8 additions & 0 deletions glean-core/src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ where
/// trim_data_to_registered_pings: false,
/// log_level: None,
/// rate_limit: None,
/// enable_event_timestamps: false,
/// };
/// let mut glean = Glean::new(cfg).unwrap();
/// let ping = PingType::new("sample", true, false, vec![]);
Expand Down Expand Up @@ -156,6 +157,7 @@ pub struct Glean {
pub(crate) schedule_metrics_pings: bool,
pub(crate) remote_settings_epoch: AtomicU8,
pub(crate) remote_settings_metrics_config: Arc<Mutex<MetricsEnabledConfig>>,
pub(crate) with_timestamps: bool,
}

impl Glean {
Expand Down Expand Up @@ -215,6 +217,7 @@ impl Glean {
schedule_metrics_pings: false,
remote_settings_epoch: AtomicU8::new(0),
remote_settings_metrics_config: Arc::new(Mutex::new(MetricsEnabledConfig::new())),
with_timestamps: cfg.enable_event_timestamps,
};

// Ensuring these pings are registered.
Expand Down Expand Up @@ -302,6 +305,7 @@ impl Glean {
trim_data_to_registered_pings: false,
log_level: None,
rate_limit: None,
enable_event_timestamps: false,
};

let mut glean = Self::new(cfg).unwrap();
Expand Down Expand Up @@ -551,6 +555,10 @@ impl Glean {
&self.event_data_store
}

pub(crate) fn with_timestamps(&self) -> bool {
self.with_timestamps
}

/// Gets the maximum number of events to store before sending a ping.
pub fn get_max_events(&self) -> usize {
self.max_events as usize
Expand Down
10 changes: 8 additions & 2 deletions glean-core/src/event_database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::io::Write;
use std::path::{Path, PathBuf};
use std::sync::RwLock;

use chrono::{DateTime, FixedOffset};
use chrono::{DateTime, FixedOffset, Utc};

use serde::{Deserialize, Serialize};
use serde_json::{json, Value as JsonValue};
Expand Down Expand Up @@ -188,7 +188,13 @@ impl EventDatabase {
..Default::default()
};
let startup = get_iso_time_string(glean.start_time(), TimeUnit::Minute);
let extra = [("glean.startup.date".into(), startup)].into();
let mut extra: HashMap<String, String> =
[("glean.startup.date".into(), startup)].into();
if glean.with_timestamps() {
let now = Utc::now();
let precise_timestamp = now.timestamp_millis() as u64;
extra.insert("glean_timestamp".to_string(), precise_timestamp.to_string());
}
self.record(
glean,
&glean_restarted.into(),
Expand Down
1 change: 1 addition & 0 deletions glean-core/src/glean.udl
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ dictionary InternalConfiguration {
boolean trim_data_to_registered_pings;
LevelFilter? log_level;
PingRateLimit? rate_limit;
boolean enable_event_timestamps;
};

// How to specify the rate pings may be uploaded before they are throttled.
Expand Down
2 changes: 2 additions & 0 deletions glean-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ pub struct InternalConfiguration {
pub log_level: Option<LevelFilter>,
/// The rate at which pings may be uploaded before they are throttled.
pub rate_limit: Option<PingRateLimit>,
/// (Experimental) Whether to add a wallclock timestamp to all events.
pub enable_event_timestamps: bool,
}

/// How to specify the rate at which pings may be uploaded before they are throttled.
Expand Down
20 changes: 18 additions & 2 deletions glean-core/src/metrics/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use crate::util::truncate_string_at_boundary_with_error;
use crate::CommonMetricData;
use crate::Glean;

use chrono::Utc;

const MAX_LENGTH_EXTRA_KEY_VALUE: usize = 500;

/// An event metric.
Expand Down Expand Up @@ -68,8 +70,13 @@ impl EventMetric {
/// If any key is not allowed, an error is reported and no event is recorded.
pub fn record_with_time(&self, timestamp: u64, extra: HashMap<String, String>) {
let metric = self.clone();

// Precise timestamp based on wallclock. Will be used if `enable_event_timestamps` is true.
let now = Utc::now();
let precise_timestamp = now.timestamp_millis() as u64;

crate::launch_with_glean(move |glean| {
let sent = metric.record_sync(glean, timestamp, extra);
let sent = metric.record_sync(glean, timestamp, extra, precise_timestamp);
if sent {
let state = crate::global_state().lock().unwrap();
if let Err(e) = state.callbacks.trigger_upload() {
Expand Down Expand Up @@ -123,16 +130,25 @@ impl EventMetric {
glean: &Glean,
timestamp: u64,
extra: HashMap<String, String>,
precise_timestamp: u64,
) -> bool {
if !self.should_record(glean) {
return false;
}

let extra_strings = match self.validate_extra(glean, extra) {
let mut extra_strings = match self.validate_extra(glean, extra) {
Ok(extra) => extra,
Err(()) => return false,
};

if glean.with_timestamps() {
if extra_strings.is_none() {
extra_strings.replace(Default::default());
}
let map = extra_strings.get_or_insert(Default::default());
map.insert("glean_timestamp".to_string(), precise_timestamp.to_string());
}

glean
.event_storage()
.record(glean, &self.meta, timestamp, extra_strings)
Expand Down
1 change: 1 addition & 0 deletions glean-core/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub fn new_glean(tempdir: Option<tempfile::TempDir>) -> (Glean, tempfile::TempDi
trim_data_to_registered_pings: false,
log_level: None,
rate_limit: None,
enable_event_timestamps: false,
};
let glean = Glean::new(cfg).unwrap();

Expand Down
Loading

0 comments on commit 8a2b889

Please sign in to comment.