Improve logging (#143)

* Improve logging

Trying to strike the right balance between verbose and non-verbose mode in the CLI.

Particularly for the upload command it's been not satisfactory.
The list of files being uploaded as coverage is now listed in non-verbose mode.

Plus there's a positive feedback now that the process (command) finished - for ALL commands.
So you know that at least the CLI did its thing, even though there might still be errors.

And again for the upload command, separate better the response from codecov-api
and the response from the PUT request afterwards.
To help debugging and all that.

* Make positive feedback less intrusive

It was a lot of data in the "process complete" message.
We don't need all that just to let users know the process terminated.
So breaking that up again into 2 messages, the 2nd having all the data in verbose level.

Author: giovanni-guidini

codecov_cli/helpers/request.py

@@ -59,8 +59,11 @@ def request_result(resp):
 def log_warnings_and_errors_if_any(
     sending_result: RequestResult, process_desc: str, fail_on_error: bool = False
 ):
+    logger.info(
+        f"Process {process_desc} complete",
+    )
     logger.debug(
-        f"Process {process_desc} complete.",
+        f"{process_desc} result",
         extra=dict(extra_log_attributes=dict(result=sending_result)),
     )
     if sending_result.warnings:

codecov_cli/services/upload/upload_collector.py

@@ -131,9 +131,9 @@ def generate_upload_data(self) -> UploadCollectionResult:
         logger.debug("Collecting relevant files")
         network = self.network_finder.find_files()
         coverage_files = self.coverage_file_finder.find_coverage_files()
-        logger.debug(f"Found {len(coverage_files)} coverage files to upload")
+        logger.info(f"Found {len(coverage_files)} coverage files to upload")
         for file in coverage_files:
-            logger.debug(f"> {file}")
+            logger.info(f"> {file}")
         return UploadCollectionResult(
             network=network,
             coverage_files=coverage_files,

codecov_cli/services/upload/upload_sender.py

@@ -1,5 +1,6 @@
 import base64
 import json
+import logging
 import typing
 import uuid
 import zlib
@@ -17,6 +18,8 @@
     UploadCollectionResultFile,
 )
 
+logger = logging.getLogger("codecovcli")
+
 
 class UploadSender(object):
     def send_upload_data(
@@ -53,11 +56,17 @@ def send_upload_data(
         # Data that goes to storage
         reports_payload = self._generate_payload(upload_data, env_vars)
 
+        logger.debug("Sending upload request to Codecov")
         resp_from_codecov = send_post_request(url=url, data=data, headers=headers)
         if resp_from_codecov.status_code >= 400:
             return resp_from_codecov
         resp_json_obj = json.loads(resp_from_codecov.text)
+        logger.debug(
+            "Upload request to Codecov complete.",
+            extra=dict(extra_log_attributes=dict(response=resp_json_obj)),
+        )
         put_url = resp_json_obj["raw_upload_location"]
+        logger.debug("Sending upload to storage")
         resp_from_storage = send_put_request(put_url, data=reports_payload)
         return resp_from_storage
 

tests/services/commit/test_commit_service.py

@@ -31,6 +31,7 @@ def test_commit_command_with_warnings(mocker):
 
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
     assert out_bytes == [
+        ("info", "Process Commit creating complete"),
         ("info", "Commit creating process had 1 warning"),
         ("warning", "Warning 1: somewarningmessage"),
     ]
@@ -73,7 +74,13 @@ def test_commit_command_with_error(mocker):
         )
 
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
-    assert out_bytes == [("error", "Commit creating failed: Permission denied")]
+    assert out_bytes == [
+        (
+            "info",
+            "Process Commit creating complete",
+        ),
+        ("error", "Commit creating failed: Permission denied"),
+    ]
     assert res == mock_send_commit_data.return_value
     mock_send_commit_data.assert_called_with(
         commit_sha="commit_sha",

tests/services/report/test_report_results.py

@@ -34,6 +34,7 @@ def test_report_results_command_with_warnings(mocker):
 
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
     assert out_bytes == [
+        ("info", "Process Report results creating complete"),
         ("info", "Report results creating process had 1 warning"),
         ("warning", "Warning 1: somewarningmessage"),
     ]
@@ -73,7 +74,10 @@ def test_report_results_command_with_error(mocker):
         )
 
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
-    assert out_bytes == [("error", "Report results creating failed: Permission denied")]
+    assert out_bytes == [
+        ("info", "Process Report results creating complete"),
+        ("error", "Report results creating failed: Permission denied"),
+    ]
     assert res == mock_send_reports_result_request.return_value
     mock_send_reports_result_request.assert_called_with(
         commit_sha="commit_sha",

tests/services/report/test_report_service.py

@@ -58,6 +58,7 @@ def test_create_report_command_with_warnings(mocker):
 
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
     assert out_bytes == [
+        ("info", "Process Report creating complete"),
         ("info", "Report creating process had 1 warning"),
         ("warning", "Warning 1: somewarningmessage"),
     ]
@@ -101,7 +102,10 @@ def test_create_report_command_with_error(mocker):
         )
 
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
-    assert out_bytes == [("error", "Report creating failed: Permission denied")]
+    assert out_bytes == [
+        ("info", "Process Report creating complete"),
+        ("error", "Report creating failed: Permission denied"),
+    ]
     assert res == RequestResult(
         error=RequestError(
             code="HTTP Error 403",

tests/services/upload/test_upload_service.py

@@ -69,6 +69,7 @@ def test_do_upload_logic_happy_path_legacy_uploader(mocker):
         )
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
     assert out_bytes == [
+        ("info", "Process Upload complete"),
         ("info", "Upload process had 1 warning"),
         ("warning", "Warning 1: somewarningmessage"),
     ]
@@ -151,6 +152,7 @@ def test_do_upload_logic_happy_path(mocker):
         )
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
     assert out_bytes == [
+        ("info", "Process Upload complete"),
         ("info", "Upload process had 1 warning"),
         ("warning", "Warning 1: somewarningmessage"),
     ]
@@ -237,7 +239,8 @@ def test_do_upload_logic_dry_run(mocker):
         cli_config, ["first_plugin", "another", "forth"]
     )
     assert out_bytes == [
-        ("info", "dry-run option activated. NOT sending data to Codecov.")
+        ("info", "dry-run option activated. NOT sending data to Codecov."),
+        ("info", "Process Upload complete"),
     ]
     assert res == RequestResult(
         error=None,
@@ -294,9 +297,10 @@ def test_do_upload_logic_verbose(mocker, use_verbose_option):
             "Selected uploader to use: <class 'codecov_cli.services.upload.legacy_upload_sender.LegacyUploadSender'>",
         ),
         ("info", "dry-run option activated. NOT sending data to Codecov."),
+        ("info", "Process Upload complete"),
         (
             "debug",
-            'Process Upload complete. --- {"result": "RequestResult(error=None, warnings=None, status_code=200, text=\'Data NOT sent to Codecov because of dry-run option\')"}',
+            'Upload result --- {"result": "RequestResult(error=None, warnings=None, status_code=200, text=\'Data NOT sent to Codecov because of dry-run option\')"}',
         ),
     ]
     assert res == RequestResult(