Flow ILogger to InstrumentationHelper

[MarcoRossignoli]

We should flow the logger to instrumentation helper and emit important log warning here

https://github.com/tonerdo/coverlet/blob/82a920864a157bfaa9417540a20a4c03b336f1d0/src/coverlet.core/Helpers/InstrumentationHelper.cs#L155-L159

cc: @tonerdo @petli

[MarcoRossignoli]

wait for #566 to allow internal helper interfaces access.

[daveMueller]

@MarcoRossignoli Could you explain this in a bit more detail. I'm not sure if I get it right. Just inject ILogger and log a message in this catch block?

[MarcoRossignoli]

Yup Dave, this class is the core of instrumentation and sometimes it's very useful log behaviour, so for now the work here is only flow ILogger and add some trace here.

[daveMueller]

I'm currently working on this and think about the message to be logged?
Actually I'm also a bit confused why true is returned when the provider throws. How is instrumentation working when the source can't be found?

[MarcoRossignoli]

I'm currently working on this and think about the message to be logged?

A warning like "BadImageFormatException during MetadataReaderProvider.FromPortablePdbStream in InstrumentationHelper .PortablePdbHasLocalSource, unable to check if module has got local source"

Actually I'm also a bit confused why true is returned when the provider throws. How is instrumentation working when the source can't be found?

That is only the procedure to automatically exclude dll that we're "SURE" that we cannot instrument, it's a new feature and wasn't present in past, so my idea is to "preserve" old semantic and go on like the check doesn't exist. Coverlet is used also with .NET Framework(not only core) coverage and pdb could not be portable. MetadataReaderProvider.FromPortablePdbStream is not part of Cecil that could work also with non portable pdb(btw I did't check, but the general idea is to preserve old behaviour if I'm not sure of new one).