Skip to content

Commit

Permalink
Review Suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@samuchila
samuchila committed Feb 13, 2024
1 parent 9aa65ce commit 19d1c4b
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 57 deletions.
10 changes: 9 additions & 1 deletion api/res/openapi.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,17 @@ paths:
description: The number of log lines to retrieve. If not present, all the lines from `since` are retrieved.
schema:
type: integer
- in: query
name: asAttachment
description: >-
Determines how the response is presented by the browser. When `true`, the response content is provided as a downloadable attachment.
If `false` or not provided, the response is displayed inline.
schema:
type: boolean
responses:
'200':
description: The available log statements
description: |
The available log statements. MIME type `text/event-stream` supports streaming of logs.
headers:
Link:
schema:
Expand Down
33 changes: 18 additions & 15 deletions api/src/apps/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -742,35 +742,38 @@ Log msg 3 of service-a of app master
.stream_logs(&app_name, service.service_name(), &None, &None)
.await;

if let Some(Ok((_, log_line))) = log_stream.next().await {
assert_eq!(
log_line,
assert_eq!(
log_stream.next().await.unwrap().unwrap(),
(
DateTime::parse_from_rfc3339("2019-07-18T07:25:00.000000000Z").unwrap(),
format!(
"Log msg 1 of {} of app {app_name}\n",
service.service_name()
)
);
}
)
);

if let Some(Ok((_, log_line))) = log_stream.next().await {
assert_eq!(
log_line,
assert_eq!(
log_stream.next().await.unwrap().unwrap(),
(
DateTime::parse_from_rfc3339("2019-07-18T07:25:00.000000000Z").unwrap(),
format!(
"Log msg 2 of {} of app {app_name}\n",
service.service_name()
)
);
}
)
);

if let Some(Ok((_, log_line))) = log_stream.next().await {
assert_eq!(
log_line,
assert_eq!(
log_stream.next().await.unwrap().unwrap(),
(
DateTime::parse_from_rfc3339("2019-07-18T07:25:00.000000000Z").unwrap(),
format!(
"Log msg 3 of {} of app {app_name}\n",
service.service_name()
)
);
}
)
);
}

Ok(())
Expand Down
105 changes: 73 additions & 32 deletions api/src/apps/routes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ use crate::models::service::{Service, ServiceStatus};
use crate::models::ServiceConfig;
use crate::models::{AppName, AppNameError, LogChunk};
use crate::models::{AppStatusChangeId, AppStatusChangeIdError};
use chrono::{DateTime, Utc};
use chrono::DateTime;
use futures::StreamExt;
use http_api_problem::{HttpApiProblem, StatusCode};
use multimap::MultiMap;
use regex::Regex;
use rocket::http::hyper::header::CONTENT_DISPOSITION;
use rocket::http::hyper::header::LINK;
use rocket::http::{RawStr, Status};
use rocket::request::{FromRequest, Outcome, Request};
use rocket::response::stream::{Event, EventStream};
Expand Down Expand Up @@ -189,22 +190,16 @@ async fn change_status(
Ok(ServiceStatusResponse { service })
}

#[get(
"/<app_name>/logs/<service_name>?<since>&<limit>&<download>",
format = "text/html",
rank = 2
)]
#[get("/<app_name>/logs/<service_name>?<log_query..>", format = "text/plain")]
async fn logs<'r>(
app_name: Result<AppName, AppNameError>,
service_name: &'r str,
since: Option<String>,
limit: Option<usize>,
download: bool,
log_query: LogQuery,
apps: &State<Arc<Apps>>,
) -> HttpResult<LogsResponse<'r>> {
let app_name = app_name?;

let since = match since {
let since = match log_query.since {
None => None,
Some(since) => match DateTime::parse_from_rfc3339(&since) {
Ok(since) => Some(since),
Expand All @@ -219,32 +214,31 @@ async fn logs<'r>(
};

let log_chunk = apps
.get_logs(&app_name, service_name, &since, &limit)
.get_logs(&app_name, service_name, &since, &log_query.limit)
.await?;

Ok(LogsResponse {
log_chunk,
app_name,
service_name,
limit,
download,
limit: log_query.limit,
as_attachment: log_query.as_attachment,
})
}

#[get(
"/<app_name>/logs/<service_name>?<since>&<limit>",
"/<app_name>/logs/<service_name>?<log_query..>",
format = "text/event-stream",
rank = 3
rank = 2
)]
async fn stream_logs<'r>(
app_name: Result<AppName, AppNameError>,
service_name: &'r str,
limit: Option<usize>,
since: Option<String>,
log_query: LogQuery,
apps: &'r State<Arc<Apps>>,
) -> HttpResult<EventStream![Event + 'r]> {
let app_name = app_name.unwrap();
let since = match since {
let since = match &log_query.since {
None => None,
Some(since) => match DateTime::parse_from_rfc3339(&since) {
Ok(since) => Some(since),
Expand All @@ -260,7 +254,7 @@ async fn stream_logs<'r>(

Ok(EventStream! {
let mut log_chunk = apps
.stream_logs(&app_name, service_name, &since, &limit)
.stream_logs(&app_name, service_name, &since, &log_query.limit)
.await;

while let Some(result) = log_chunk.as_mut().next().await {
Expand Down Expand Up @@ -319,7 +313,15 @@ pub struct LogsResponse<'a> {
app_name: AppName,
service_name: &'a str,
limit: Option<usize>,
download: bool,
as_attachment: bool,
}

#[derive(FromForm)]
struct LogQuery {
since: Option<String>,
limit: Option<usize>,
#[field(name = "asAttachment")]
as_attachment: bool,
}

#[derive(FromForm)]
Expand Down Expand Up @@ -350,7 +352,7 @@ impl<'r> Responder<'r, 'static> for LogsResponse<'r> {
Some(log_chunk) => log_chunk,
};

let from = *log_chunk.until() + chrono::Duration::seconds(1);
let from = *log_chunk.until() + chrono::Duration::milliseconds(1);

let next_logs_url = match self.limit {
Some(limit) => format!(
Expand All @@ -368,20 +370,20 @@ impl<'r> Responder<'r, 'static> for LogsResponse<'r> {
),
};

let content_dispositon_value = if self.download {
let content_dispositon_value = if self.as_attachment {
format!(
"attachment; filename=\"{}_{}_{}.txt\"",
self.app_name,
self.service_name,
Utc::now().format("%Y%m%d_%H%M%S")
log_chunk.until().format("%Y%m%d_%H%M%S")
)
} else {
format!("inline")
String::from("inline")
};

let log_lines = log_chunk.log_lines();
Response::build()
.raw_header("Link", format!("<{}>;rel=next", next_logs_url))
.raw_header(LINK.as_str(), format!("<{}>;rel=next", next_logs_url))
.raw_header(CONTENT_DISPOSITION.as_str(), content_dispositon_value)
.sized_body(log_lines.len(), Cursor::new(log_lines.clone()))
.ok()
Expand Down Expand Up @@ -601,9 +603,9 @@ mod tests {
use crate::models::{AppName, AppStatusChangeId};
use crate::sc;
use assert_json_diff::assert_json_include;
use rocket::http::ContentType;
use rocket::http::Header;
use rocket::http::Status;
use rocket::http::{Accept, ContentType};
use rocket::local::asynchronous::Client;
use serde_json::json;
use serde_json::Value;
Expand Down Expand Up @@ -838,46 +840,85 @@ mod tests {
}

#[tokio::test]
async fn render_log_weblink_with_no_limit() -> Result<(), crate::apps::AppsServiceError> {
async fn log_weblink_with_no_limit() -> Result<(), crate::apps::AppsServiceError> {
let (host_meta_cache, mut _host_meta_crawler) = crate::host_meta_crawling();

let client =
set_up_rocket_with_dummy_infrastructure_and_a_running_app(host_meta_cache).await?;

let response = client
.get("/api/apps/master/logs/service-a")
.header(ContentType::Text)
.header(Accept::Text)
.dispatch()
.await;
let mut link_header = response.headers().get("Link");
assert_eq!(
link_header.next(),
Some(
"</api/apps/master/logs/service-a?since=2019-07-18T07:35:01%2B00:00>;rel=next"
"</api/apps/master/logs/service-a?since=2019-07-18T07:35:00.001%2B00:00>;rel=next"
)
);
Ok(())
}

#[tokio::test]
async fn render_log_weblink_with_some_limit() -> Result<(), crate::apps::AppsServiceError> {
async fn log_weblink_with_some_limit() -> Result<(), crate::apps::AppsServiceError> {
let (host_meta_cache, mut _host_meta_crawler) = crate::host_meta_crawling();

let client =
set_up_rocket_with_dummy_infrastructure_and_a_running_app(host_meta_cache).await?;

let response = client
.get("/api/apps/master/logs/service-a?limit=20000&since=2019-07-22T08:42:47-00:00")
.header(ContentType::Text)
.header(Accept::Text)
.dispatch()
.await;
let mut link_header = response.headers().get("Link");
assert_eq!(
link_header.next(),
Some("</api/apps/master/logs/service-a?limit=20000&since=2019-07-18T07:35:01%2B00:00>;rel=next")
Some("</api/apps/master/logs/service-a?limit=20000&since=2019-07-18T07:35:00.001%2B00:00>;rel=next")
);
Ok(())
}

#[tokio::test]
async fn log_content_disposition_for_downloading_as_attachment(
) -> Result<(), crate::apps::AppsServiceError> {
let (host_meta_cache, mut _host_meta_crawler) = crate::host_meta_crawling();

let client =
set_up_rocket_with_dummy_infrastructure_and_a_running_app(host_meta_cache).await?;

let response = client
.get("/api/apps/master/logs/service-a?limit=20000&since=2019-07-22T08:42:47-00:00&asAttachment=true")
.header(Accept::Text)
.dispatch()
.await;
let mut link_header = response.headers().get("Content-Disposition");
assert_eq!(
link_header.next(),
Some("attachment; filename=\"master_service-a_20190718_073500.txt\"")
);
Ok(())
}

#[tokio::test]
async fn log_content_disposition_for_displaying_as_inline(
) -> Result<(), crate::apps::AppsServiceError> {
let (host_meta_cache, mut _host_meta_crawler) = crate::host_meta_crawling();

let client =
set_up_rocket_with_dummy_infrastructure_and_a_running_app(host_meta_cache).await?;

let response = client
.get("/api/apps/master/logs/service-a?limit=20000&since=2019-07-22T08:42:47-00:00")
.header(Accept::Text)
.dispatch()
.await;
let mut link_header = response.headers().get("Content-Disposition");
assert_eq!(link_header.next(), Some("inline"));
Ok(())
}
}

mod http_api_error {
Expand Down
13 changes: 4 additions & 9 deletions frontend/src/LogsDialog.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
:errorStatusText="this.errorText">
<template v-slot:body>
<div class="d-flex justify-content-end align-items-center">
<alert type="alert" class="alert-primary mr-auto" v-if="scrollPosition" role="alert">
<alert type="alert" class="alert-primary mr-auto" v-if="scrollPosition === 0" role="alert">
Maximum Log View Reached. For complete log details, please use the 'Download Full Logs' button.
</alert>
<a :href="downloadLink" class="btn btn-primary ml-auto" download="filename.txt"><font-awesome-icon
Expand Down Expand Up @@ -85,7 +85,7 @@
return {
logLines: [],
eventSource: null,
scrollPosition: false,
scrollPosition: null,
nextPageLink: null,
errorText: '',
};
Expand All @@ -101,7 +101,7 @@
return `/api/apps/${this.$route.params.app}/logs/${this.$route.params.service}?since=${since}`;
},
downloadLink() {
return `/api/apps/${this.$route.params.app}/logs/${this.$route.params.service}?download=true`;
return `/api/apps/${this.$route.params.app}/logs/${this.$route.params.service}?asAttachment=true`;
},
},
mounted() {
Expand Down Expand Up @@ -187,12 +187,7 @@
},
handleScroll() {
const el = this.$refs.scroller.$el;
if (el.scrollTop === 0) {
this.scrollPosition = true;
} else {
this.scrollPosition = false;
}
this.scrollPosition = this.$refs.scroller.$el.scrollTop;
},
},
};
Expand Down

0 comments on commit 19d1c4b

Please sign in to comment.