Azure / dotnet-template-azure-iot-edge-module

Scaffolding tool for Azure IoT Edge C# module
MIT License
15 stars 23 forks source link

[FeatureRequest] Logging to Console using ILogger #64

Open SychevIgor opened 4 years ago

SychevIgor commented 4 years ago

Thank you for all what you are doing. In a template, you are logging using Console.WriteLine - that it's definitely not the best practice for .net core based apps in general.

We all know, that people don't like to customize initial templates and just adding their code on standard positions. Some of my "students" decided that log to console directly- is a best practice for iot.

From my perspective- I would like to see in this template- logging to console via ILogger interface and reading configuration of ILogger from appconfig.json file as it was done for asp.net core apps. And add default logging level into "Container creating option" to simplify the overriding

What it will give:

image Chart showing that 50% of logs are useless, but expensive \n and debug output.

Please, add logging via ILogger to template and help us with best practices and save our money when running on Kubernetes https://docs.microsoft.com/en-us/azure/iot-edge/how-to-install-iot-edge-kubernetes

grahamehorner commented 4 years ago

@SychevIgor I would agree and also suggest similarly using the .net core host builder & dependency injection as used in asp.net core worker projects and/or BackgroundService in an improved template; may be we can work together on a template that we can submit as a PR ?

grahamehorner commented 4 years ago

@SychevIgor check out the PR https://github.com/Azure/dotnet-template-azure-iot-edge-module/pull/68

blackchoey commented 4 years ago

@SychevIgor @grahamehorner Thanks for the great idea and contribution. The major purpose of template is help people create a new module from scratch. So it's better to keep the dependencies and sample code as simple as possible. People can introduce dependencies according to their actual requirement. For your idea about using ILogger, you can add it as a sample at https://github.com/Azure-Samples/azure-iot-samples-csharp. Please let us know if you have further questions.

LarsKemmann commented 4 years ago

@blackchoey I hear that you want to keep the learning curve as simple as possible. Because I support that goal, I agree with the suggestion to adapt the new .NET Core Worker Service template for IoT Edge (and at least provide it as an option for C# in the add-module wizard in VS alongside the current "bare-bones" C# template). By using a very well-documented standard framework for the module structure, potential IoT Edge developers will have a much easier time finding documentation and best practices. I say this from experience both personally and working with several teams on IoT Edge implementations -- you have an opportunity here to short-circuit a bunch of not-invented-here syndrome issues. :)

konichi3 commented 3 years ago

Hi

Thank you for filing the issue.

We are looking to clean up the reported issues that are obsolete such that our team can focus on the latest issues reported. As such, we are going to close the issue for now.

If the issue is still blocking or critical to you, please file a new issue and we will do our best to respond as soon as possible. When you file a new issue, please ensure following additional information are provided.

o If you believe it’s a bug, provide detail repro steps. o If it’s a feature request, please note as such o Expected outcome o Is this a blocking issue?

Thank you, Azure IoT Developer and Device team

LarsKemmann commented 3 years ago

This is a feature request and it's still very much applicable. I disagree with the reasoning that a new project template should be minimalist -- you need to provide enough standard/familiar .NET concepts that it will be easy for people to start working with. (Edit: Or at least do that in an optional in-box template.) Please reopen.

konichi3 commented 3 years ago

Reopened and this is now in our backlog

LarsKemmann commented 3 years ago

@konichi3 Will this help? 😊 Feedback is welcome! Happy to submit a PR as well! https://github.com/LarsKemmann/AzureIoTEdgeExtensions https://www.nuget.org/packages/IoTEdge.Extensions.Hosting/

rido-min commented 1 year ago

Hi @SychevIgor

The latest version of this template is using ILogger. Do you need anything else? otherwise, can you close this issue?