Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record new error type "invalid state" to accurately describe the… #230

Merged
merged 2 commits into from
Oct 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
[Full changelog](https://github.com/mozilla/glean/compare/v19.0.0...master)

* Fixed a crash calling `start` on a timing distribution metric before Glean is initialized.
Timings are always measured, but only recorded when upload is enabled.
* BUGFIX: When the Debug Activity is used to log pings, each ping is now logged only once.
Timings are always measured, but only recorded when upload is enabled ([#400](https://github.com/mozilla/glean/pull/400))
* BUGFIX: When the Debug Activity is used to log pings, each ping is now logged only once ([#407](https://github.com/mozilla/glean/pull/407))
* New `invalid state` error, used in timespan recording ([#230](https://github.com/mozilla/glean/pull/230))

# v19.0.0 (2019-10-22)

Expand Down
1 change: 1 addition & 0 deletions docs/user/metrics/timespan.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ HistorySync.setRawNanos(duration)

* `invalid_value`
* If recording a negative timespan.
* `invalid_state`
* If starting a timer while a previous timer is running.
* If stopping a timer while it is not running.
* If trying to set a raw timespan while a timer is running.
Expand Down
3 changes: 3 additions & 0 deletions glean-core/src/error_recording.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ pub enum ErrorType {
InvalidValue,
/// For when the label of a labeled metric does not match the restrictions
InvalidLabel,
/// For when the metric caught an invalid state while recording
InvalidState,
}

impl ErrorType {
Expand All @@ -35,6 +37,7 @@ impl ErrorType {
match self {
ErrorType::InvalidValue => "invalid_value",
ErrorType::InvalidLabel => "invalid_label",
ErrorType::InvalidState => "invalid_state",
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions glean-core/src/metrics/timespan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl TimespanMetric {
record_error(
glean,
&self.meta,
ErrorType::InvalidValue,
ErrorType::InvalidState,
"Timespan already started",
None,
);
Expand All @@ -74,7 +74,7 @@ impl TimespanMetric {
record_error(
glean,
&self.meta,
ErrorType::InvalidValue,
ErrorType::InvalidState,
"Timespan not running",
None,
);
Expand Down Expand Up @@ -113,7 +113,7 @@ impl TimespanMetric {
record_error(
glean,
&self.meta,
ErrorType::InvalidValue,
ErrorType::InvalidState,
"Timespan already running. Raw value not recorded.",
None,
);
Expand Down Expand Up @@ -142,7 +142,7 @@ impl TimespanMetric {
record_error(
glean,
&self.meta,
ErrorType::InvalidValue,
ErrorType::InvalidState,
"Timespan value already recorded. New value discarded.",
None,
);
Expand Down
6 changes: 3 additions & 3 deletions glean-core/tests/timespan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ fn second_timer_run_is_skipped() {

// No error should be recorded here: we had no prior value stored.
assert!(
test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidValue, None).is_err()
test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidState, None).is_err()
);

let first_value = metric.test_get_value(&glean, "store1").unwrap();
Expand All @@ -135,7 +135,7 @@ fn second_timer_run_is_skipped() {
// new measurement was dropped.
assert_eq!(
Ok(1),
test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidValue, None)
test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidState, None)
);
}

Expand Down Expand Up @@ -284,6 +284,6 @@ fn set_raw_time_does_nothing_when_timer_running() {
// Make sure that the error has been recorded
assert_eq!(
Ok(1),
test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidValue, None)
test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidState, None)
);
}