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

Not all envoy access log command operators are mapped #2523

Closed
jpeach opened this issue May 14, 2020 · 7 comments
Closed

Not all envoy access log command operators are mapped #2523

jpeach opened this issue May 14, 2020 · 7 comments
Labels
area/operational Issues or PRs about making Contour easier to operate as a production service. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/accepted Denotes an issue that has been triaged and determined to be valid.

Comments

@jpeach
Copy link
Contributor

jpeach commented May 14, 2020

There's no log field mapping for %RESPONSE_CODE_DETAILS%.

https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#command-operators

Looks like we are missing

  • %REQUEST_DURATION%
  • %RESPONSE_DURATION%
  • %RESPONSE_CODE_DETAILS%
  • all the %DOWNSTREAM keys
  • %HOSTNAME

Canonical envoy list of keys looks like it is in source/common/access_log/access_log_formatter.cc

@youngnick
Copy link
Member

When we did the original setup for JSON logging, we used the default JSON logging output of Envoy at the time, with the intent we would add more later. I guess it's later?

@jpeach
Copy link
Contributor Author

jpeach commented May 15, 2020

When we did the original setup for JSON logging, we used the default JSON logging output of Envoy at the time, with the intent we would add more later. I guess it's later?

Makes sense. There are some keys that take parameters which makes them hard to express in JSON, but I think we should just add everything else that's missing.

@jpeach jpeach added this to Unprioritized in Contour Project Board via automation May 15, 2020
@jpeach jpeach added area/metrics Issues or PRs related to exposing time series metrics. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/accepted Denotes an issue that has been triaged and determined to be valid. labels May 15, 2020
@Tazminia
Copy link

Hello, I just discovered the project although, I have been working with kubernetes and ingress controllers for a few months now (mainly nginx). I would like to give you a hand if you want, but I will probably need some assistance

@jpeach
Copy link
Contributor Author

jpeach commented May 25, 2020

Hello, I just discovered the project although, I have been working with kubernetes and ingress controllers for a few months now (mainly nginx). I would like to give you a hand if you want, but I will probably need some assistance

That sounds great! Take a look at our contribution guide to get started. Contour people hang out in slack if you have questions.

For this PR, it should be a matter of adding field mappings for all the Envoy log fields that don't require a parameter (there's no way to specify parameters in JSON). I'll be happy to shepherd your changes.

@Tazminia
Copy link

So if I get it right, the ones we are not willing to map are the following:

  • %REQ(X?Y):Z%
  • %RESP(X?Y):Z%
  • %TRAILER(X?Y):Z%
  • %DYNAMIC_METADATA(NAMESPACE:KEY*):Z%
  • %FILTER_STATE(KEY:F):Z%

There are the fields that look like they need some paramters.

@jpeach
Copy link
Contributor Author

jpeach commented May 26, 2020

So if I get it right, the ones we are not willing to map are the following:

  • %REQ(X?Y):Z%
  • %RESP(X?Y):Z%
  • %TRAILER(X?Y):Z%
  • %DYNAMIC_METADATA(NAMESPACE:KEY*):Z%
  • %FILTER_STATE(KEY:F):Z%

There are the fields that look like they need some paramters.

Yep, that sounds about right. It would be good to think about how to support these kinds of logging keys, but that's a larger problem and we can leave that to a separate issue :)

@stevesloka stevesloka added the Hacktoberfest Denotes an issue ready for any "Hacktoberfest" contributor. label Oct 1, 2020
@jpeach jpeach changed the title Not all envoy log command operators are mapped Not all envoy access log command operators are mapped Oct 18, 2020
@jpeach jpeach added area/operational Issues or PRs about making Contour easier to operate as a production service. and removed area/metrics Issues or PRs related to exposing time series metrics. labels Oct 18, 2020
@stevesloka stevesloka removed the Hacktoberfest Denotes an issue ready for any "Hacktoberfest" contributor. label Nov 5, 2020
@sunjayBhatia
Copy link
Member

this is out of date, the access log operators listed here are all supported

Contour Project Board automation moved this from Unprioritized to 1.21 release (candidates) Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operational Issues or PRs about making Contour easier to operate as a production service. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/accepted Denotes an issue that has been triaged and determined to be valid.
Projects
No open projects
Contour Project Board
  
1.21 release (candidates)
Development

No branches or pull requests

5 participants