microsoft / ApplicationInsights-Java

Application Insights for Java
http://aka.ms/application-insights
Other
296 stars 199 forks source link

Revisit All logAlways() Logs in the SDK #590

Closed dhaval24 closed 6 years ago

dhaval24 commented 6 years ago

We have several places in the SDK where logAlways() messages have been used and sometimes it is just not necessary to have them. Let;'s revisit them and start pruning or converting them to ERROR level verbosity as needed.

This would facilitate the cold start of our SDK.

littleaj commented 6 years ago

I agree with what you said @dhaval24 , but since logAlways has a level parameter, I'd vote to just use that severity unless we find a reason to change it during auditing.

In the same regard, I think we should consider SLF4j so users can pipe our logs however they want. Someone has already requested this feature: #309.

With slf4j, we can provide docs on how to include our logs on the console or in their own file. The latter would be great because they can send that file to us if there are issues.

dhaval24 commented 6 years ago

@littleaj yes I agree we should use SLF4J bridge to pump our logs. That definitely gives us much more flexibility in our internal logging story. Even more people have been asking for this almost always. We should also allow similar support like we have for log4j and logback for slf4j too (both internal logs as well as application production logs collection). Let's though triage this as separate issue and get this one done. Also I thought logAlwaysregardless of severity will always log. Severity will affect how it would print the output. For example INFO level would go like -

AI-INFO : "....." and so for other levels. So really we would have to switch to methods like InternalLogger.instance.ERROR("..") to achieve the cold start functionality.

cc: @nikmd23

grlima commented 6 years ago

We should get rid of logAlways. It's just not needed and it removes control out of users, who cannot opt out of it.

dhaval24 commented 6 years ago

I am working on this. Will submit a PR soon.