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

Eight Additional layout renders for NLog.Web #784

Merged
merged 12 commits into from
Jun 4, 2022

Conversation

bakgerman
Copy link
Contributor

@bakgerman bakgerman commented Jun 1, 2022

Eight additional layout renders

Client certificate

Remote Port
Local Address
Local Port

Is Web socket request
Web socket requested protocols

Response Content Type
Response Headers

Response Cookies will be separate PR since that require add of Microsoft.AspNetCore.Http.Extensions NuGet package.

@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2022

Codecov Report

Merging #784 (b706802) into master (55a7fca) will increase coverage by 4%.
The diff coverage is 76%.

@@          Coverage Diff           @@
##           master   #784    +/-   ##
======================================
+ Coverage      63%    67%    +4%     
======================================
  Files          48     58    +10     
  Lines         790    909   +119     
  Branches      196    226    +30     
======================================
+ Hits          498    606   +108     
+ Misses        223    212    -11     
- Partials       69     91    +22     
Impacted Files Coverage Δ
...rs/AspNetRequestClientCertificateLayoutRenderer.cs 50% <50%> (ø)
...enderers/AspNetRequestIsWebSocketLayoutRenderer.cs 60% <60%> (ø)
...equestWebSocketNegotiatedProtocolLayoutRenderer.cs 60% <60%> (ø)
...outRenderers/AspNetRequestLocalIpLayoutRenderer.cs 67% <67%> (ø)
...tRenderers/AspNetRequestLocalPortLayoutRenderer.cs 67% <67%> (ø)
...Renderers/AspNetRequestRemotePortLayoutRenderer.cs 67% <67%> (ø)
...nderers/AspNetResponseContentTypeLayoutRenderer.cs 71% <71%> (ø)
...utRenderers/AspNetResponseHeadersLayoutRenderer.cs 78% <78%> (ø)
...equestWebSocketRequestedProtocolsLayoutRenderer.cs 80% <80%> (ø)
...outRenderers/AspNetLayoutMultiValueRendererBase.cs 87% <92%> (+2%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55a7fca...b706802. Read the comment docs.

@bakgerman
Copy link
Contributor Author

I see, I need to fix 4 code smell, I will do that soon.

@sonarcloud
Copy link

sonarcloud bot commented Jun 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

88.5% 88.5% Coverage
0.0% 0.0% Duplication

@bakgerman bakgerman changed the title Nine Additional layout renders for NLog.Web Eight Additional layout renders for NLog.Web Jun 3, 2022
@snakefoot snakefoot added this to the 5.0.1 milestone Jun 4, 2022
@snakefoot
Copy link
Contributor

Think it looks great, and excellent unit-test coverage.

@snakefoot
Copy link
Contributor

Created #786 that changes AspNetRequestIsWebSocketLayoutRenderer from "true" / "false" to "1" / "0"

@snakefoot
Copy link
Contributor

snakefoot commented Jun 4, 2022

Created wiki-pages:

https://github.com/NLog/NLog/wiki/AspNet-Request-Duration-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-ContentLength-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Response-ContentLength-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-Client-Certificate-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-IsWebSocket-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-Local-IP-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-Local-Port-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-Remote-Port-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-WebSocket-Requested-Protocols-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Response-ContentType-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Response-Headers-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Response-Cookie-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Response-HasStarted-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-ConnectionId-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-TLS-Handshake-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-ServerVariable-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-Bidirectional-Stream-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-HTTP-Transport-Type-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-Inherent-KeepAlive-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-TLS-Token-Bindng-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-Tracking-Consent-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Request-Trailers-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Response-Trailers-Layout-Renderer
https://github.com/NLog/NLog/wiki/AspNet-Response-HTTPS-Compression-Layout-Renderer

@bakgerman
Copy link
Contributor Author

Think it looks great, and excellent unit-test coverage.

You are welcome!

@snakefoot
Copy link
Contributor

I guess if we should follow semver, then with all these new features/funcationality, then next version should be v5.1 and not just v5.0.1

@bakgerman
Copy link
Contributor Author

I guess if we should follow semver, then with all these new features/funcationality, then next version should be v5.1 and not just v5.0.1

I agree. With response cookie PR and the earlier PR that is a total of 9 new layout renders.

I am going to start work on a 'public bool Verbose' property for response cookie layout renderer that will also log the domain, path, expire, etc of the cookie. For the Json output format, i will do just with StringBuilder.Append() and not use System.text.Json or Newtonsoft.Json.

Then I will add the response body to the request body middleware. This one I already have a version working for 18 months at my employer.

Then I will try to do HttpModule version of request and response body logging. I find a legacy .NET 4.8 ASPX application at my employer I can test this with.

After that I may look at up-for-grab features at NLog.

@snakefoot
Copy link
Contributor

snakefoot commented Jun 6, 2022

I will add the response body to the request body middleware. This one I already have a version working for 18 months at my employer.

Then I will not press the release-button just yet.

I will try to do HttpModule version of request and response body logging. I find a legacy .NET 4.8 ASPX application at my employer I can test this with.

Notice #785 have introduced a basic HttpModule for ASP.NET Classic (NLogRequestPostedBodyModule)

@snakefoot snakefoot added ASP.NET 4 ASP.NET MVC Classic ASP.NET Core ASP.NET Core - all versions labels Jun 6, 2022
@snakefoot
Copy link
Contributor

Updated https://nlog-project.org/config/?tab=layout-renderers with the new LayoutRenderers for NLog.Web.AspNetCore v5.1

@snakefoot
Copy link
Contributor

Updated https://nlog-project.org/documentation/v5.0.0/html/N_NLog_Web_LayoutRenderers.htm with the new LayoutRenderers for NLog.Web.AspNetCore v5.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASP.NET Core ASP.NET Core - all versions ASP.NET 4 ASP.NET MVC Classic feature size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants