mirrors/zed
1
0
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2025-02-03 17:44:30 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

Fix bug with improperly reported environment (#4020)

Browse source 
When logging the edit environment, we were logging the newest
environment being sent into the EventCoalescer on the latest activity
log, when we should've been logging the environment that was associated
with the ended period within the EventCoalescer.

Release Notes:

- N/A
This commit is contained in:
Joseph T. Lyons 2024-01-11 12:07:48 -05:00 committed by GitHub
commit ac5f8254a2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
crates/client/src/telemetry.rs
View file

@ -404,10 +404,10 @@ impl Telemetry {
pub fn log_edit_event(self: &Arc<Self>, environment: &'static str) {
let mut state = self.state.lock();
let coalesced_duration = state.event_coalescer.log_event(environment);
let period_data = state.event_coalescer.log_event(environment);
drop(state);
if let Some((start, end)) = coalesced_duration {
if let (Some((start, end)), Some(environment)) = period_data {
let event = Event::Edit {
duration: end.timestamp_millis() - start.timestamp_millis(),
environment,

83
crates/client/src/telemetry/event_coalescer.rs
View file

@ -22,7 +22,7 @@ impl EventCoalescer {
pub fn log_event(
&mut self,
environment: &'static str,
) -> Option<(DateTime<Utc>, DateTime<Utc>)> {
) -> (Option<(DateTime<Utc>, DateTime<Utc>)>, Option<&'static str>) {
self.log_event_with_time(Utc::now(), environment)
}
@ -34,13 +34,13 @@ impl EventCoalescer {
&mut self,
log_time: DateTime<Utc>,
environment: &'static str,
) -> Option<(DateTime<Utc>, DateTime<Utc>)> {
) -> (Option<(DateTime<Utc>, DateTime<Utc>)>, Option<&'static str>) {
let coalesce_timeout = Duration::from_std(COALESCE_TIMEOUT).unwrap();
let Some(period_start) = self.period_start else {
self.period_start = Some(log_time);
self.environment = Some(environment);
return None;
return (None, None);
};
let period_end = self
@ -51,18 +51,24 @@ impl EventCoalescer {
let should_coaelesce = !within_timeout || !environment_is_same;
if should_coaelesce {
let previous_environment = self.environment;
self.period_start = Some(log_time);
self.period_end = None;
self.environment = Some(environment);
return Some((
period_start,
if within_timeout { log_time } else { period_end },
));
return (
Some((
period_start,
if within_timeout { log_time } else { period_end },
)),
previous_environment,
);
}
self.period_end = Some(log_time);
None
(None, None)
}
}
@ -82,9 +88,9 @@ mod tests {
assert_eq!(event_coalescer.environment, None);
let period_start = Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap();
let coalesced_duration = event_coalescer.log_event_with_time(period_start, environment_1);
let period_data = event_coalescer.log_event_with_time(period_start, environment_1);
assert_eq!(coalesced_duration, None);
assert_eq!(period_data, (None, None));
assert_eq!(event_coalescer.period_start, Some(period_start));
assert_eq!(event_coalescer.period_end, None);
assert_eq!(event_coalescer.environment, Some(environment_1));
@ -95,9 +101,9 @@ mod tests {
// Ensure that many calls within the timeout don't start a new period
for _ in 0..100 {
period_end += within_timeout_adjustment;
let coalesced_duration = event_coalescer.log_event_with_time(period_end, environment_1);
let period_data = event_coalescer.log_event_with_time(period_end, environment_1);
assert_eq!(coalesced_duration, None);
assert_eq!(period_data, (None, None));
assert_eq!(event_coalescer.period_start, Some(period_start));
assert_eq!(event_coalescer.period_end, Some(period_end));
assert_eq!(event_coalescer.environment, Some(environment_1));
@ -106,10 +112,12 @@ mod tests {
let exceed_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT * 2).unwrap();
// Logging an event exceeding the timeout should start a new period
let new_period_start = period_end + exceed_timeout_adjustment;
let coalesced_duration =
event_coalescer.log_event_with_time(new_period_start, environment_1);
let period_data = event_coalescer.log_event_with_time(new_period_start, environment_1);
assert_eq!(coalesced_duration, Some((period_start, period_end)));
assert_eq!(
period_data,
(Some((period_start, period_end)), Some(environment_1))
);
assert_eq!(event_coalescer.period_start, Some(new_period_start));
assert_eq!(event_coalescer.period_end, None);
assert_eq!(event_coalescer.environment, Some(environment_1));
@ -125,18 +133,18 @@ mod tests {
assert_eq!(event_coalescer.environment, None);
let period_start = Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap();
let coalesced_duration = event_coalescer.log_event_with_time(period_start, environment_1);
let period_data = event_coalescer.log_event_with_time(period_start, environment_1);
assert_eq!(coalesced_duration, None);
assert_eq!(period_data, (None, None));
assert_eq!(event_coalescer.period_start, Some(period_start));
assert_eq!(event_coalescer.period_end, None);
assert_eq!(event_coalescer.environment, Some(environment_1));
let within_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT / 2).unwrap();
let period_end = period_start + within_timeout_adjustment;
let coalesced_duration = event_coalescer.log_event_with_time(period_end, environment_1);
let period_data = event_coalescer.log_event_with_time(period_end, environment_1);
assert_eq!(coalesced_duration, None);
assert_eq!(period_data, (None, None));
assert_eq!(event_coalescer.period_start, Some(period_start));
assert_eq!(event_coalescer.period_end, Some(period_end));
assert_eq!(event_coalescer.environment, Some(environment_1));
@ -144,9 +152,12 @@ mod tests {
// Logging an event within the timeout but with a different environment should start a new period
let period_end = period_end + within_timeout_adjustment;
let environment_2 = "environment_2";
let coalesced_duration = event_coalescer.log_event_with_time(period_end, environment_2);
let period_data = event_coalescer.log_event_with_time(period_end, environment_2);
assert_eq!(coalesced_duration, Some((period_start, period_end)));
assert_eq!(
period_data,
(Some((period_start, period_end)), Some(environment_1))
);
assert_eq!(event_coalescer.period_start, Some(period_end));
assert_eq!(event_coalescer.period_end, None);
assert_eq!(event_coalescer.environment, Some(environment_2));
@ -162,9 +173,9 @@ mod tests {
assert_eq!(event_coalescer.environment, None);
let period_start = Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap();
let coalesced_duration = event_coalescer.log_event_with_time(period_start, environment_1);
let period_data = event_coalescer.log_event_with_time(period_start, environment_1);
assert_eq!(coalesced_duration, None);
assert_eq!(period_data, (None, None));
assert_eq!(event_coalescer.period_start, Some(period_start));
assert_eq!(event_coalescer.period_end, None);
assert_eq!(event_coalescer.environment, Some(environment_1));
@ -172,9 +183,12 @@ mod tests {
let within_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT / 2).unwrap();
let period_end = period_start + within_timeout_adjustment;
let environment_2 = "environment_2";
let coalesced_duration = event_coalescer.log_event_with_time(period_end, environment_2);
let period_data = event_coalescer.log_event_with_time(period_end, environment_2);
assert_eq!(coalesced_duration, Some((period_start, period_end)));
assert_eq!(
period_data,
(Some((period_start, period_end)), Some(environment_1))
);
assert_eq!(event_coalescer.period_start, Some(period_end));
assert_eq!(event_coalescer.period_end, None);
assert_eq!(event_coalescer.environment, Some(environment_2));
@ -196,9 +210,9 @@ mod tests {
assert_eq!(event_coalescer.environment, None);
let period_start = Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap();
let coalesced_duration = event_coalescer.log_event_with_time(period_start, environment_1);
let period_data = event_coalescer.log_event_with_time(period_start, environment_1);
assert_eq!(coalesced_duration, None);
assert_eq!(period_data, (None, None));
assert_eq!(event_coalescer.period_start, Some(period_start));
assert_eq!(event_coalescer.period_end, None);
assert_eq!(event_coalescer.environment, Some(environment_1));
@ -206,14 +220,17 @@ mod tests {
let exceed_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT * 2).unwrap();
let period_end = period_start + exceed_timeout_adjustment;
let environment_2 = "environment_2";
let coalesced_duration = event_coalescer.log_event_with_time(period_end, environment_2);
let period_data = event_coalescer.log_event_with_time(period_end, environment_2);
assert_eq!(
coalesced_duration,
Some((
period_start,
period_start + SIMULATED_DURATION_FOR_SINGLE_EVENT
))
period_data,
(
Some((
period_start,
period_start + SIMULATED_DURATION_FOR_SINGLE_EVENT
)),
Some(environment_1)
)
);
assert_eq!(event_coalescer.period_start, Some(period_end));
assert_eq!(event_coalescer.period_end, None);