Skip to content

Commit

Permalink
editoast: replace log with tracing
Browse files Browse the repository at this point in the history
With the objective to instrument `editoast` with OpenTelemetry,
`tracing` can bring some ease-of-use macros for instrumenting
functions.

This PR is a first step to move to `tracing` (only the log event generation).
  • Loading branch information
woshilapin committed Feb 19, 2024
1 parent d1f7533 commit 44a0cd1
Show file tree
Hide file tree
Showing 23 changed files with 51 additions and 35 deletions.
14 changes: 13 additions & 1 deletion editoast/Cargo.lock

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

2 changes: 1 addition & 1 deletion editoast/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ actix-web = "4.4.1"
actix-http = "3.5.1"
actix-cors = "0.7.0"
env_logger = "0.10.1"
log = "0.4.20"
tracing = { version = "0.1.40", features = [ "log" ] }
redis = { version = "0.24.0", features = [
"tokio-comp",
"connection-manager",
Expand Down
8 changes: 4 additions & 4 deletions editoast/src/converters/osm_to_railjson.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::utils::*;
use crate::{converters::generate_routes, schema::*};
use log::{error, info};
use tracing::{debug, error, info};

use std::{collections::HashMap, error::Error, path::PathBuf};
/// Run the osm-to-railjson subcommand
Expand Down Expand Up @@ -110,12 +110,12 @@ pub fn parse_osm(osm_pbf_in: PathBuf) -> Result<RailJson, Box<dyn Error + Send +
(4, 4) => railjson
.switches
.push(double_slip_switch(node, &adj.branches)),
_ => log::debug!("node {id} with {edges_count} edges and {branches_count} branches"),
_ => debug!("node {id} with {edges_count} edges and {branches_count} branches"),
}
}
log::debug!("Start generating routes");
debug!("Start generating routes");
railjson.routes = generate_routes::routes(&railjson);
log::debug!("Done, got {} routes", railjson.routes.len());
debug!("Done, got {} routes", railjson.routes.len());
Ok(railjson)
}

Expand Down
2 changes: 1 addition & 1 deletion editoast/src/converters/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ use std::collections::HashMap;

use crate::schema::utils::Identifier;
use crate::schema::*;
use log::{error, warn};
use osm4routing::{Coord, Edge, NodeId};
use osmpbfreader::Node;
use std::str::FromStr;
use tracing::{error, warn};

// Given an edge and a coordinate, returns the coordinates used to compute the angle
// It uses the nearest OpenStreetMap node, and the other as the the rails might do a loop
Expand Down
10 changes: 5 additions & 5 deletions editoast/src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ use async_trait::async_trait;
use colored::{ColoredString, Colorize};
use editoast_derive::EditoastError;
pub use http_client::{HttpClient, HttpClientBuilder};
use log::info;
use reqwest::Url;
use serde::{de::DeserializeOwned, Serialize};
use serde_derive::Deserialize;
use serde_json::Value;
use thiserror::Error;
use tracing::{debug, error, info};

#[cfg(test)]
use crate::core::mocking::MockingError;
Expand Down Expand Up @@ -93,8 +93,8 @@ impl CoreClient {
body: Option<&B>,
) -> Result<R::Response> {
let method_s = colored_method(&method);
log::info!(target: "editoast::coreclient", "{method_s} {path}");
log::debug!(target: "editoast::coreclient", "Request content: {body}", body = body.and_then(|b| serde_json::to_string_pretty(b).ok()).unwrap_or_default());
info!(target: "editoast::coreclient", "{method_s} {path}");
debug!(target: "editoast::coreclient", "Request content: {body}", body = body.and_then(|b| serde_json::to_string_pretty(b).ok()).unwrap_or_default());
match self {
CoreClient::Direct(client) => {
let mut i_try = 0;
Expand Down Expand Up @@ -129,11 +129,11 @@ impl CoreClient {
msg: err.to_string(),
})?;
if status.is_success() {
log::info!(target: "editoast::coreclient", "{method_s} {path} {status}", status = status.to_string().bold().green());
info!(target: "editoast::coreclient", "{method_s} {path} {status}", status = status.to_string().bold().green());
return R::from_bytes(bytes.as_ref());
}

log::error!(target: "editoast::coreclient", "{method_s} {path} {status}", status = status.to_string().bold().red());
error!(target: "editoast::coreclient", "{method_s} {path} {status}", status = status.to_string().bold().red());
Err(self.handle_error(bytes.as_ref(), status, url))
}
#[cfg(test)]
Expand Down
3 changes: 2 additions & 1 deletion editoast/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::{
error::Error,
fmt::{Display, Formatter},
};
use tracing::error;
use utoipa::ToSchema;
use validator::{ValidationErrors, ValidationErrorsKind};

Expand Down Expand Up @@ -104,7 +105,7 @@ impl ResponseError for InternalError {
}

fn error_response(&self) -> HttpResponse {
log::error!(
error!(
"[{}] {}: {}",
self.error_type.bold(),
self.message,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/generated_data/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ use diesel::sql_types::{Array, BigInt, Json, Text};
use diesel_async::{AsyncPgConnection as PgConnection, RunQueryDsl};
use futures_util::Future;
use itertools::Itertools;
use log::warn;
use serde_json::to_value;
use sha1::{Digest, Sha1};
use std::pin::Pin;
use tracing::warn;

use super::GeneratedData;
use crate::error::Result;
Expand Down
9 changes: 5 additions & 4 deletions editoast/src/generated_data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use psl_sign::PSLSignLayer;
use signal::SignalLayer;
use speed_section::SpeedSectionLayer;
use switch::SwitchLayer;
use tracing::debug;
use track_section::TrackSectionLayer;

use crate::error::Result;
Expand Down Expand Up @@ -86,13 +87,13 @@ pub async fn refresh_all(
// The other layers depend on track section layer.
// We must wait until its completion before running the other requests in parallel
TrackSectionLayer::refresh_pool(db_pool.clone(), infra, infra_cache).await?;
log::debug!("⚙️ Infra {infra}: track section layer is generated");
debug!("⚙️ Infra {infra}: track section layer is generated");
let mut conn = db_pool.get().await?;
// The analyze step significantly improves the performance when importing and generating together
// It doesn’t seem to make a different when the generation step is ran separately
// It isn’t clear why without analyze the Postgres server seems to run at 100% without halting
sql_query("analyze").execute(&mut conn).await?;
log::debug!("⚙️ Infra {infra}: database analyzed");
debug!("⚙️ Infra {infra}: database analyzed");
futures::try_join!(
SpeedSectionLayer::refresh_pool(db_pool.clone(), infra, infra_cache),
SignalLayer::refresh_pool(db_pool.clone(), infra, infra_cache),
Expand All @@ -105,10 +106,10 @@ pub async fn refresh_all(
NeutralSectionLayer::refresh_pool(db_pool.clone(), infra, infra_cache),
NeutralSignLayer::refresh_pool(db_pool.clone(), infra, infra_cache),
)?;
log::debug!("⚙️ Infra {infra}: object layers is generated");
debug!("⚙️ Infra {infra}: object layers is generated");
// The error layer depends on the other layers and must be executed at the end.
ErrorLayer::refresh_pool(db_pool.clone(), infra, infra_cache).await?;
log::debug!("⚙️ Infra {infra}: errors layer is generated");
debug!("⚙️ Infra {infra}: errors layer is generated");
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion editoast/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ use diesel_json::Json as DieselJson;
use futures_util::future::BoxFuture;
use futures_util::FutureExt;
use infra_cache::InfraCache;
use log::{error, info, warn};
use map::MapLayers;
use models::electrical_profiles::ElectricalProfileSet;
use models::infra::InfraError;
Expand All @@ -63,6 +62,7 @@ use std::io::{BufReader, IsTerminal};
use std::process::exit;
use std::{env, fs};
use thiserror::Error;
use tracing::{error, info, warn};
use url::Url;
use validator::{Validate, ValidationErrorsKind};
use views::infra::InfraApiError;
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/models/infra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ use diesel_async::{AsyncPgConnection as PgConnection, RunQueryDsl};
use editoast_derive::{EditoastError, Model};
use futures::future::try_join_all;
use futures::Future;
use log::{debug, error};
use serde::{Deserialize, Serialize};
use strum::IntoEnumIterator;
use thiserror::Error;
use tracing::{debug, error};
use uuid::Uuid;

pub const INFRA_VERSION: &str = "0";
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/buffer_stop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use super::{new_ref_fix_delete_pair, Fix};
use crate::schema::{
BufferStopCache, InfraError, InfraErrorType, OSRDObject as _, ObjectRef, ObjectType,
};
use log::debug;
use std::collections::HashMap;
use tracing::debug;

pub fn fix_buffer_stop(
buffer_stop: &BufferStopCache,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use super::{new_ref_fix_delete_pair, Fix};
use crate::schema::{
DetectorCache, InfraError, InfraErrorType, OSRDObject as _, ObjectRef, ObjectType,
};
use log::debug;
use std::collections::HashMap;
use tracing::debug;

pub fn fix_detector(
detector: &DetectorCache,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/electrifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use crate::schema::{
};
use itertools::Itertools as _;
use json_patch::{Patch, PatchOperation, RemoveOperation};
use log::{debug, error};
use std::collections::HashMap;
use tracing::{debug, error};

fn invalid_reference_to_ordered_operation(
electrification: &Electrification,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ use actix_web::web::{Data, Json as WebJson, Path};
use chashmap::CHashMap;
use editoast_derive::EditoastError;
use itertools::Itertools as _;
use log::{debug, error};
use std::collections::hash_map::{Entry, HashMap};
use thiserror::Error;
use tracing::{debug, error};

mod buffer_stop;
mod detector;
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/operational_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use super::{new_ref_fix_delete_pair, Fix};
use crate::schema::{
InfraError, InfraErrorType, OSRDObject as _, ObjectRef, OperationalPointCache,
};
use log::debug;
use std::collections::HashMap;
use tracing::debug;

pub fn fix_operational_point(
operational_point: &OperationalPointCache,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use super::{new_ref_fix_delete_pair, Fix};
use crate::schema::{
InfraError, InfraErrorType, OSRDIdentified as _, OSRDObject as _, ObjectRef, ObjectType, Route,
};
use log::debug;
use std::collections::HashMap;
use tracing::debug;

pub fn fix_route(
route: &Route,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use super::{new_ref_fix_delete_pair, Fix};
use crate::schema::{
InfraError, InfraErrorType, OSRDObject as _, ObjectRef, ObjectType, SignalCache,
};
use log::debug;
use std::collections::HashMap;
use tracing::debug;

pub fn fix_signal(
signal: &SignalCache,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/speed_section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use crate::schema::{
};
use itertools::Itertools as _;
use json_patch::{Patch, PatchOperation, RemoveOperation};
use log::{debug, error};
use std::collections::HashMap;
use tracing::{debug, error};

fn invalid_reference_to_ordered_operation(
speed_section: &SpeedSection,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use super::{new_ref_fix_delete_pair, Fix};
use crate::schema::{
InfraError, InfraErrorType, OSRDObject as _, ObjectRef, ObjectType, SwitchCache,
};
use log::debug;
use std::collections::HashMap;
use tracing::debug;

pub fn fix_switch(
switch: &SwitchCache,
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/infra/auto_fixes/track_section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use crate::schema::{
operation::RailjsonObject, utils::Identifier, BufferStop, Endpoint, InfraError, InfraErrorType,
OSRDIdentified as _, OSRDObject as _, ObjectRef, TrackSectionCache,
};
use log::debug;
use std::collections::HashMap;
use tracing::debug;
use uuid::Uuid;

pub fn fix_track_section(
Expand Down
3 changes: 2 additions & 1 deletion editoast/src/views/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use actix_web::{get, services};
use diesel::sql_query;
use redis::cmd;
use serde_derive::{Deserialize, Serialize};
use tracing::debug;
use utoipa::openapi::RefOr;
use utoipa::{OpenApi, ToSchema};

Expand Down Expand Up @@ -144,7 +145,7 @@ impl OpenApiRoot {

let routes = routes_v2();
for (path, path_item) in routes.paths.into_flat_path_list() {
log::debug!("processing {path}");
debug!("processing {path}");
if openapi.paths.paths.contains_key(&path) {
let existing_path_item = openapi.paths.paths.remove(&path).unwrap();
let merged = merge_path_items(existing_path_item, path_item);
Expand Down
7 changes: 4 additions & 3 deletions editoast/src/views/openapi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::{

use actix_web::dev::HttpServiceFactory;
use serde_json::Value as Json;
use tracing::warn;
use utoipa::{
openapi::{
path::PathItemBuilder, schema::AnyOf, AllOf, Array, Object, OneOf, PathItem, RefOr, Schema,
Expand Down Expand Up @@ -119,7 +120,7 @@ pub(super) fn merge_path_items(a: PathItem, b: PathItem) -> PathItem {
let mut operations: BTreeMap<_, _> = a.operations;
for (method, operation) in b.operations {
if operations.contains_key(&method) {
log::warn!(
warn!(
"duplicate operation for method {}",
serde_json::to_string(&method).unwrap() // PathItemType does not implement Display or Debug :(
);
Expand Down Expand Up @@ -487,7 +488,7 @@ impl OpenApiMerger {
.unwrap();
for (key, schema) in new_schemas.iter_mut() {
if old_schemas.insert(key.clone(), schema.clone()).is_some() {
log::warn!("duplicated schema replaced: {}", key);
warn!("duplicated schema replaced: {}", key);
}
}

Expand All @@ -506,7 +507,7 @@ impl OpenApiMerger {
let old_path = old_path.as_object_mut().unwrap();
for (method, operation) in path.as_object_mut().unwrap().iter_mut() {
if old_path.insert(method.clone(), operation.clone()).is_some() {
log::warn!("duplicated operation replaced: {} {}", method, key);
warn!("duplicated operation replaced: {} {}", method, key);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion editoast/src/views/pagination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ use diesel::sql_types::Untyped;
use diesel::{QueryResult, QueryableByName};
use diesel_async::methods::LoadQuery;
use diesel_async::AsyncPgConnection as PgConnection;
use log::warn;
use serde::Deserialize;
use serde::Serialize;
use thiserror::Error;
use tracing::warn;

use editoast_derive::EditoastError;
use utoipa::IntoParams;
Expand Down

0 comments on commit 44a0cd1

Please sign in to comment.