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

Fix: avoid log spam for log links generated during the pod's pending phase #5945

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

fg91
Copy link
Member

@fg91 fg91 commented Oct 31, 2024

Why are the changes needed?

In #4726 I introduced the option to configure the lifecycle of log links as is documented here. In particular, log links can be configured to be already shown before a task starts running or be configured to disappear after a task stops.

@Tom-Newton noted that their flytepropeller logs are spammed with error messages because the log link generation logic for pod tasks checks whether container statuses exist and if not logs this as an error. Prior to #4726 it made sense to always log this as an error because log links were only generated in the "Running" phase of the task when the pod should have container statuses. Now that we optionally generate log links already in the pending phase, it is not unexpected that the container statuses are empty.

What changes were proposed in this pull request?

Log the respective message with debug log level if the pod is in the pending phase. Keep logging with error log level in other pod phases.

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

…phase

Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
@fg91 fg91 self-assigned this Oct 31, 2024
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

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

Project coverage is 36.84%. Comparing base (2b0af2b) to head (6d125d8).
Report is 143 commits behind head on master.

Files with missing lines Patch % Lines
flyteplugins/go/tasks/logs/logging_utils.go 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5945      +/-   ##
==========================================
- Coverage   36.84%   36.84%   -0.01%     
==========================================
  Files        1309     1309              
  Lines      130994   131001       +7     
==========================================
- Hits        48271    48269       -2     
- Misses      78533    78543      +10     
+ Partials     4190     4189       -1     
Flag Coverage Δ
unittests-datacatalog 51.58% <ø> (ø)
unittests-flyteadmin 54.10% <ø> (ø)
unittests-flytecopilot 11.73% <ø> (ø)
unittests-flytectl 62.40% <ø> (-0.05%) ⬇️
unittests-flyteidl 6.92% <ø> (ø)
unittests-flyteplugins 53.62% <0.00%> (-0.02%) ⬇️
unittests-flytepropeller 43.02% <ø> (ø)
unittests-flytestdlib 55.41% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Contributor

@Tom-Newton Tom-Newton left a comment

Choose a reason for hiding this comment

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

Thanks

@fg91 fg91 merged commit 2fa77a5 into master Nov 5, 2024
50 of 51 checks passed
@fg91 fg91 deleted the fg91/fix/log-link-pending-phase-log-spam branch November 5, 2024 18:41
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