feat(access-log): omit empty fields in Envoy logs

[abbas-gheydi]

Pull Request

Description

This PR addresses the issue #6067 by implementing the feature to omit empty fields in Envoy logs for the access log.

Changes Made

• Modified the access log to exclude empty fields in Envoy logs.

Related Issue

#6067

[github-actions]

Hi @abbas-gheydi! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

[tsaarni]

Thank you for the PR!

For JSON based logs, omitting values is possibly less likely to break systems that process the logs, but what about text based access logs? I did not try this change myself yet but Envoy docs seem to suggest that fields will be missing from there too

for text_format, the output of the empty operator is changed from - to an empty string, so that empty values are omitted entirely.

How would one reliably count the position of fields, to recognise which field is which, if this is the case?

[sunjayBhatia]

yeah I think I agree with @tsaarni, I would rather this is implemented only for JSON logging if we want it as a default, otherwise text based logs will be hard to read

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (61b6fae) 78.85% compared to head (edce293) 78.85%.
Report is 10 commits behind head on main.

❗ Current head edce293 differs from pull request most recent head 41461d9. Consider uploading reports for the commit 41461d9 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6077   +/-   ##
=======================================
  Coverage   78.85%   78.85%           
=======================================
  Files         138      138           
  Lines       19731    19732    +1     
=======================================
+ Hits        15558    15559    +1     
  Misses       3870     3870           
  Partials      303      303           

Files	Coverage Δ
internal/envoy/v3/accesslog.go	93.44% <100.00%> (+0.05%)	⬆️

[abbas-gheydi]

I removed the 'omit empty fields' feature for Envoy format logs as it may disrupt the log structure. Instead, I've enabled this feature exclusively for JSON logs.

[sunjayBhatia]

LGTM

example with the change:

{"downstream_local_address":"10.244.1.167:8080","@timestamp":"2024-01-22T19:17:35.988Z","upstream_host":"10.244.1.152:8080","bytes_received":0,"upstream_local_address":"10.244.1.167:60970","upstream_service_time":"6","response_flags":"-","x_forwarded_for":"172.18.0.1","method":"GET","user_agent":"curl/8.5.0","upstream_cluster":"default_s1_80","downstream_remote_address":"172.18.0.1:59700","path":"/","authority":"foo-basic.bar.com","protocol":"HTTP/1.1","request_id":"c49948b0-d0b0-4bc5-8ff1-862dbb11874b","bytes_sent":1674,"response_code":200,"duration":8}

vs. without

{"bytes_received":0,"@timestamp":"2024-01-22T19:21:17.543Z","path":"/","upstream_service_time":"5","x_forwarded_for":"172.18.0.1","response_flags":"-","bytes_sent":1674,"downstream_local_address":"10.244.1.175:8080","request_id":"8f3f29dd-e66d-40a4-b288-ee226167a852","upstream_cluster":"default_s1_80","protocol":"HTTP/1.1","grpc_status":null,"method":"GET","downstream_remote_address":"172.18.0.1:59752","user_agent":"curl/8.5.0","upstream_host":"10.244.1.152:8080","duration":6,"requested_server_name":null,"upstream_local_address":"10.244.1.175:48790","grpc_status_number":null,"uber_trace_id":null,"authority":"foo-basic.bar.com","response_code":200}

[izturn]

LGTM