gh-2618: Cleanup inconsistent use of logging strings

[RobertG111]

Hi. This is my first ever contribution, thanks for the opportunity. The other pull request I created was closed due to having my personal email address. I have fixed the minor issues in relation to inconsistent use of logging strings. All logging functions used the correct format and was able to find only 1 extra mistake apart from the mentioned one in the issue. Thanks.

[GCHQDeveloper314]

Hello, thanks for your contribution. I've renamed the PR title to fit the naming scheme we use, this is partly described on our new ways of working page.

In your commit you've missed out the {} mentioned in the issue. This is required to add the parameter to the formatted log string.

Also, the issue calls for the problem to be fixed across the repo. You've found one other instance, but there are many others. Searching the repo using LOGGER.*\+ as a regex query, I can see 20+ other instances of string concatenation (some of which are partly using parameters).

[RobertG111]

Noted. Thanks for the fast reply.

[codecov-commenter]

Codecov Report

Merging #2651 (dee22a4) into develop (c33d0d8) will decrease coverage by 0.45%.
The diff coverage is 44.82%.

@@              Coverage Diff              @@
##             develop    #2651      +/-   ##
=============================================
- Coverage      66.71%   66.25%   -0.46%     
  Complexity      2521     2521              
=============================================
  Files            997      864     -133     
  Lines          32492    27302    -5190     
  Branches        3936     3285     -651     
=============================================
- Hits           21678    18090    -3588     
+ Misses          9117     7746    -1371     
+ Partials        1697     1466     -231     

Impacted Files	Coverage Δ
...uk/gov/gchq/gaffer/commonutil/ExecutorService.java	0.00% <0.00%> (ø)
...java/uk/gov/gchq/gaffer/commonutil/StreamUtil.java	20.63% <0.00%> (ø)
.../gchq/gaffer/cache/impl/HazelcastCacheService.java	73.91% <0.00%> (ø)
...q/gaffer/federatedstore/FederatedGraphStorage.java	79.33% <0.00%> (ø)
...etstore/query/JavaPredicateToParquetPredicate.java	50.54% <33.33%> (-0.39%)	⬇️
.../src/main/java/uk/gov/gchq/gaffer/graph/Graph.java	65.01% <50.00%> (ø)
...ration/handler/FederatedAddGraphHandlerParent.java	70.83% <50.00%> (ø)
...gov/gchq/gaffer/graph/hook/FunctionAuthoriser.java	93.54% <100.00%> (ø)
...on/resolver/named/NamedOperationScoreResolver.java	80.00% <100.00%> (ø)
.../uk/gov/gchq/gaffer/rest/config/FactoryConfig.java	78.12% <100.00%> (ø)
... and 139 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a0c863...dee22a4. Read the comment docs.

[t92549]

Thanks for the PR! Just a small change of syntax is required to get this working, I have linked an article in the review comment.

[t92549]

Thanks for all the updates! Still a couple more issues

[t92549]

Looking good and passing CI now, however, after a quick look I can see 5 more uses that aren't changed: 4 in example and one in store-implementation/parquet-store

[t92549]

Great changes, just a few more I found that are missed:

• Gaffer/example/road-traffic/road-traffic-generators/src/main/java/uk/gov/gchq/gaffer/traffic/generator/RoadTrafficDataLoader.java

  Line 124 in 6a0c863

  LOGGER.info("Unable to load data: " + e.getMessage());

• Gaffer/example/road-traffic/road-traffic-rest/src/main/java/uk/gov/gchq/gaffer/traffic/listeners/DataLoader.java

  Line 60 in 6a0c863

  LOGGER.info("Unable to load data: " + e.getMessage());

• Gaffer/example/road-traffic/road-traffic-rest/src/test/java/uk.gov.gchq.gaffer.traffic/RoadTrafficTestQueries.java

  Line 116 in 6a0c863

  LOGGER.info("Error parsing endDate: " + e.getMessage());

• Gaffer/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorage.java

  Line 507 in 6a0c863

  LOGGER.error(String.format("Skipping graphId: %s due to: %s", graphId, e.getMessage()), e);

• Gaffer/example/road-traffic/road-traffic-rest/src/main/java/uk/gov/gchq/gaffer/traffic/listeners/ConsoleBanner.java

  Line 36 in 6a0c863

  LOGGER.info(String.format("Gaffer road-traffic example is ready at: http:/localhost:%s/%s", port, path));

[GCHQDeveloper314]

The latest error is caused by the logging in RoadTrafficDataLoader.java being done from Java's util logger. This is used in a few places across road-traffic. Ideally SLF4J would be used, however that is not within scope of this ticket. Reverting any changes to the util logger (because it doesn't support parameterised logging) is what we'll want to do in this PR. Investigating changing to SLF4J is something we might raise as a new ticket.

[t92549]

The latest error is caused by the logging in RoadTrafficDataLoader.java being done from Java's util logger. This is used in a few places across road-traffic. Ideally SLF4J would be used, however that is not within scope of this ticket. Reverting any changes to the util logger (because it doesn't support parameterised logging) is what we'll want to do in this PR. Investigating changing to SLF4J is something we might raise as a new ticket.

That's my bad sorry, I said it needs changing but didn't realise it was using the java.util logger, apologies!

[t92549]

I think all cases are now covered, to get the CI passing the road-traffic example cases need to be reverted because they aren't using SLF4J, sorry!

[GCHQDeveloper314]

I can see a few remaining string concatenations across multiple lines, but there's nothing we can do about that as we are not using a new enough version of Java to use avoid this. PR looks to be ready to merge.

[RobertG111]

Thanks @t92549 @GCHQDeveloper314 for the help and reviews. It has been great working on this and will continue contributing. Many thanks.

[t92549]

Thanks for the PR! Apologies that a couple of times I suggested things that were actually incorrect!