Skip to content

feat(cli): allow specifying log filters #6513

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Mar 6, 2025
Merged

feat(cli): allow specifying log filters #6513

merged 6 commits into from
Mar 6, 2025

Conversation

georgijd-form3
Copy link
Contributor

@georgijd-form3 georgijd-form3 commented Mar 3, 2025 •
edited
Loading

Filter CLI logs by span prefix. Multiple filters can be specified and
exclusion takes precedence.

For example:

  • to print only pod logs: tilt up --log-filters 'pod'
  • to exclude pod logs: tilt up --log-filters '!pod'
  • to print only pod and build logs: tilt up --log-filters 'pod' --log-filters 'build' or tilt up --log-filters 'pod,build'
  • to exclude pod and build logs: tilt up --log-filters '!pod' --log-filters '!build' or tilt up --log-filters '!pod,!build'
  • to exclude all pod logs except one: tilt up --log-filters '!pod' --log-filters 'pod:my-namespace:my-pod' or tilt up --log-filters '!pod,pod:my-namespace:my-pod'

Also fix a golangci-lint (errcheck) error which was not introduced by this change.

@georgijd-form3 georgijd-form3 force-pushed the cli-log-filters branch 8 times, most recently from 6b585a0 to ab4ecb9 Compare March 3, 2025 14:19
@mateusz-drzymala-form3
Copy link

+1 Great idea !
This is very useful especially combined with the tilt ci command. It's very hard to debug issues when there are many resources. Restricting the resources that can print logs to stdout will make our life much easier !

@georgijd-form3
feat(cli): allow specifying log filters
996d983 
Filter CLI logs by span prefix. Multiple filters can be specified and
exclusion take precedence.

Signed-off-by: Georgi Dimitrov <georgi.dimitrov@form3.tech>
@georgijd-form3
style(lint): fix a linter error in internal/watch
fe7fd2c 
Check function return values

Signed-off-by: Georgi Dimitrov <georgi.dimitrov@form3.tech>
georgijd-form3 and others added 2 commits March 3, 2025 14:54
nicks
nicks reviewed Mar 5, 2025
View reviewed changes
Copy link
Member

@nicks nicks left a comment

Choose a reason for hiding this comment

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

hi! thanks for the contribution!

i like the overall motivation and direction

i think my major concern with this PR is that we already have a log filtering system in the UI. This system is similar but subtly different.

What do you think of modifying it to work more like the UI's filtering system? in other words: filter by log type (all vs runtime vs build), filter by resource

span ids were always kind of an internal implementation detail that's not exposed anywhere else, so filtering by span id seems kinda weird to me.

@georgijd-form3
Copy link
Contributor Author

georgijd-form3 commented Mar 5, 2025 •
edited
Loading

Hi, yes, I figured using the span IDs would be controversial but I chose it as it provides the greatest flexibility. I am happy to refactor this to be more in line with the UI log filtering. Let me see what I can come up with.

Are these the features that I should limit it to?

  1. Ability to display one of: all logs, runtime logs, build logs (--log-source)
    Question: Does build mean all logs except for runtime logs or does it mean logs specifically with the build:* and cmdimage:* span IDs (that's what the UI filtering is doing)? This is not ideal as selecting build logs means tiltfile logs will be omitted, for example, and they are quite useful.
  2. Ability to filter logs for a single resource (--log-manifest)
    Question: How should we pass resources? Using something like <type>:<namespace>:<name> or a manifest name (that's what the UI filtering is doing)?
  3. Ability to filter logs for a specific level (WARN or ERROR) (--log-level)
  4. Ability to filter logs for a term (text) (--log-term)

@nicks
Copy link
Member

nicks commented Mar 5, 2025

re: 1) ya, right now, build log includes build:* and cmdimage:*. This has evolved over time. Sure, making them include tiltfile:* sounds reasonable but we should update the UI as well

re: 2) let's use --log-resource to match other cli terminology (like tilt enable)

re: 4) I'd probably skip --log-term because grep exists in the cli?

@georgijd-form3
refactor(cli): mimic UI log filtering
7db08b8 
Match the UI log filtering implementation as closely as possible.

Signed-off-by: Georgi Dimitrov <georgi.dimitrov@form3.tech>
@georgijd-form3
Copy link
Contributor Author

Hi @nicks,

Could you, please, have a look at the latest changes?

@georgijd-form3
chore(cli): fix log flag completion functions
2c2680a 
Signed-off-by: Georgi Dimitrov <georgi.dimitrov@form3.tech>
nicks
nicks approved these changes Mar 6, 2025
View reviewed changes
Copy link
Member

@nicks nicks left a comment

Choose a reason for hiding this comment

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

lgtm!

@nicks nicks merged commit 62b10e4 into tilt-dev:master Mar 6, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicks nicks nicks approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@georgijd-form3 @mateusz-drzymala-form3 @nicks