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.

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

Enhanced EventWatcher logs #5558

Merged
merged 10 commits into from
Feb 7, 2025
Merged

Enhanced EventWatcher logs #5558

merged 10 commits into from
Feb 7, 2025

Conversation

t-kikuc
Copy link
Member

@t-kikuc t-kikuc commented Feb 7, 2025

What this PR does:

Mainly:

  • add messages about "will retry in the next loop" and retry counts
  • convert log lever from Error to Warn in retry.Do()
    • After failing for the max count, ErrorLog will be shown
  • add event-ids field

Why we need it:

  • To clarify that some errors can be solved in the next loop or retry
  • To suppress too many logs while retry
  • To enable to track which events failed

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

ffjlabo
ffjlabo previously approved these changes Feb 7, 2025
Copy link
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

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

👍

Copy link

codecov bot commented Feb 7, 2025

Codecov Report

Attention: Patch coverage is 0% with 35 lines in your changes missing coverage. Please review.

Project coverage is 26.41%. Comparing base (de1800e) to head (c99f771).
Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
pkg/app/piped/eventwatcher/eventwatcher.go 0.00% 35 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5558      +/-   ##
==========================================
- Coverage   26.42%   26.41%   -0.02%     
==========================================
  Files         465      465              
  Lines       49907    49938      +31     
==========================================
  Hits        13189    13189              
- Misses      35663    35694      +31     
  Partials     1055     1055              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: t-kikuc <[email protected]>
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

👍

@t-kikuc t-kikuc merged commit 75db0fa into master Feb 7, 2025
16 of 18 checks passed
@t-kikuc t-kikuc deleted the eventwatcher/log-improve branch February 7, 2025 16:00
github-actions bot pushed a commit that referenced this pull request Feb 17, 2025
* Show push error log earlier than reporting

Signed-off-by: t-kikuc <[email protected]>

* Use WarnLog in retry

Signed-off-by: t-kikuc <[email protected]>

* clarify log messages

Signed-off-by: t-kikuc <[email protected]>

* clarify log messages

Signed-off-by: t-kikuc <[email protected]>

* add TestDoCalls for asserting counts

Signed-off-by: t-kikuc <[email protected]>

* add eventIDs in log

Signed-off-by: t-kikuc <[email protected]>

* enrich logs in updateValues

Signed-off-by: t-kikuc <[email protected]>

* nits

Signed-off-by: t-kikuc <[email protected]>

* Revert "add TestDoCalls for asserting counts"

This reverts commit de3f112.

Signed-off-by: t-kikuc <[email protected]>

---------

Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>
github-actions bot pushed a commit that referenced this pull request Feb 17, 2025
* Show push error log earlier than reporting

Signed-off-by: t-kikuc <[email protected]>

* Use WarnLog in retry

Signed-off-by: t-kikuc <[email protected]>

* clarify log messages

Signed-off-by: t-kikuc <[email protected]>

* clarify log messages

Signed-off-by: t-kikuc <[email protected]>

* add TestDoCalls for asserting counts

Signed-off-by: t-kikuc <[email protected]>

* add eventIDs in log

Signed-off-by: t-kikuc <[email protected]>

* enrich logs in updateValues

Signed-off-by: t-kikuc <[email protected]>

* nits

Signed-off-by: t-kikuc <[email protected]>

* Revert "add TestDoCalls for asserting counts"

This reverts commit de3f112.

Signed-off-by: t-kikuc <[email protected]>

---------

Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>
t-kikuc added a commit that referenced this pull request Feb 17, 2025
* Correct notification routing for `DEPLOYMENT_STARTED` (#5523)

* Correct notification routing for `DEPLOYMENT_STARTED`

Signed-off-by: Yuki Okushi <[email protected]>

* Harden test case

Signed-off-by: Yuki Okushi <[email protected]>

---------

Signed-off-by: Yuki Okushi <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>

* Sort results of plan-preview (#5540)

* Sort results of plan-preview

Signed-off-by: kj455 <[email protected]>

* Ensure the order of list piped

Signed-off-by: kj455 <[email protected]>

* fix: lint

Signed-off-by: kj455 <[email protected]>

* fix: move sorting to pipectl

Signed-off-by: kj455 <[email protected]>

* fix: add testcase

Signed-off-by: kj455 <[email protected]>

* fix: dev docs

Signed-off-by: kj455 <[email protected]>

* add docs

Signed-off-by: kj455 <[email protected]>

---------

Signed-off-by: kj455 <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>

* Enhanced EventWatcher logs (#5558)

* Show push error log earlier than reporting

Signed-off-by: t-kikuc <[email protected]>

* Use WarnLog in retry

Signed-off-by: t-kikuc <[email protected]>

* clarify log messages

Signed-off-by: t-kikuc <[email protected]>

* clarify log messages

Signed-off-by: t-kikuc <[email protected]>

* add TestDoCalls for asserting counts

Signed-off-by: t-kikuc <[email protected]>

* add eventIDs in log

Signed-off-by: t-kikuc <[email protected]>

* enrich logs in updateValues

Signed-off-by: t-kikuc <[email protected]>

* nits

Signed-off-by: t-kikuc <[email protected]>

* Revert "add TestDoCalls for asserting counts"

This reverts commit de3f112.

Signed-off-by: t-kikuc <[email protected]>

---------

Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>

* update RELEASE to v0.50.2 with doc update (#5571)

Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>

---------

Signed-off-by: Yuki Okushi <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>
Signed-off-by: kj455 <[email protected]>
Signed-off-by: t-kikuc <[email protected]>
Co-authored-by: Yuki Okushi <[email protected]>
Co-authored-by: Ibuki Kaji <[email protected]>
Co-authored-by: Tetsuya KIKUCHI <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants