gax-php icon indicating copy to clipboard operation
gax-php copied to clipboard

Make AgentHeader::buildAgentHeader() log either grpc or rest

Open fiboknacky opened this issue 10 months ago • 0 comments

Background

AgentHeader::buildAgentHeader() logs both grpc and rest versions, regardless of whether the versions in the passed parameter ($headerInfo) are null or not.

Although there are some justifications described in lines 87-92, I doubt if this is a correct behavior for all use cases. If the passed parameter contains null for one of the versions, shouldn't we treat it like that the client using this lib intends to log only one of them? If both versions are null, I'd understand that this may signal that something is wrong here and we may want to just log both versions that we can find in the system.

In our use case (google-ads-php), we want to understand if our users use gRPC or REST. We don't have a scenario when they can use both of them at the same time. Thus, the current implementation of buildAgentHeader is blocking us from gathering correct stats.

In addition, this implementation seems to a bit contradictory (or redundant?) with these lines done before calling buildAgentHeader.

Proposal

If that implementation is still a valid use case for some client libraries, I'm fine with that, but I'd love to request an option to log only one of the versions.

I'd leave the implementation details to the lib owners, but what I think about is something like another element passed to $options (named like $nonNullTransportVersionsOnly). Then, this value should be passed to buildAgentHeader() as another parameter to signal whether to respect the grpc and rest versions passed by the client.

If a passed version is null, don't log it. If not, then log it. This means it's fine to log both values if they're deliberately set by the client to be non-null.

fiboknacky avatar Mar 06 '25 06:03 fiboknacky