Should we include an ILogger as part of ApiCall etc?
jskeet opened this issue · 8 comments
For GAX v4, we've got a dependency on the logging abstractions anyway.
If we added ILogger to ServiceSettingsBase (or potentially ClientBuilderBase, or both) we could add per-client logging, including retries etc.
If we added an ILoggerFactory instead, we could potentially create the logger ourselves with the right client name.
(We should probably prototype this before v4, to see whether we need to implement it properly before v4 ships or whether it could be added afterwards.)
After a few minutes of looking, we'd either need to add an ILoggerFactory or ILogger to ServiceSettingsBase, or add an ILogger parameter to the constructor of each client implementation type. The latter would probably be the most sensible approach, to be honest - and would definitely need to be done before the major release.
Weird idea: we could include ILogger in CallSettings. Probably not a good idea, but worth at least considering...
(CallSettings would actually make this somewhat easier to plumb through, although we'd also want the method name to be plumbed through too, which is trickier...)
Interesting decision: a single logger for the client, or a logger per method? The latter makes it easier to give different log levels to different methods...
One thing we need to look at: We might need to disable the Logging capabilities as introduced by this feature in the Diagnostics library itself, else I think we'll generate infinite log entries???
Ha! Good point, yes. Not entirely sure the best way of doing that, but we can come to it later...