Skip to content

feat(logs): global search #3906

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

Merged
merged 7 commits into from
Apr 25, 2025

Conversation

bodinsamuel
Copy link
Collaborator

@bodinsamuel bodinsamuel commented Apr 15, 2025

Changes

Fixes https://linear.app/nango/issue/NAN-3039/top-level-log-search

  • Logs global search

You will see some comments in the PR, but basically, the main challenge is that it's a two-step process. First, we do a regular search on operations with the filters (if any), and then we search inside the operationIds that were found.

Doing post-filtering instead of pre-filtering saves a lot of headaches for pagination and security, but it's not perfect.
Since the pagination is working (we have a valid cursor) but the limit might be incorrect (we asked for 20 and got less) the UI will ask for more due to the infinite scroll.
To prevent excessive HTTP calls, I have added a manual mode. As soon as you type, we stop the live refresh and disable infinite scroll. It will still try to load as much as it can when we possibly, but the initial load is always manual.

I'm actually surprised the PR is that small, hopefully it's not bringing too much bugs 🙏🏻

Screen.Recording.2025-04-24.at.15.52.06.mov

@bodinsamuel bodinsamuel self-assigned this Apr 15, 2025
Copy link

linear bot commented Apr 24, 2025

@bodinsamuel bodinsamuel marked this pull request as ready for review April 24, 2025 14:16
@bodinsamuel bodinsamuel requested a review from a team April 24, 2025 14:16
Copy link

@mrge-io mrge-io bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mrge found 1 issue across 6 files. View it in mrge.io

Copy link
Collaborator

@TBonnin TBonnin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested locally. worked well.
The total logs found (top right corner) is not matching the logs being showed. Showing the total amount when the search is actually filtering rows. Is that on purpose to always show the total, regardless of the search

I would also expect the search to "follow me" when opening an operation panel. If I search for something and then click on an operation that has lots of logs. I must search again to only see the specific log entries that matches what I am looking for

@bodinsamuel
Copy link
Collaborator Author

The total logs found (top right corner) do not match the. Showing the total amount when the search is actually filtering rows. Is that on purpose to always show the total, regardless of the search

Not really on purpose but it's because I don't have a way to have actual accurate numbers. The only way to fix all issues regarding count and pagination would be to:

  • do a first search, getting all operations that match the filters (I mean all that could be 10K)
  • Search in all of those operationsIds, getting a list of operations IDs that can match the search
  • do a third search to get the first 20 operations that match both

Not sure if worth it tbh

I would also expect the search to "follow me" when opening an operation panel. If I search for something and then click on an operation that has lots of logs. I must search again to only see the specific log entries that matches what I am looking for

Yes good feedback, I'm planning on doing that just after this PR ☺️

@bodinsamuel bodinsamuel enabled auto-merge (squash) April 25, 2025 09:20
@bodinsamuel bodinsamuel merged commit c519178 into master Apr 25, 2025
16 checks passed
@bodinsamuel bodinsamuel deleted the sam/25_04_15/feat/logs-search-in-all-logs branch April 25, 2025 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants