Skip to content

Commit

Permalink
merge master
Browse files Browse the repository at this point in the history
  • Loading branch information
pjenvey committed Dec 5, 2024
2 parents 6abde91 + c01021b commit 2837c58
Show file tree
Hide file tree
Showing 16 changed files with 159 additions and 113 deletions.
64 changes: 36 additions & 28 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 2 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,13 @@ regex = "1.4"
reqwest = { version = "0.12", default-features = false, features = [
"rustls-tls",
] }
# rev 1b65b5c includes https://github.com/getsentry/sentry-rust/pull/701
# TODO: bump to 0.35 as soon as it's available
sentry = { version = "0.34", git = "https://github.com/getsentry/sentry-rust", rev = "1b65b5c", default-features = false, features = [
sentry = { version = "0.35", default-features = false, features = [
"curl",
"backtrace",
"contexts",
"debug-images",
] }
sentry-backtrace = { version = "0.34", git = "https://github.com/getsentry/sentry-rust", rev = "1b65b5c" }
sentry-backtrace = "0.35"
serde = "1.0"
serde_derive = "1.0"
serde_json = { version = "1.0", features = ["arbitrary_precision"] }
Expand Down
42 changes: 19 additions & 23 deletions syncserver-db-common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ use http::StatusCode;
use syncserver_common::{from_error, impl_fmt_display, ReportableError};
use thiserror::Error;

/// Error specific to any MySQL database backend. These errors are not related to the syncstorage
/// Error specific to any SQL database backend. These errors are not related to the syncstorage
/// or tokenserver application logic; rather, they are lower-level errors arising from diesel.
#[derive(Debug)]
pub struct MysqlError {
kind: MysqlErrorKind,
pub struct SqlError {
kind: SqlErrorKind,
pub status: StatusCode,
pub backtrace: Backtrace,
}

#[derive(Debug, Error)]
enum MysqlErrorKind {
enum SqlErrorKind {
#[error("A database error occurred: {}", _0)]
DieselQuery(#[from] diesel::result::Error),

Expand All @@ -29,8 +29,8 @@ enum MysqlErrorKind {
Migration(diesel_migrations::RunMigrationsError),
}

impl From<MysqlErrorKind> for MysqlError {
fn from(kind: MysqlErrorKind) -> Self {
impl From<SqlErrorKind> for SqlError {
fn from(kind: SqlErrorKind) -> Self {
Self {
kind,
status: StatusCode::INTERNAL_SERVER_ERROR,
Expand All @@ -39,22 +39,22 @@ impl From<MysqlErrorKind> for MysqlError {
}
}

impl ReportableError for MysqlError {
impl ReportableError for SqlError {
fn is_sentry_event(&self) -> bool {
#[allow(clippy::match_like_matches_macro)]
match &self.kind {
MysqlErrorKind::Pool(_) => false,
SqlErrorKind::Pool(_) => false,
_ => true,
}
}

fn metric_label(&self) -> Option<String> {
Some(
match self.kind {
MysqlErrorKind::DieselQuery(_) => "storage.mysql.error.diesel_query",
MysqlErrorKind::DieselConnection(_) => "storage.mysql.error.diesel_connection",
MysqlErrorKind::Pool(_) => "storage.mysql.error.pool",
MysqlErrorKind::Migration(_) => "storage.mysql.error.migration",
SqlErrorKind::DieselQuery(_) => "storage.sql.error.diesel_query",
SqlErrorKind::DieselConnection(_) => "storage.sql.error.diesel_connection",
SqlErrorKind::Pool(_) => "storage.sql.error.pool",
SqlErrorKind::Migration(_) => "storage.sql.error.migration",
}
.to_string(),
)
Expand All @@ -65,21 +65,17 @@ impl ReportableError for MysqlError {
}
}

impl_fmt_display!(MysqlError, MysqlErrorKind);
impl_fmt_display!(SqlError, SqlErrorKind);

from_error!(
diesel::result::Error,
MysqlError,
MysqlErrorKind::DieselQuery
);
from_error!(diesel::result::Error, SqlError, SqlErrorKind::DieselQuery);
from_error!(
diesel::result::ConnectionError,
MysqlError,
MysqlErrorKind::DieselConnection
SqlError,
SqlErrorKind::DieselConnection
);
from_error!(diesel::r2d2::PoolError, MysqlError, MysqlErrorKind::Pool);
from_error!(diesel::r2d2::PoolError, SqlError, SqlErrorKind::Pool);
from_error!(
diesel_migrations::RunMigrationsError,
MysqlError,
MysqlErrorKind::Migration
SqlError,
SqlErrorKind::Migration
);
7 changes: 7 additions & 0 deletions syncserver/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,13 @@ impl Server {
&Metrics::from(&metrics),
blocking_threadpool.clone(),
)?;
// Spawns sweeper that calls Deadpool `retain` method, clearing unused connections.
db_pool.spawn_sweeper(Duration::from_secs(
settings
.syncstorage
.database_pool_sweeper_task_interval
.into(),
));
let glean_logger = Arc::new(GleanEventsLogger {
// app_id corresponds to probe-scraper entry.
// https://github.com/mozilla/probe-scraper/blob/main/repositories.yaml
Expand Down
51 changes: 25 additions & 26 deletions syncserver/src/web/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,34 +40,33 @@ pub async fn get_collections(
request: HttpRequest,
state: Data<ServerState>,
) -> Result<HttpResponse, ApiError> {
if state.glean_enabled {
// Values below are be passed to the Glean logic to emit metrics.
// This is used to measure DAU (Daily Active Use) of Sync.
let user_agent = request
.headers()
.get(header::USER_AGENT)
.and_then(|header| header.to_str().ok())
.unwrap_or("");
let device_info: DeviceInfo = get_device_info(user_agent);

state.glean_logger.record_events_ping(
&RequestInfo {
user_agent: user_agent.to_owned(),
ip_address: "".to_owned(),
},
&EventsPing {
syncstorage_device_family: device_info.device_family.to_string(),
syncstorage_hashed_device_id: meta.user_id.hashed_device_id.clone(),
syncstorage_hashed_fxa_uid: meta.user_id.hashed_fxa_uid.clone(),
syncstorage_platform: device_info.platform.to_string(),
event: Some(Box::new(SyncstorageGetCollectionsEvent {})),
},
);
}

db_pool
.transaction_http(request, |db| async move {
.transaction_http(request.clone(), |db| async move {
meta.emit_api_metric("request.get_collections");
if state.glean_enabled {
// Values below are be passed to the Glean logic to emit metrics.
// This is used to measure DAU (Daily Active Use) of Sync.
let user_agent = request
.headers()
.get(header::USER_AGENT)
.and_then(|header| header.to_str().ok())
.unwrap_or("");
let device_info: DeviceInfo = get_device_info(user_agent);

state.glean_logger.record_events_ping(
&RequestInfo {
user_agent: user_agent.to_owned(),
ip_address: "".to_owned(),
},
&EventsPing {
syncstorage_device_family: device_info.device_family.to_string(),
syncstorage_hashed_device_id: meta.user_id.hashed_device_id.clone(),
syncstorage_hashed_fxa_uid: meta.user_id.hashed_fxa_uid.clone(),
syncstorage_platform: device_info.platform.to_string(),
event: Some(Box::new(SyncstorageGetCollectionsEvent {})),
},
);
}
let result = db.get_collection_timestamps(meta.user_id).await?;

Ok(HttpResponse::build(StatusCode::OK)
Expand Down
2 changes: 1 addition & 1 deletion syncstorage-db-common/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{convert::TryInto, u64};
use std::convert::TryInto;

use chrono::{
offset::{FixedOffset, TimeZone, Utc},
Expand Down
Loading

0 comments on commit 2837c58

Please sign in to comment.