Project

Profile

Help

Issue #2315

Better logging at INFO level

Added by mhrivnak over 5 years ago. Updated almost 3 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version - Nectar:
Platform Release:
2.13.1
Target Release - Nectar:
1.5.4
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
Yes
Tags:
Pulp 2
Sprint:
Sprint 13
Quarter:

Description

Downloads are currently logged at the DEBUG level, but a number of users have requested that they be logged at INFO.

The streamer already logs them at INFO, and a good discussion was had about that behavior where those involved came to consensus. A similar example that comes to mind is MTAs, where each email coming and going normally gets logged at INFO.

This would be done in two modules. HTTPThreadedDownloader and LocalFileDownloader. For example, here are some links to these where DEBUG logging currently takes place:

https://github.com/pulp/nectar/blob/4fe7327c4ad1bb2f8c040b75d80306ee047de615/nectar/downloaders/threaded.py#L267
https://github.com/pulp/nectar/blob/4fe7327c4ad1bb2f8c040b75d80306ee047de615/nectar/downloaders/local.py#L161

Implementation Details

It might make more sense to log right before calling the success and failure handlers, but that might miss the download_one workflow. That detail aside, I would imagine something like:

log.info('Download succeeded: {url}')

or

log.info('Download failed: {url}')

Related issues

Related to Pulp - Issue #2836: Pulp makes a wall of logs during syncCLOSED - WONTFIX<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

Associated revisions

Revision 66ce1c62 View on GitHub
Added by daviddavis about 5 years ago

Better logging at INFO level for downloads

Instead of showing success/error messages at debug level, we're now showing them at INFO. If the error message was at a level above INFO, I left it as is.

fixes #2315 https://pulp.plan.io/issues/2315

History

#1 Updated by bmbouter about 5 years ago

  • Checklist item changed from Convert the debug log statement to use info to Convert the debug log statements in HTTPThreadedDownloader to use info
  • Checklist item Convert the debug log statements in LocalFileDownloader to use info added
  • Description updated (diff)

If we're doing this for the HTTPThreadedDownloader, we should also make similar changes in LocalFileDownloader[0]. I've edited the description to include links to both modules.

Also if this is a new Feature for nectar, that would mean this would come as a new nectar Y release right? Does the scope of this issue also include building and releasing the new nectar? If not, we need to make another issue to track that.

Since I made changes to the scope I'm not going to groom it myself.

[0]: https://github.com/pulp/nectar/blob/4fe7327c4ad1bb2f8c040b75d80306ee047de615/nectar/downloaders/local.py#L25

#2 Updated by mhrivnak about 5 years ago

I don't see changing the log level as a new feature. It's a slight modification to an existing behavior. Adding the same log statement to a new part of the code is an expansion of the behavior, but making a new Y release of nectar for this strikes me as overkill. In semver lingo, I wouldn't call log statements part of a "public API".

#3 Updated by bmbouter about 5 years ago

  • Tracker changed from Story to Issue
  • Subject changed from As a user, I can see downloads logged at INFO to Better logging at INFO level
  • Severity set to 2. Medium
  • Triaged set to No

mhrivnak: that sounds fine. I retitled it and switched its tracker to being an issue which makes sense I think. If this looks good to you, feel free to groom it or revert them.

#4 Updated by bizhang about 5 years ago

  • Sprint/Milestone set to 31
  • Triaged changed from No to Yes

#5 Updated by daviddavis about 5 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to daviddavis

#6 Updated by daviddavis about 5 years ago

  • Checklist item Convert the debug log statements in HTTPThreadedDownloader to use info set to Done

#7 Updated by daviddavis about 5 years ago

  • Checklist item Convert the debug log statements in HTTPThreadedDownloader to use info set to Not done

#8 Updated by daviddavis about 5 years ago

  • Checklist item Convert the debug log statements in HTTPThreadedDownloader to use info set to Done

#9 Updated by daviddavis about 5 years ago

  • Checklist item Also report success or failure in the log statement set to Done

#10 Updated by daviddavis about 5 years ago

  • Checklist item Convert the debug log statements in LocalFileDownloader to use info set to Done
  • Status changed from ASSIGNED to POST

#11 Updated by daviddavis about 5 years ago

  • Status changed from POST to MODIFIED

#12 Updated by bizhang over 4 years ago

  • Platform Release set to 2.13.1
  • Target Release - Nectar set to 1.5.4

#13 Updated by bizhang over 4 years ago

  • Status changed from MODIFIED to 5

#14 Updated by bizhang over 4 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE

#15 Updated by daviddavis over 4 years ago

  • Related to Issue #2836: Pulp makes a wall of logs during sync added

#16 Updated by bmbouter almost 4 years ago

  • Sprint set to Sprint 13

#17 Updated by bmbouter almost 4 years ago

  • Sprint/Milestone deleted (31)

#18 Updated by bmbouter almost 3 years ago

  • Tags Pulp 2 added

Also available in: Atom PDF