Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP PATCH logged as METHOD_UNSPECIFIED when logging with ALS #6694

Closed
glebmish opened this issue Apr 24, 2019 · 2 comments · Fixed by #6737
Closed

HTTP PATCH logged as METHOD_UNSPECIFIED when logging with ALS #6694

glebmish opened this issue Apr 24, 2019 · 2 comments · Fixed by #6737
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Milestone

Comments

@glebmish
Copy link
Contributor

Title: HTTP PATCH logged as METHOD_UNSPECIFIED when logging with ALS

Description:
I use PATCH requests that go through envoy and logs are written with als implemented on golang. I expect to see "PATCH" as a result of this code:

Instead, I see "METHOD_UNSPECIFIED".
When I tried to find the reason in the library code, I found that PATCH is not in the list of request methods in base.proto (which is used in the library)
https://github.com/envoyproxy/envoy/blob/3cb3571f093602b7d6a06945906802666febcf66/api/envoy/api/v2/core/base.proto

However, if I don't use ALS and let envoy log requests, PATCH is written as expected.

I created an issue in the go-control-plane and they advised to ask here: envoyproxy/go-control-plane#176

Repro steps:
golang sample:
import (
envoy "github.com/envoyproxy/go-control-plane/envoy/data/accesslog/v2"
)

func RequestMethod(ent *envoy.HTTPAccessLogEntry) string {
return ent.GetRequest().GetRequestMethod().String()
}

Config:
access_log:
- name: envoy.http_grpc_access_log
config:
common_config:
log_name: access_log
grpc_service: { envoy_grpc: { cluster_name: als } }
additional_request_headers_to_log: []
additional_response_headers_to_log: []
additional_response_trailers_to_log: []

  • name: als
    connect_timeout: 0.25s
    http2_protocol_options: {}
    common_http_protocol_options:
    idle_timeout: 30s
    upstream_connection_options:
    tcp_keepalive:
    keepalive_probes: 1
    keepalive_time: 10
    keepalive_interval: 10
    type: STATIC
    hosts:
    • socket_address:
      address: 127.0.0.1
      port_value: 4436
@junr03 junr03 added enhancement Feature requests. Not bugs or questions. help wanted Needs help! labels Apr 25, 2019
@junr03
Copy link
Member

junr03 commented Apr 25, 2019

The context is right here. For the ALS to be able to correctly send that value, that value needs to be added to the enum. The text logs don't work off of the proto, so that is why you are seeing the correct value there.

Feel free to submit a PR, contributions are greatly appreciated!

@glebmish
Copy link
Contributor Author

Thank you @junr03 ! I'll try to fix and submit PR next week then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants