Repository navigation
feat(dgw): search slog artifacts across recordings #2112
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
Open
irvingouj@Devolutions (irvingoujAtDevolution)
wants to merge
19
commits into
master
from
feat/recording-search
+1,241
−2
Open
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
9fc9af1
feat(dgw): search session logs across recordings
irvingoujAtDevolution ce252c8
fix(dgw): measure search log lines like the log viewer
irvingoujAtDevolution 0474064
fix(dgw): inline "all" in the recording selection schema
irvingoujAtDevolution 9d0e9c8
fix(dgw): read at most 16 MiB of logs per search
irvingoujAtDevolution 9fcadd6
feat(dgw): search the raw text of slog artifacts
irvingoujAtDevolution 632ee1f
docs(dgw): state the recording search intent
irvingoujAtDevolution 7d69f98
fix(dgw): list the start time range in the OpenAPI schemas
irvingoujAtDevolution ce74751
fix(dgw): match the search query as sent and fix limit edges
irvingoujAtDevolution 27db4ed
fix(dgw): fold the case of a sigma and search each log once
irvingoujAtDevolution f35fc7f
fix(dgw): list each recording once and lowercase lines quickly
irvingoujAtDevolution 56c3533
feat(dgw): reject unknown fields in a recording search
irvingoujAtDevolution 8acff8f
feat(dgw): search the string values of JSON log lines
irvingoujAtDevolution 2b93139
docs(dgw): state the value matching rule in the search intent
irvingoujAtDevolution 862d068
test(dgw): check unescaping, arrays and null in recording search
irvingoujAtDevolution 4918b94
test(dgw): pin that a cut line still parses in recording search
irvingoujAtDevolution 8da08df
test(dgw): match an escaped value in a cut line in recording search
irvingoujAtDevolution 3ee0497
docs(dgw): wrap the cut line sentence at the line limit
irvingoujAtDevolution 880576a
docs(dgw): state the search intent reasons as rationale
irvingoujAtDevolution ca3fb61
style(dgw): wrap a recording search test line as rustfmt does
irvingoujAtDevolution File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| ## 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. | ||
|
|
||
| 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. | ||
| 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`. | ||
| The rule never depends on field names, so a producer can change the fields it writes without any change to search. | ||
| Per-field search, such as matching an entry's `description` only, is not supported: the fields a producer writes are unknown to Gateway, which only cares about values. | ||
|
|
||
| ## Hits | ||
|
|
||
| A hit is a line that matches the query. | ||
| A hit carries the recording ID, the file name, the line number counting from 1, the byte offset of the line, and the raw line itself as a string. | ||
| Line numbers count from 1 like the slog package in `webapp/packages/session-recording-log`, so that a hit and that package's own search give a line the same number. | ||
| Gateway does not require a line to be JSON: a `.slog` is NDJSON by format, but a line holds whatever its producer pushed, such as a half-written last line while the push is live. | ||
| A line longer than 256 KiB is matched on its first 256 KiB and returned cut, with `truncated: true`; the caller fetches the file for the whole line. | ||
|
|
||
| ## Access | ||
|
|
||
| A valid token with the `gateway.recordings.search` scope is the only check, and the provisioner that issues the token decides who gets one. | ||
| Search has its own scope because `gateway.recordings.read` only lists recording IDs and must not reveal log contents. | ||
|
|
||
| ## Request | ||
|
|
||
| `recordings` is `"all"` or a list of recording IDs, and stays open to new forms added without breaking existing callers. | ||
| `"all"` orders recordings newest first by start time, then by ID, so that the order is the same on every request; a list keeps its order and searches a repeated ID once. | ||
|
|
||
| The optional `startTime` range takes RFC 3339 date-times and keeps a recording when `from <= startTime < to`, using the start time in `recording.json`, never anything inside a log. | ||
| A missing end leaves that side of the range open. | ||
|
|
||
| ## Limits | ||
|
|
||
| One search reads about 16 MiB of logs at most and returns at most 1,000 hits, so that searching every recording takes a bounded time. | ||
|
|
||
| ## 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. | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: Sounds to me that searching into .slog files that are in
filescould be useful as well, but I also get why we could prefer avoiding that. Just curious: did you consider how this feature could be extended if we wanted to support for search sources?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up question: Also this intent reads as we take all the .slog files in artifacts, with no regards for the "category"; one category is
ai-transcriptIIRC. Did you consider having requests explicitly select based on that? E.g.: search all AI transcripts (as opposed to search everything, including any new type of artifact we may add in the future)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you, we only have ai transcript for now however. We can certainly extend it further if we have a second type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'd like to keep it scoped for now, only implement what we need. If further, as product request grows, we can easily extend the request body to include different kind of files.