envoyproxy / gateway

Manages Envoy Proxy as a Standalone or Kubernetes-based Application Gateway
https://gateway.envoyproxy.io
Apache License 2.0
1.55k stars 335 forks source link

Support Listener Access Logging #3600

Closed guydc closed 6 days ago

guydc commented 3 months ago

Description: Envoy supports several types of Access Loggers. The access log that is most commonly in use is configured in HCM or TCP/UDP/Thrift Proxy. Envoy additionally supports a listener-level access log. The listener access log is particularly useful for:

Currently, Envoy Gateway uses the user-defined access log format for the listener access log, but the listener access log is only emitted when there is no matching filter chain.

https://github.com/envoyproxy/gateway/blob/0abdda7a033f0e37647177a588a904c90b783149/internal/xds/translator/accesslog.go#L216

Envoy Gateway should also support listener access logs for TLS troubleshooting. Common issues include:

To support the TLS troubleshooting use case, Envoy Gateway may:

  1. Make the listener access log fully configurable for end-users (including format, filters, etc.)
  2. Conditionally emit a listener access log containing the %DOWNSTREAM_TRANSPORT_FAILURE_REASON% when a TLS connection error occurs. This can be done by appending the TLS details to the user-defined/default access log format, or by emitting a different log that is fully controlled by EG and contains relevant information such as Client IP, Available TLS Context (SNI, Cert details, ...)

To filter listener logs based on a TLS failure, a CEL access log filter can be used:

filter:
extension_filter:
  name: envoy.access_loggers.extension_filters.cel
  typed_config:
    '@type': type.googleapis.com/envoy.extensions.access_loggers.filters.cel.v3.ExpressionFilter
    expression: connection.transport_failure_reason != "-"

To support general-purpose listener access logs, the Telemetry Proxy Access Log Setting API may be extended with a new source attributes with values network (default, TCP/HTTP Proxy Log) and listener. For example:

  telemetry:
    accessLog:
      settings:
      - format:
          source: [network|listener] 
          text: |
            [%START_TIME%] "%DOWNSTREAM_REMOTE_ADDRESS%" "%DOWNSTREAM_TRANSPORT_FAILURE_REASON%" "%CONNECTION_TERMINATION_DETAILS%" "%CONNECTION_ID%" "%REQUESTED_SERVER_NAME%"
          type: Text
        matches:
          - 'connection.transport_failure_reason != "-"'
        sinks:
          - file:
              path: /dev/stdout
            type: File

Alternatively, a new telemetry attribute called listenerAccessLog can be supported:

  telemetry:
    listenerAccessLog:
      settings:
      - format:
          text: |
            [%START_TIME%] "%DOWNSTREAM_REMOTE_ADDRESS%" "%DOWNSTREAM_TRANSPORT_FAILURE_REASON%" "%CONNECTION_TERMINATION_DETAILS%" "%CONNECTION_ID%" "%REQUESTED_SERVER_NAME%"
          type: Text
        matches:
          - 'connection.transport_failure_reason != "-"'
        sinks:
          - file:
              path: /dev/stdout
            type: File
guydc commented 3 months ago

Notes from today's community meeting:

zhaohuabing commented 3 months ago

Emit the user-defined access log

@guydc @arkodg does this mean EG will have a separate use-defined access log format for listener-level log? From the envoy docs, listener-specific fields seem very limited.

zirain commented 3 months ago

TBH, is this a common use case? can it be covered by using EnvoyPatchPolicy (or ExtensionManager) instead of introuduce an new API?

arkodg commented 3 months ago

@zhaohuabing my vote would be to use the same format thats defined in envoyProxy.spec.telemetry for all access log cases, and use the default value even for the listener access log case

guydc commented 3 months ago

is this a common use case?

I would say that observability for TLS failures, especially when client certificate auth is a feature, is important. Having said that, most proxies support this sort of troubleshooting through error logs, rather than access log.

can it be covered by using EnvoyPatchPolicy (or ExtensionManager) instead of introuduce an new API?

Yes. But, the current proposal is not to extend the API. Rather, we will emit listener access logs in more scenarios and not just NR.

guydc commented 3 months ago

In light of #3688, another approach here would be to make Listener log matcher configurable. This will:

@arkodg , @zirain - WDYT?

zirain commented 3 months ago

In light of #3688, another approach here would be to make Listener log matcher configurable. This will:

  • not change current default filters (only log for NR) or format
  • allow users to emit listener access log for TLS troubleshooting or other reasons (e.g. observability for TCP connections in general)

@arkodg , @zirain - WDYT?

sounds good

zhaohuabing commented 3 months ago

In light of https://github.com/envoyproxy/gateway/pull/3688, another approach here would be to make Listener log > matcher configurable. This will:

not change current default filters (only log for NR) or format allow users to emit listener access log for TLS troubleshooting or other reasons (e.g. observability for TCP connections in general)

Should the connection.transport_failure_reason != "-" and other listener-level errors(if any) be include in the default filters?

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days.