METRON-633: Create better logging for HbaseEnrichmentWriter

[zezutom]

Contributor Comments

I have added logging to critical paths, so that issues can be tracked down more easily.

Pull Request Checklist

In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:

For all changes:

• Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
• Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
• Has your PR been rebased against the latest commit within the target branch (typically master)?

For code changes:

• Have you ensured that the full suite of tests and checks have been executed in the root incubating-metron folder via:

  mvn -q clean integration-test install && build_utils/verify_licenses.sh 
  

• Have you written or updated unit tests and or integration tests to verify your changes?

[zezutom]

Hey folks, this one has been open for a while. Could I please get some +1's? Thank you!

[justinleet]

Can you use parameterized messages for the various log statements or logger.isDebugEnabled(), rather than constructing the strings every time? The common case is for this to not be run, so I'd rather not have this stuff constructed to much. See: https://www.slf4j.org/faq.html#logging_performance

If I recall correctly, typically it's better to use parameterized messages if it's just using the toString() of an object, and logger.isDebugEnabled() if not (to avoid whatever calls are made). I'd have to double check that though.

[mmiklavc]

Agreed with @justinleet about switching away from isDebugEnabled(). Except for instances where you're doing something that has a fair amount of overhead in evaluating the pre-formatted strings, using message parameters is the current recommended approach. Also allow me to plug my PR that normalizes logging across the board - #599 :-)

[mmiklavc]

Better to turn this and similar statements into LOG.debug("Key column: '{}'", o );. Per the slf4j docs, this has the potential to improve calls by up to 30x where debugging is not enabled.

[zezutom]

Thanks everyone for a great feedback! @justinleet I will rebase once #599 is merged, cheers.

[cestella]

Just piling in here and saying thanks @zezutom for the contribution. This looks great!

[zezutom]

Thank you @cestella and sorry for the delay, now I am back to it.

[justinleet]

@zezutom When you have a chance, could you merge in master and deconflict this? The Travis build timeouts should be fixed as of #624. Sorry about the trouble with Travis, and I look forward to getting this into master.

[zezutom]

@justinleet Will do, thanks for sorting out Travis!

[zezutom]

@justinleet I can't merge this PR, do you mind merging to master yourself? Cheers.

[justinleet]

+1, thanks again for the contribution.