Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Azure Functions template should use configurable Serilog settings (🚧) #495

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

spasal
Copy link

@spasal spasal commented Dec 16, 2021

Implements configurable Serilog settings for Azure Function templates

Closes #480

@netlify
Copy link

netlify bot commented Dec 16, 2021

👷 Deploy request for arcus-templates pending review.
Visit the deploys page to approve it

🔨 Explore the source changes: 1dc97cb

@spasal spasal changed the title feat: Azure Functions template should use configurable Serilog settings feat: Azure Functions template should use configurable Serilog settings (🚧) Dec 16, 2021
@spasal
Copy link
Author

spasal commented Dec 16, 2021

Hi @stijnmoreels @fgheysels ,

A few questions:

  • Should the integration tests be updated to check whether the config settings are really used in the serilog implementation?
  • Any suggestions on how I can test this change locally? Create a project from the local template?

I noticed that there seems to be something wrong with the application insights custom logging levels. From what I saw, you need to specify it as well in the serilog settings of the config as well as in the application insights section of the config in host.json. Should this be included in this PR or should I open a separate issue / PR to investigate this further?

Thanks for the guidance/feedback/help!

@stijnmoreels
Copy link
Member

Great to have you working on this! This repo is not an easy one to contribute, so awesome job! 👍

To test that the Serilog actually uses the local file configuration, we could create a test that disrupts the configuration files after the test project is being created, and see that the project's endpoints aren't available.

This can be done with the UpdateFileInProject method of the project instance. (see web API project template integration tests for including app settings, for example).

@stijnmoreels
Copy link
Member

For the other issue about logging levels: we could indeed look into that more after we have established a baseline for reading the configuration in Serilog.
To be clear on this: you mean that logging to Azure Application Insights only works with logging levels when both the Serilog and Application Insights-specific sections in the host.json file are updated?

@stijnmoreels
Copy link
Member

stijnmoreels commented Dec 16, 2021

Let us know if you come across any problems. We'll hold off with our review until this PR is out of construction 😉 .

Ps.: Oh, and a good tip while writing tests: the project instance has TearDownOptions which is great to see how the template creates the project. TearDownOptions.KeepProjectDirectory will for example keep the generated project after the test is run so you can see on your local disk how the project was created.

@spasal
Copy link
Author

spasal commented Dec 16, 2021

For the other issue about logging levels: we could indeed look into that more after we have established a baseline for reading the configuration in Serilog. To be clear on this: you mean that logging to Azure Application Insights only works with logging levels when both the Serilog and Application Insights-specific sections in the host.json file are updated?

Indeed, that seemed to be the case at one of my projects. If you only specify the logLevels in the Serilog section of the host.json file, more was logged than specified. Only when I also put the logLevels in the applicationInsights logging section in the host.json file, the set logLevels were respected. Note that this was a function that also had some other issues, so will replicate to another function

Will first focus on the goal for this PR, and after it we can look at the potential issue/fix for the custom loggingLevels

@spasal
Copy link
Author

spasal commented Dec 16, 2021

Let us know if you come across any problems. We'll hold off with our review until this PR is out of construction 😉 .

Ps.: Oh, and a good tip while writing tests: the project instance has TearDownOptions which is great to see how the template creates the project. TearDownOptions.KeepProjectDirectory will for example keep the generated project after the test is run so you can see on your local disk how the project was created.

One of the things that would make testing locally easier is if someone could provide me their appsettings.private.json, could that be arranged? :)

@stijnmoreels
Copy link
Member

One of the things that would make testing locally easier is if someone could provide me their appsettings.private.json, could that be arranged? :)

I think you only need the instrumentation key of Application Insights, but sure, I can provide it. For your future contributions 😉

@fgheysels
Copy link
Member

Hi @spasal , is this something you still want to keep working on ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure Function templates should be updated to use configurable serilog settings
3 participants