grpc access logs: include passed in command parsers when validating commands

[AntonKanug]

Fixes: #43396

Commit Message: grpc access logs: include passed in command parsers when validating commands
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:

Fixes listener initializing errors when using custom tag substitution for dns filter logs.

delta config for type.googleapis.com/envoy.config.listener.v3.Listener rejected: Error adding/updating listener(s) listener: Not supported field in StreamInfo: QUERY_NAME

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #42915 was opened by AntonKanug.

see: more, trace.

[AntonKanug]

cc @wbpcode

[wbpcode]

Thanks for this contribution. But

1. I think to configure DNS's related format in the listener make no sense anyway?
2. We prefer only keep HTTP and TCP's format as built-in and to reduce the potential name conflict?

[AntonKanug]

1. I think to configure DNS's related format in the listener make no sense anyway?

We use a udp listener with dns filter (similar to this example)

2. We prefer only keep HTTP and TCP's format as built-in and to reduce the potential name conflict?

whats the suggestion to fix envoy rejecting listener config due to command not being recognized in this case?

delta config for type.googleapis.com/envoy.config.listener.v3.Listener rejected: Error adding/updating listener(s) listener: Not supported field in StreamInfo: QUERY_NAME

[wbpcode]

We use a udp listener with dns filter (similar to this example)

Sorry, I didn't make it clear. I mean, at the listener level, we cannot access the single DNS query attributes anyway, right? Then, we we need to configure the DNS command that? Could we remove the UDP listener's access logger or to update listener level format?

[wbpcode]

/wait

[AntonKanug]

From slack conversation:

the issue seems to be that non built in command parsers are not taken into account when validating commands, updated PR to fix that

[yanjunxiang-google]

Could you please add some integration tests to reproduce the issue and verify the change?

[yanjunxiang-google]

Maybe put the new access log integration tests in a new file: dns_filter_access_log_integration_test.cc?

Otherwise LGTM!

[AntonKanug]

done in 20c6ad0

[AntonKanug]

@yanjunxiang-google @wbpcode
#43457 was opened after this pr with the same changes and was merged without the integration tests 😕

cc @farmpiggie

[botengyao]

Need to resolve conflicts.

And pinging @yanjunxiang-google @wbpcode for more reviews.

[wbpcode]

I think @yanavlasov didn't aware this when merge anohter PR. Sorry @AntonKanug. But the integration tests self still make sense. Could you merge main and then we can merge this new tests? Anyway, I think your problem has been resolved?
Sorry again for that because I was on a long vacation and cannot review the PR in time.

[yanjunxiang-google]

Agree! At this point, please merge main and commit the tests added. Thanks for the contribution! @AntonKanug

[yanjunxiang-google]

LGTM

[RyanTheOptimist]

Looks like this needs a main merge
/wait