[BugFix] Fix bug with logging request headers.

[vvivekiyer]

In PR https://github.com/apache/pinot/pull/8907, RequestIdentity is assumed to be of type HttpRequestIdentity. Using GrpcRequesterIdentity will result in error like cannot be cast to class org.apache.pinot.broker.api.HttpRequesterIdentity.

Potential fix is to have a function in RequesterIdentity called getClientIp that is implemented by all subclasses.

[vvivekiyer]

@abhs50 would you be able to pick this up?

[Jackie-Jiang]

We may also add getHttpHeaders() to the interface, and by default returns an empty map

[abhs50]

Yes I will look into this.

[abhs50]

@vvivekiyer : Just to understand the design here, are you suggesting we move https://github.com/apache/pinot/blob/67344859c270b9e64638b6691d456e57a7570403/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java#L671 inside a standalone method called getClientIp in org.apache.pinot.broker.api.HttpRequesterIdentity ?

[abhs50]

Another option is we could also do below to avoid the issue and explicit check if (enableClientIpLogging && requesterIdentity instanceof HttpRequesterIdentity) instead of https://github.com/apache/pinot/blob/67344859c270b9e64638b6691d456e57a7570403/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java#L678

[vvivekiyer]

IMO, I would avoid using instanceOf to achieve this. Here's the reason for the same.

What I was suggesting is as follows: Have a method called getClientIp in RequesterIdentity. The default implementation can be to return "unknown"/empty. Allow all the subclasses to have their own implementation for getClientIp. The implementation of getClientIp in HttpRequesterIdentity would borrow the code from here.