HttpServerConnection: always log request filter if any, for debugging

[Al2Klimov]

Even if it's in the request body.

Tests

No filter

[2024-01-16 17:38:23 +0100] information/HttpServerConnection: Request POST https://127.0.0.1:5665/v1/console/execute-script?session=3270231b-f3fe-401d-a77b-2a5e9d3b0fd6&command&sandboxed=0 (from [::ffff:127.0.0.1]:58972), user: root, agent: Icinga/DebugConsole/v2.14.0-92-gf075d1e0d, status: OK) took 1ms.

URL filter

[2024-01-16 17:40:35 +0100] information/HttpServerConnection: Request GET /v1/objects/services?filter=1 (from [::ffff:127.0.0.1]:58978), user: root, agent: curl/8.1.2, status: OK) took 31ms.

Body filter

[2024-01-16 17:39:13 +0100] information/HttpServerConnection: Request GET /v1/objects/services (from [::ffff:127.0.0.1]:58973), user: root, agent: curl/8.1.2, status: OK) took 0ms. Service filter: y == service.name && x == host.name

Long body filter

[2024-01-17 11:11:45 +0100] information/HttpServerConnection: Request GET /v1/objects/services (from [::ffff:127.0.0.1]:59835), user: root, agent: curl/8.1.2, status: OK) took 1ms. Service filter: y == service.name && x == host.name // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

Too long body filter

[2024-01-17 11:11:25 +0100] information/HttpServerConnection: Request GET /v1/objects/services (from [::ffff:127.0.0.1]:59834), user: root, agent: curl/8.1.2, status: OK) took 1ms. Service filter: y == service.name && x == host.name // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx...

👍

[julianbrost]

IMHO this needs a bit more consideration, nothing for a quick release.

[julianbrost]

At least those should be clearly marked as output parameters, at least from the signature it's not really clear why user is an input parameter but url and params are not. For response, on can at least argue that this seems likely that a function ProcessRequest() generates a response.

But in general, a version where such indirect passing around of values wasn't needed at all might be preferably, maybe by doing the parameter evaluation where it's needed or moving more specific logging to the inside where more context is available (and only logging errors in the existing place).

Maybe something like a more explicit "extra values to be logged" output parameter could be an option. That could also be set to the concrete relevant values when they are actually used rather then extracting them separately in the logging code.

[Al2Klimov]

A matter of taste I'd say. I prefer to keep all the log stuff in that place.

[julianbrost]

The way it's currently implemented, this places a non-obvious assumption on the rest of this function and all the functions the references to url and param are passed to: if they set param, they must also set url, otherwise this could result in a null pointer dereference.

So if we decide to keep the general current implementation, this should probably also check && url in the if at the top to only become active if both values were set to non-null values.