Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.07k stars 1.19k forks source link

Q: Core logging and Application Insights #11999

Closed diberry closed 4 years ago

diberry commented 4 years ago

Core logging is meant to be used by Azure Service teams - or that is how I understand it. But it also appears to be a logging library akin to Winston in some ways.

Application Insights is more about collecting logging and metrics into an Azure data store (conceptual data store). I've used Application Insights and like it.

A few questions: Would a customer ever use both, such as core logging feeding into application insights?
While core logging is meant to be used by Service teams for their client SDK libraries, is there a reason a customer couldn't/shouldn't pick up the core logging library and use it?

xirzec commented 4 years ago

I'm assuming this is a question about @azure/logger? The main motivation for the package is we wanted a consistent logging story for our packages that gave the developer control and mimicked popular NodeJS logging behavior (i.e. the debug package.)

I would expect most Node applications to use debug or similar directly. The only reason we couldn't depend upon debug ourselves is that it doesn't support IE11, which is an unfortunate requirement for storage. That said, there's no reason why someone couldn't use @azure/logger for their own logging, though it would be rather awkward that all of their logs would be under the azure namespace.

I don't think we wanted to force customers to adopt AppInsights by depending strongly on it, though there is no reason that you can't funnel all output from @azure/logger into AppInsights.

Perhaps @bterlson can correct anything I am mistaken about, as he was the original designer of the package.

diberry commented 4 years ago

@xirzec When you use the term 'developer', is that generally an Azure SDK service team building a client library or is that any one inside or outside MSFT? Who is the intended audience?

xirzec commented 4 years ago

Sorry I should have said consumer which is our term for developers who depend on SDK packages.

In general though, we don't expect consumers to depend directly on any of the core packages.

diberry commented 4 years ago

Ok, got it.

jmealo commented 3 years ago

@xirzec: The monkey patching of debug that AI does is very problematic and isn't compatible with community modules, namely pino-debug. The original PR to merge the IE10 compatibility for that voiced concerns that this would happen. I have to include some third party dependencies follow-redirect in various places to prevent AI from throwing errors from the monkey patches. If at all possible removing or making the shim aware/compatible with other shims would be advisable.

Working with Azure modules has made debugging, logging and error handling a real challenge that I haven't faced on any other Node.js projects in the past. As a consumer, it was very unclear what was for internal development/usage versus what I was supposed to use. What errors/comments/mechanisms for Microsoft's internal use or ours. This has been getting better with more recent development but was very rough on earlier Service Bus versions.

xirzec commented 3 years ago

@xirzec: The monkey patching of debug that AI does is very problematic and isn't compatible with community modules, namely pino-debug. The original PR to merge the IE10 compatibility for that voiced concerns that this would happen. I have to include some third party dependencies follow-redirect in various places to prevent AI from throwing errors from the monkey patches. If at all possible removing or making the shim aware/compatible with other shims would be advisable.

I'm not sure what you are referring to by "the monkey patching of debug" though it appears you're talking about something that AppInsights is doing?

Perhaps you want to file an issue in their repository? https://github.com/Microsoft/ApplicationInsights-node.js/

Working with Azure modules has made debugging, logging and error handling a real challenge that I haven't faced on any other Node.js projects in the past. As a consumer, it was very unclear what was for internal development/usage versus what I was supposed to use. What errors/comments/mechanisms for Microsoft's internal use or ours. This has been getting better with more recent development but was very rough on earlier Service Bus versions.

I'd love to send feedback to the appropriate teams if you can file an issue that describes where you think we can do a better job.

For one thing, I think we could do a better job documenting how @azure/logger works. That's the only logging mechanism that I think we support in SDK packages, but perhaps there are some one-off solutions that I'm not aware of.

As far as I know, none of the packages in this repository send any error telemetry to Microsoft, so all of what is there is for the benefit of the developer using the package. Services do have logging on the server-side that support teams can access, but there's nothing the SDK does for this other than setting a descriptive user agent and providing unique client request ids.

jmealo commented 3 years ago

The follow-redirects bug is fixed in the latest version (and the debug dependency was removed) we were able to remove two hacks/workarounds just in time for handoff to the client! 🎉

It would be really nice is @azure/logger accepted a structured logging mechanism that follows the logs4j interface. The current interface is bespoke.

@xirzec @ramya-rao-a: https://github.com/microsoft/ApplicationInsights-node.js/issues/731

jmealo commented 3 years ago

It's important to be aware of the ecosystem around debug particularly: https://github.com/pinojs/pino-debug

I can share the hoops that we're jumping through on our end if it'll help give you an idea of what a customer who uses structured logging needs.

Even if the eventual destination is Azure Monitor/AI that doesn't mean it's not going to go out on stdout and be processed using standard K8s mechanisms.

Standardizing the logging and error handling with both internal/external developers in mind would go a long way.

ramya-rao-a commented 3 years ago

I can share the hoops that we're jumping through on our end if it'll help give you an idea of what a customer who uses structured logging needs.

We would love to hear more on this @jmealo Would you mind logging a new issue for this? Or does #13817 cover what you are thinking of?

jmealo commented 3 years ago

@ramya-rao-a: I think some additional work might be required before a customer can realistically use structured logging (#13817 may not be useful on its own). I made a feature request to track that work separately: https://github.com/Azure/azure-sdk-for-js/issues/13824