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

Proxy supplies logs with relevant info for each stage of the request #660

Closed
MihaZupan opened this issue Jan 12, 2021 · 14 comments · Fixed by #1286
Closed

Proxy supplies logs with relevant info for each stage of the request #660

MihaZupan opened this issue Jan 12, 2021 · 14 comments · Fixed by #1286
Assignees
Labels
Priority:0 Used for divisional .NET planning Type: Documentation Improvements or additions to documentation User Story Used for divisional .NET planning
Milestone

Comments

@MihaZupan
Copy link
Member

Opening a new issue as discussion in #115 was unrelated.

@karelz
Copy link
Member

karelz commented Jan 14, 2021

Triage: We need to check that we are logging useful info about request progress and failures in YARP (E2E).

@karelz karelz changed the title Proxy supplies logs for with pertinent info for each stage of the request Proxy supplies logs with relevant info for each stage of the request Jan 14, 2021
@samsp-msft samsp-msft added the Priority:0 Used for divisional .NET planning label Jan 20, 2021
@luizfbicalho
Copy link

luizfbicalho commented Jan 26, 2021

How can I enable logging on the yarp, I added this code but nothing happens.
I'm with preview 1.0.0-preview.9.21075.2

 services.AddLogging(builder => builder
              .AddConsole().AddDebug()
          );
            services.AddReverseProxy()
                .LoadFromConfig(Configuration.GetSection("ReverseProxy"));
            services.AddTelemetryListeners();

@luizfbicalho
Copy link

luizfbicalho commented Jan 26, 2021

Sorry @Tratcher I didn't explain my point, this code returns me my log

 endpoints.MapReverseProxy(proxyPipeline =>
                {
                    proxyPipeline.Use(async (context, next) =>
                    {

                        var provider = context.RequestServices;
                        var logger = provider.GetService<ILogger<Startup>>();
                        logger.Log(LogLevel.Information, "TEST");

But I couldn't make the log from the requests on the other server to be logged, I have an error 500 in the proxy that I can't reproduce, and I would like to see all of the information that is going and coming back to find ot what is going on this request.

Thanks for the help

@karelz
Copy link
Member

karelz commented Jan 28, 2021

@MihaZupan plans to write docs about Telemetry in #676. The linked consumption sample may help?

@luizfbicalho

This comment has been minimized.

@karelz
Copy link
Member

karelz commented Feb 4, 2021

@luizfbicalho if you have troubles with specific case, let's move it to separate issue. This is User Story describing the general capability, not troubleshooting specific bugs / shortcomings.

@Tratcher
Copy link
Member

The log INFO: Microsoft.ReverseProxy.Service.Proxy.HttpProxy Proxying to "https://vmx0007223:9999/graphql" should include more fields like method, protocol version, version policy, content-length, etc. comparable to the ASP.NET event INFO: Microsoft.AspNetCore.Hosting.Diagnostics Request starting HTTP/2 POST https://vmx0007223:8443/api/graphql application/json 406
Do we need a matching log for the response, or is the ASP.NET log sufficient?
INFO: Microsoft.AspNetCore.Hosting.Diagnostics Request finished HTTP/2 POST https://vmx0007223:8443/api/graphql application/json 501 - 502 0 - 56.7773ms

@karelz
Copy link
Member

karelz commented Mar 29, 2021

Triage: We need to test few important scenarios and review what we log from them.

@samsp-msft samsp-msft removed this from the YARP 1.0.0 milestone Jul 2, 2021
@samsp-msft
Copy link
Contributor

With the output included in the default console log, and the samples for more detailed telemetry and metrics, IMHO we have this covered.
Removing the milestone so it gets shown in triage again.

@Tratcher
Copy link
Member

@samsp-msft please run a few error scenarios and see if you get the logs you expect.

@Tratcher Tratcher added this to the YARP 1.0.0 milestone Jul 13, 2021
@samsp-msft
Copy link
Contributor

Playing with this a bit more, I think in the default log we could be including more information about the destination request/response.

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 POST https://localhost:5001/test text/plain 55
info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0]
      Executing endpoint 'minimumroute'
info: Yarp.ReverseProxy.Forwarder.HttpForwarder[9]
      Proxying to http://www.example.com/test
info: Microsoft.AspNetCore.Routing.EndpointMiddleware[1]
      Executed endpoint 'minimumroute'
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished HTTP/1.1 POST https://localhost:5001/test text/plain 55 - 404 445 text/html;+charset=utf-8 85.3726ms

What I could be considered missing is the destination IP, protocol version, protocol version policy.

The following should not change from the request: method, content size
The response content size and status code are reported by the proxy - assuming that the proxy hasn't changed them.

I suspect the main question is who needs the data and when? For general purpose logging - its probably sufficient.

However in the case that there is a problem, and the user is trying to determine what has gone wrong then its probably insufficient. In those cases having log entries that detail the request and response header stats would be useful. Ideally this would be a feature included in the proxy that can be turned on and off via configuration (which enables commandline and env variables) and possibly through a request header. It could be delivered via a sample, but seems like it would be better to be baked in as a standard module. It really needs to run after the transforms so is not readily insertable by the user.

@Tratcher
Copy link
Member

Tratcher commented Sep 1, 2021

What I could be considered missing is the destination IP, protocol version, protocol version policy.

The destination IP isn't available at the YARP layer, that's managed internally by SocketsHttpHandler.

The requested protocol version and policy make sense to add.

However in the case that there is a problem, and the user is trying to determine what has gone wrong then its probably insufficient. In those cases having log entries that detail the request and response header stats would be useful. Ideally this would be a feature included in the proxy that can be turned on and off via configuration (which enables commandline and env variables) and possibly through a request header. It could be delivered via a sample, but seems like it would be better to be baked in as a standard module. It really needs to run after the transforms so is not readily insertable by the user.

.NET 6 has a new logging middleware for this. Give it a try?
https://github.com/dotnet/aspnetcore/blob/05008201d4d5f29d5f348cc26b47b5e7e73fa410/src/Middleware/HttpLogging/samples/HttpLogging.Sample/Startup.cs#L27

@samsp-msft
Copy link
Contributor

Now that the filtering list has been updated, the logging middleware has sufficient data. I am going to change this to a doc topic.

@samsp-msft samsp-msft added the Type: Documentation Improvements or additions to documentation label Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:0 Used for divisional .NET planning Type: Documentation Improvements or additions to documentation User Story Used for divisional .NET planning
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants