Skip to content
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

feat: Parse JSON structured logs in Argo UI. Fixes #6856 #10145

Merged
merged 3 commits into from
Mar 21, 2023
Merged

feat: Parse JSON structured logs in Argo UI. Fixes #6856 #10145

merged 3 commits into from
Mar 21, 2023

Conversation

isubasinghe
Copy link
Member

@isubasinghe isubasinghe commented Dec 1, 2022 •
edited by agilgur5
Loading

Fixes #6856
Supersedes #7386

Please do not open a pull request until you have checked ALL of these:

  • Create the PR as draft .
  • Run make pre-commit -B to fix codegen and lint problems.
  • Sign-off your commits (otherwise the DCO check will fail).
  • Use a conventional commit message (otherwise the commit message check will fail).
  • "Fixes #" is in both the PR title (for release notes) and this description (to automatically link and close the issue).
  • Add unit or e2e tests. Say how you tested your changes. If you changed the UI, attach screenshots.
  • Github checks are green.
  • Once required tests have passed, mark your PR "Ready for review".

If changes were requested, and you've made them, dismiss the review to get it reviewed again.

@isubasinghe isubasinghe changed the title Workflow json feat: Parse JSON structured logs in Argo UI. Fixes #6856 Dec 1, 2022
@isubasinghe
Copy link
Member Author

Just revived @krrrr38's PR. Thanks @krrrr38 for your work.

@isubasinghe isubasinghe marked this pull request as ready for review December 1, 2022 07:42
juliev0
juliev0 approved these changes Dec 9, 2022
View reviewed changes
Copy link
Contributor

@juliev0 juliev0 left a comment

Choose a reason for hiding this comment

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

neat!

@krrrr38 krrrr38 mentioned this pull request Dec 9, 2022
Closed
1 task
@sarabala1979
Copy link
Member

@isubasinghe Can you attach the screenshots for log viewer? before and after

@stale

This comment was marked as resolved.

Sign in to view
@stale stale bot added the problem/stale This has not had a response in some time label Dec 31, 2022
@stale

This comment was marked as resolved.

Sign in to view
@stale stale bot closed this Jan 8, 2023
@isubasinghe isubasinghe reopened this Jan 10, 2023
@stale stale bot removed the problem/stale This has not had a response in some time label Jan 10, 2023
@isubasinghe
Copy link
Member Author

@sarabala1979
Before (master branch):
image

After with filtering on other.showme
image

After without filtering:
image

alexec
alexec requested changes Feb 12, 2023
View reviewed changes
Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

I will approve this once you fix the lint issue.

@alexec
Copy link
Contributor

alexec commented Feb 12, 2023 •
edited
Loading

@isubasinghe question for you. I feel like you might find a few bugs in this that you need to fix afterwards. I've worked on similar feature and found a log tail of bugs that caused me to give up:

https://github.com/kubectl-logz/kubectl-logz

LMK is 30d, no reply --> don't want to fix.

@isubasinghe
Copy link
Member Author

@alexec Thanks for the heads up, I will test this manually a bit more in that case, regardless I will be working on Argo Workflows anyway so I should be able to squash these bugs when they do come up.
Fixing merge conflict now.

krrrr38 and others added 2 commits February 16, 2023 16:21
@krrrr38 @isubasinghe
feat: add workflow json log field selector. Close argoproj#6856
4c0cbb2 
Signed-off-by: krrrr38 <k.kaizu38@gmail.com>
@isubasinghe
fix: addressed linter issue
12535d8 
Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
@caelan-io
Copy link
Member

Hi @alexec - @isubasinghe fixed the linter issue you highlighted. Can we please get your approval and merge when you have a moment? Thanks!

sarabala1979
sarabala1979 approved these changes Mar 21, 2023
View reviewed changes
Copy link
Member

@sarabala1979 sarabala1979 left a comment

Choose a reason for hiding this comment

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

LGMT

@sarabala1979 sarabala1979 merged commit 7da30bd into argoproj:master Mar 21, 2023
terrytangyuan added a commit that referenced this pull request Apr 6, 2023
@terrytangyuan
Revert "feat: Parse JSON structured logs in Argo UI. Fixes #6856 (#10145
2abca7f 
)"

This reverts commit 7da30bd.
JPZ13 pushed a commit to pipekit/argo-workflows that referenced this pull request Jul 4, 2023
@isubasinghe @krrrr38
@alexec @JPZ13
feat: Parse JSON structured logs in Argo UI. Fixes argoproj#6856 (arg…
396674d 
…oproj#10145)

Signed-off-by: krrrr38 <k.kaizu38@gmail.com>
Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
Co-authored-by: krrrr38 <k.kaizu38@gmail.com>
Co-authored-by: Alex Collins <alexec@users.noreply.github.com>
@terrytangyuan terrytangyuan mentioned this pull request Aug 2, 2023
Closed
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
@terrytangyuan @dpadhiar
Revert "feat: Parse JSON structured logs in Argo UI. Fixes argoproj#6856
e56ab00 
 (argoproj#10145)"

This reverts commit 7da30bd.

Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexec alexec alexec approved these changes

@juliev0 juliev0 juliev0 approved these changes

@sarabala1979 sarabala1979 sarabala1979 approved these changes

Assignees

@juliev0 juliev0

@sarabala1979 sarabala1979

Labels
area/ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parse JSON structured logs in Argo UI
7 participants
@isubasinghe @sarabala1979 @alexec @caelan-io @juliev0 @agilgur5 @krrrr38