May we just try to find the grpcgatewayuser-agent key and use it if found or fallback to User-Agent otherwise?

I think the code here has clearer intent: if grpc-gateway is used, use grpc-gateway header; if it does not work or we are not on gRPC protocol, use HTTP header. It would be unexpected to see grpcgateway-user-agent in the HTTP interceptor, and I would think we should ignore such header in such context.

I think there is too much code for this intent: you even had to add more parameters to two functions and compute the normal User-Agent twice.
Instead, you could move this grpc related check (i.e. try to read the grpcgatewayuser-agent header, and you don't need any bool parameter) to the beginning of the function, and put the existing User-Agent retrieval under else.

➕

FWIW, with such branching you don't need userAgent declaration at L43. If you keep the current structure, pull L43 right before branching and make it var userAgent string. However, I'm in favor of Michaël's suggestion below.

➕

➕

I understand that metadata.MD converts keys to lowercase but this is not obvious here, is not common knowledge, and who knows when it changes. Canonical form is "User-Agent".

Always press y in GitHub to get a link with pinned line references.

It's actually not clear to me what this code reference proves. cc.dopts.copts.UserAgent += " " + grpcUA hints that User-Agent value is not overwritten but appended to, so it will be something like ACS roxctl user agent blah grpc-go/1.50.1. Which sort of contradicts what's claimed in ROX-13596.

Could you please split the "Testing Performed" section into before / after?

Regarding cc.dopts.copts.UserAgent: this shows where the user agent's value grpc-go/1.51.0 is initially taken from.

Answering the question of why there's no other user-agent value:

1. To transform from HTTP to GRPC request, we connect to our gRPC server as a client from within the central
   

   stackrox/pkg/grpc/server.go

   Lines 352 to 353 in 0529062

   	dialCtxFunc := a.listenOnLocalEndpoint(grpcServer)
   	localConn, err := a.connectToLocalEndpoint(dialCtxFunc)

   
   

   stackrox/pkg/grpc/server.go

   Lines 311 to 317 in 0529062

   	if localConn != nil {
   	for _, service := range a.apiServices {
   	if err := service.RegisterServiceHandler(context.Background(), gwMux, localConn); err != nil {
   	log.Panicf("failed to register API service: %v", err)
   	}
   	}
   	}

2. Connection for this client is created once during central initialization, thus we cannot override it on every request.
3. Value for user-agent dial option is not specified in our code, thus we use the default
   

   stackrox/pkg/grpc/server.go

   Lines 226 to 233 in 0529062

   	func (a *apiImpl) connectToLocalEndpoint(dialCtxFunc pipeconn.DialContextFunc) (*grpc.ClientConn, error) {
   	return grpc.Dial("", grpc.WithTransportCredentials(insecure.NewCredentials()),
   	grpc.WithContextDialer(func(ctx context.Context, endpoint string) (net.Conn, error) {
   	return dialCtxFunc(ctx)
   	}),
   	grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(maxResponseMsgSize())),
   	)
   	}

Ah, I see, didn't know that we persist the connection — thanks. This makes sense.

It does not make sense to capture your reply as a code comment because it builds on non-local knowledge which might change any time and render the comment misleading, but it does make sense to explain that in the PR description.

One last thing: do you want to follow up and augment grpc server init code with User-Agent for HTTP muxer?

One last thing: do you want to follow up and augment grpc server init code with User-Agent for HTTP muxer?

If I understand correctly what you suggest, that would add some additional user-agent value to all requests going through the muxer. Essentially, what @0x656b694d did here:
https://github.com/stackrox/stackrox/pull/3672/files#diff-af62cb3a3a7c777bd1b8feed18657c3ecae951d2dbdea55527f08c8c67967c71R233

I think it works as a fallback option. It's better than displaying grpc-go/1.51.0, so why not?

I had a small concern regarding introducing more variety to potential user-agent values in Segment, but Segment supports contains filter, so that shouldn't be a problem.

Yeah, essentially what's done in #3672, so my comment should be irrelevant. Why do we still see "User-Agent": "grpc-go/1.51.0" in the Before section then?

"grpc-go/1.51.0" is always appended by the gogrpc library to the User-Agent, should we set or not anything with WithUserAgent.