feat: Add the ability to send user feedback to the console even when logging to file.

[fredcarle]

Relevant issue(s)

Resolves #507

Description

This PR adds the ability to send user feedback to the console even when we configure Defra to send the log to a file. Feedback is sent in plain text as a simple message.

Tasks

• I made sure the code is well commented, particularly hard-to-understand areas.
• I made sure the repository-held documentation is changed accordingly.
• I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
• I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

unit test

Specify the platform(s) on which this was tested:

• MacOS

[shahzadlone]

You are missing a resolving keyword from the description which is why it's not linking to the issue.

[codecov]

Codecov Report

Merging #568 (eeab10f) into develop (c3c6495) will increase coverage by 0.59%.
The diff coverage is 77.77%.

❗ Current head eeab10f differs from pull request most recent head 2895ba6. Consider uploading reports for the commit 2895ba6 to get more accurate results

@@             Coverage Diff             @@
##           develop     #568      +/-   ##
===========================================
+ Coverage    56.08%   56.68%   +0.59%     
===========================================
  Files          121      118       -3     
  Lines        14231    13997     -234     
===========================================
- Hits          7982     7934      -48     
+ Misses        5549     5372     -177     
+ Partials       700      691       -9     

Impacted Files	Coverage Δ
logging/logging.go	100.00% <ø> (ø)
logging/logger.go	78.52% <77.77%> (-0.29%)	⬇️
query/graphql/planner/sort.go	76.00% <0.00%> (-7.51%)	⬇️
query/graphql/planner/sum.go	74.80% <0.00%> (-6.90%)	⬇️
query/graphql/parser/mutation.go	82.35% <0.00%> (-1.35%)	⬇️
core/doc.go	93.61% <0.00%> (-0.78%)	⬇️
query/graphql/parser/query.go	79.62% <0.00%> (-0.56%)	⬇️
query/graphql/planner/dagscan.go	73.22% <0.00%> (-0.44%)	⬇️
core/data.go	88.73% <0.00%> (-0.39%)	⬇️
query/graphql/planner/scan.go	87.82% <0.00%> (-0.21%)	⬇️
... and 19 more

[AndrewSisley]

question: I am slightly surprised by this PR and assumed it was something else at first. Are you sure the current log functions don't output to both if configured to write to file?

suggestion: Assuming it doesn't already do this, might it be better to replace the extra functions with a config boolean that enables logging to both, and have that enabled by default?

[fredcarle]

question: I am slightly surprised by this PR and assumed it was something else at first. Are you sure the current log functions don't output to both if configured to write to file?

It doesn't. You can try it on the default branch. Set the logging output in your config file and you'll notice that nothing will be logged to the console anymore.

suggestion: Assuming it doesn't already do this, might it be better to replace the extra functions with a config boolean that enables logging to both, and have that enabled by default?

I think it would be better not to do that. In my opinion, interactive feedback should be explicit. Like something the user benefits from seeing. For example there is no benefit to a user to see all the DB log messages for each interaction. Neither is there any benefit in seeing the logs from HTTP requests. But there is benefit in knowing the API port and other such details.

[AndrewSisley]

I think it would be better not to do that. In my opinion, interactive feedback should be explicit. Like something the user benefits from seeing. For example there is no benefit to a user to see all the DB log messages for each interaction. Neither is there any benefit in seeing the logs from HTTP requests. But there is benefit in knowing the API port and other such details.

The loggers I have worked with in the past have logged to both file and console - that may have been a deliberately configured choice (not sure, 70-30 odds in favour of deliberate config vs default behaviour). It can be quite useful to have both, and more importantly, this appears to prevent users from making that choice without us reworking the logging interface, and all the places that call it.

[fredcarle]

The loggers I have worked with in the past have logged to both file and console - that may have been a deliberately configured choice (not sure, 70-30 odds in favour of deliberate config vs default behaviour). It can be quite useful to have both, and more importantly, this appears to prevent users from making that choice without us reworking the logging interface, and all the places that call it.

This doesn't prevent the user from making the choice (log output can be set as []string{"stderr", "path/to/log/file"} and it will output to both). It just give us a way to be explicit about sending some feedback to the console if the user didn't configure it to do so. Also, these explicit feedback methods only output the message. Not the full log structure.

[AndrewSisley]

This doesn't prevent the user from making the choice (log output can be set as []string{"stderr", "path/to/log/file"} and it will output to both).

Of course! My bad for forgetting that. Thanks. Am totally happy with the extra funcs then lol

[AndrewSisley]

Cheers for answering my questions. I have doubts as to whether we'd want these calls split by level (would they not all just be info?), but see that as part of the other discussion RE log levels.

[fredcarle]

Cheers for answering my questions. I have doubts as to whether we'd want these calls split by level (would they not all just be info?), but see that as part of the other discussion RE log levels.

Yes that's out scope here and I'll create a new PR once we've made the decision on levels :)

[shahzadlone]

super-tiny-nitpick: All other comments are ending with a period haha, add ending dots ?