Skip to content
Open
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
9 changes: 5 additions & 4 deletions devolutions-gateway/src/api/jrec/search.intent.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
## What is searched

Search reads only the files listed under `artifacts` in `recording.json` whose type is `slog`.
Recording files listed under `files` are not searched, a `.slog` recording included.
Search covers the `.slog` files listed in `recording.json`, under `files` as well as under `artifacts`.
`files` is searched so that Active Directory Console logs, which are pushed as recordings, are searchable, the ones already stored included, with no change to how they are pushed.
Within a recording, the `.slog` files listed under `files` are searched first, then those listed under `artifacts`, by convention, in the order `recording.json` lists them and a recording pull zips them.
Each log is searched once, even when the request repeats its recording ID or the manifest lists it twice.
Search finds a recording and its logs where a recording pull does: in the folder named by the hyphenated recording ID, and only under a file name the pull accepts, so a log listed as `../outside.slog` is not searched.
A log that is missing or cannot be read is skipped and the search goes on, as `rg` and `grep` go on past a file they cannot open.

The query is matched literally, as `rg -F` and `grep -F` do, so its spaces are part of it and an empty query matches every line.
A line holding a JSON object is matched case-insensitively against the object's string values, at any depth and once unescaped; field names, numbers, booleans and null never match.
Any other line is matched case-insensitively against its raw text, so that any kind of artifact, present or future, can be searched.
Any other line is matched case-insensitively against its raw text, so that any kind of log, present or future, can be searched.
A line cut at 256 KiB follows the same rule: what was read is matched by its string values when it parses as a JSON object, and as raw text otherwise.

Values are matched rather than raw text because users search the text they see: raw matching would make a field name such as `description` match every line, and would miss text that JSON escapes, such as `DOMAIN\user` stored as `DOMAIN\\user`.
Expand Down Expand Up @@ -57,4 +58,4 @@ A log replaced while paging, as an AI analysis rerun does, is read from the curs
## Later

Regular expressions or wildcards, a total hit count, and combining words are wanted later, and the request shape must not preclude them.
Choosing which artifact kinds to search, or searching other sources such as the `.slog` recordings under `files`, is wanted later; the request shape must not preclude it either.
Choosing which file types to search, such as `.cast` besides `.slog`, is wanted later; the request shape must not preclude it either.
58 changes: 47 additions & 11 deletions devolutions-gateway/src/api/jrec/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ pub(crate) struct RecordingSearchResponse {
#[derive(Serialize)]
#[serde(rename_all = "camelCase")]
pub(crate) struct RecordingSearchHit {
/// The recording whose artifact holds the line.
/// The recording whose log holds the line.
recording_id: Uuid,
/// Name of the `.slog` artifact holding the line, as listed in `recording.json`.
/// Name of the `.slog` file holding the line, as listed in `recording.json`.
file_name: String,
/// Number of the line in the file, counting from 1.
#[cfg_attr(feature = "openapi", schema(minimum = 1))]
Expand All @@ -161,15 +161,15 @@ pub(crate) struct RecordingSearchHit {
truncated: bool,
}

/// Searches the `.slog` artifacts, such as AI analysis logs, of recordings stored on this instance
/// Searches the `.slog` files of recordings stored on this instance, such as Active Directory Console logs
///
/// The query is a case-insensitive substring, and an empty query matches every line.
/// In a line holding a JSON object, it is matched against the string values of the object at any depth, as they read
/// once unescaped, so field names, numbers, booleans and null never match.
/// Any other line, such as one still being written, is matched as raw text.
/// A line longer than 256 KiB is matched on its first 256 KiB only, by the same rule, so a cut JSON object is usually
/// matched as raw text.
/// Only the files listed under `artifacts` in `recording.json` with the `slog` type are searched.
/// Only the files listed in `recording.json` with the `slog` type are searched, under `files` and under `artifacts`.
/// Recordings without a readable `recording.json` and IDs with no recording on this instance give no hits.
///
/// Results come in pages.
Expand Down Expand Up @@ -773,24 +773,32 @@ impl Search {
}
}

/// The `.slog` artifacts of one recording.
/// The `.slog` files of one recording.
struct RecordingLogs {
recording_id: Uuid,
dir: Utf8PathBuf,
start_time: i64,
file_names: Vec<String>,
}

/// Lists the `.slog` artifacts of a recording, such as its AI analysis, from `recording.json`.
/// Lists the `.slog` files of a recording from `recording.json`, those under `files` first, then the artifacts.
async fn read_recording_logs(dir: Utf8PathBuf, recording_id: Uuid) -> Option<RecordingLogs> {
#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
struct Manifest {
start_time: i64,
#[serde(default)]
files: Vec<ManifestFile>,
#[serde(default)]
artifacts: JrecArtifacts,
}

#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
struct ManifestFile {
file_name: String,
}

let path = dir.join("recording.json");

// Every search reads every manifest again, so a failure here repeats on each search.
Expand All @@ -806,9 +814,10 @@ async fn read_recording_logs(dir: Utf8PathBuf, recording_id: Uuid) -> Option<Rec
// A log listed twice would be searched twice.
let mut seen = HashSet::new();
let file_names = manifest
.artifacts
.into_file_names()
.files
.into_iter()
.map(|file| file.file_name)
.chain(manifest.artifacts.into_file_names())
.filter(|file_name| {
is_safe_recording_file_name(file_name)
&& Utf8Path::new(file_name)
Expand Down Expand Up @@ -1360,20 +1369,47 @@ mod tests {
assert_eq!(Cursor::decode(&cursor).is_some(), accepted);
}

// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §What is searched
#[rstest]
#[case::twice_in_artifacts(
r#"{"startTime":100,"artifacts":{"ai-analysis":[{"fileName":"ai-analysis-0.slog"},{"fileName":"ai-analysis-0.slog"}]}}"#
)]
#[case::in_files_and_in_artifacts(
r#"{"startTime":100,"files":[{"fileName":"ai-analysis-0.slog"}],"artifacts":{"ai-analysis":[{"fileName":"ai-analysis-0.slog"}]}}"#
)]
#[tokio::test]
async fn lists_a_log_once_when_the_manifest_repeats_it(#[case] manifest: &str) {
let root = tempfile::tempdir().expect("temp dir");
std::fs::write(root.path().join("recording.json"), manifest).expect("write manifest");
let dir = Utf8PathBuf::from_path_buf(root.path().to_owned()).expect("UTF-8 path");

let recording = read_recording_logs(dir, Uuid::nil()).await.expect("readable manifest");

assert_eq!(recording.file_names, ["ai-analysis-0.slog"]);
}

// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §What is searched
#[tokio::test]
async fn lists_a_log_once_when_the_manifest_repeats_it() {
async fn lists_logs_in_manifest_order_files_first() {
let root = tempfile::tempdir().expect("temp dir");
std::fs::write(
root.path().join("recording.json"),
r#"{"startTime":100,"artifacts":{"ai-analysis":[{"fileName":"ai-analysis-0.slog"},{"fileName":"ai-analysis-0.slog"}]}}"#,
r#"{"startTime":100,"files":[{"fileName":"recording-1.slog"},{"fileName":"recording-0.slog"}],"artifacts":{"ai-analysis":[{"fileName":"ai-analysis-1.slog"},{"fileName":"ai-analysis-0.slog"}]}}"#,
)
.expect("write manifest");
let dir = Utf8PathBuf::from_path_buf(root.path().to_owned()).expect("UTF-8 path");

let recording = read_recording_logs(dir, Uuid::nil()).await.expect("readable manifest");

assert_eq!(recording.file_names, ["ai-analysis-0.slog"]);
assert_eq!(
recording.file_names,
[
"recording-1.slog",
"recording-0.slog",
"ai-analysis-1.slog",
"ai-analysis-0.slog"
]
);
}

/// Lists the recordings of a root holding a folder per name, each with a manifest that starts at the same time.
Expand Down
19 changes: 14 additions & 5 deletions testsuite/tests/cli/dgw/recording_search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ fn write_analysis_recording(root: &Path, id: Uuid, start_time: i64, log: &str) -
}

/// Two recordings whose AI analysis logs hold the searched text.
/// The older one also holds it in a recorded `.slog`, in its video, and in a log outside the recording folder that
/// its manifest points to, to show that only the `.slog` artifacts of a recording are searched.
/// The older one also holds it in its recorded `.slog`, in its video, and in a log outside the recording folder that
/// its manifest lists under both `files` and `artifacts`, to show that every `.slog` of a recording is searched, and
/// nothing else.
/// Its manifest also lists a log that is gone, and a third recording folder has no manifest, to show that neither
/// fails the search.
fn write_recordings(root: &Path) -> anyhow::Result<()> {
Expand All @@ -81,6 +82,7 @@ fn write_recordings(root: &Path) -> anyhow::Result<()> {
"files": [
{ "fileName": "recording-0.slog", "startTime": 100, "duration": 10 },
{ "fileName": "recording-1.webm", "startTime": 100, "duration": 10 },
{ "fileName": "../outside.slog", "startTime": 100, "duration": 10 },
],
"artifacts": {
"ai-analysis": [
Expand Down Expand Up @@ -200,7 +202,7 @@ fn hit_summaries(body: &serde_json::Value) -> anyhow::Result<Vec<(String, String
// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §What is searched
// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §Request
#[tokio::test]
async fn searches_the_slog_artifacts_of_every_recording_newest_first() -> anyhow::Result<()> {
async fn searches_the_slog_files_of_every_recording_newest_first() -> anyhow::Result<()> {
let (_recordings, config_handle, _process) = start_with_recordings().await?;

let response = search(
Expand All @@ -216,6 +218,7 @@ async fn searches_the_slog_artifacts_of_every_recording_newest_first() -> anyhow
hit_summaries(&body)?,
[
(NEWER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 1),
(OLDER_RECORDING.to_string(), "recording-0.slog".to_owned(), 1),
(OLDER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 2),
]
);
Expand All @@ -224,6 +227,7 @@ async fn searches_the_slog_artifacts_of_every_recording_newest_first() -> anyhow
Ok(())
}

// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §What is searched
// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §Request
#[tokio::test]
async fn searches_each_listed_recording_once() -> anyhow::Result<()> {
Expand All @@ -243,7 +247,10 @@ async fn searches_each_listed_recording_once() -> anyhow::Result<()> {
let body: serde_json::Value = response.json().await?;
assert_eq!(
hit_summaries(&body)?,
[(OLDER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 2)]
[
(OLDER_RECORDING.to_string(), "recording-0.slog".to_owned(), 1),
(OLDER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 2),
]
);

Ok(())
Expand Down Expand Up @@ -336,6 +343,7 @@ async fn pages_through_recordings_that_start_together_highest_id_first() -> anyh
Ok(())
}

// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §What is searched
// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §Request
// oracle: intent devolutions-gateway/src/api/jrec/search.intent.md §Pages
#[tokio::test]
Expand All @@ -352,10 +360,11 @@ async fn pages_through_listed_recordings_in_their_order() -> anyhow::Result<()>
)
.await?;

assert_eq!(pages.len(), 2);
assert_eq!(pages.len(), 3);
assert_eq!(
hit_summaries_of(&pages)?,
[
(OLDER_RECORDING.to_string(), "recording-0.slog".to_owned(), 1),
(OLDER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 2),
(NEWER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 1),
]
Expand Down
Loading