SAP / cf-java-logging-support

The Java Logging Support for Cloud Foundry supports the creation of structured log messages and the collection of request metrics
Apache License 2.0
76 stars 46 forks source link

AddHttpHeaderToLogContextFilter: Filter immediately for getField presence #184

Closed j-denner closed 1 month ago

j-denner commented 2 months ago

AddHttpHeaderToLogContextFilter: Filter immediately for getField presence

j-denner commented 2 months ago

I phrase it the other way round. There is the invariant propagated=true => field != null that is currently not enforced in any location. Such a check could be added, but then it should be introduce in all places that accept HttpHeader as a configuration. Since HttpHeader is an interface consumers may have derived classes that do no fulfill this contract.

The code has probably little affect.

I think no. When field is null, the retrieved header value is ignored. So the early filtering reduces work - like for fields.

j-denner commented 2 months ago

I don't known how many breaking changes you 'dare' with the next major release. The HttpHeader interface could be changed to a sealed one, that allows the current Enum and a new HttpHeaderRecord record class. The latter can enforce the contract and consumers can still create own Headers. Just some thoughts.

KarstenSchnitter commented 2 months ago

For the next major release, I am thinking of moving the HttpHeader interface from the core module to the servlet module. This would already be a breaking change. Implementing your suggestion would be an additional and possible option.