api: gRPC Access Log Service (ALS) sink #3078

davidalger · 2024-04-01T14:45:32Z

What type of PR is this?

API to support a gRPC Access Log Service (ALS) sink.

What this PR does / why we need it:

When implemented, a sink may be configured to send log messages to a gRPC stream with rich and strongly typed protobufs including all the details for the request.

We are using a lightweight gRPC logging service to post-process logs (ex: enrich with user session metadata) and then publish them to Stackdriver which in turn routes them to both BigQuery and Pub/Sub for further processing and long-term storage.

Currently we are relying on a patched bootstrap and two XDS patches on each listener to replace the default access logging config with an HttpGrpcAccessLogConfig, but would like to natively support this in the EnvoyProxy CRD in order to keep configuration simple and not rely on having a Terraform configured EnvoyPatchPolicy for each listener on our Gateways.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/access_loggers/grpc/v3/als.proto

Signed-off-by: David Alger <davidmalger@gmail.com>

codecov · 2024-04-01T14:53:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.58%. Comparing base (29946b0) to head (21ae2a5).
Report is 34 commits behind head on main.

❗ Current head 21ae2a5 differs from pull request most recent head dd46082. Consider uploading reports for the commit dd46082 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3078      +/-   ##
==========================================
+ Coverage   66.51%   66.58%   +0.07%     
==========================================
  Files         161      157       -4     
  Lines       22673    21956     -717     
==========================================
- Hits        15080    14620     -460     
+ Misses       6720     6494     -226     
+ Partials      873      842      -31

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

api/v1alpha1/accesslogging_types.go

zhaohuabing · 2024-04-02T03:06:30Z

api/v1alpha1/accesslogging_types.go

+	// +optional
+	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:default="accesslog"
+	LogName string `json:"logName,omitempty"`


Instead of asking users to specify the LogName, can we generate this name in EG?

Since access log filters could be implemented in the future to send different logs to different places my preference would be to leave it a user-configurable 'friendly name' with a simple default of accesslog so it's not mandatory.

If it were generated, we'd have to determine a way to differentiate between different log sinks on the same listener so the name would be useful to differentiate the logs in the receiver.

With the quickstart demo deployed locally, here is a portion of an example log entry generated by our ALS server:

"envoy_node": "envoy-default-eg-9fd49bdd-556b9fdf4864klx", // --service-node value from bootstrap "envoy_version": "1.29.2", "envoy_cluster": "default/eg", // --service-cluster value from bootstrap "envoy_log_name": "accesslog", // logName value from CRD "upstream_cluster": "httproute/default/backend/rule/0", "route_name": "httproute/default/backend/rule/0/match/0/www_example_com"

Just to say that an EG generated name is already present, although missing the listener name.

Given this context and the possibility of arbitrary access log filters resulting in multiple sinks to the same logging service in the future, what are your thoughts?

Thanks for the explanation. Sounds reasonable for me to allow the user to specify the log name.

For the default name, probably add the listener name and the protocol? Like: lisetener1/http/accesslog

Signed-off-by: David Alger <davidmalger@gmail.com>

davidalger · 2024-04-02T16:52:57Z

/retest

zhaohuabing

LGTM. Defer to @arkodg

davidalger · 2024-04-04T20:58:43Z

api/v1alpha1/accesslogging_types.go

+type ALSEnvoyProxyAccessLog struct {
+	// BackendRef references a Kubernetes object that represents the gRPC service to which
+	// the access logs will be sent. Currently only Service is supported.
+	BackendRef gwapiv1.BackendObjectReference `json:"backendRef"`


@arkodg given the conversation surrounding #3091 today, would it be preferred to amend this to pluralize now or address with sweeping pass updating all affected areas (including this one) when #3091 is done for everything else?

prefer if we wait another day or two to make sure #3091 has majority votes, and a decision is made, and that can be incorporated in this PR

looks like there were enough votes to support pluralizing backendRefs , suggest waiting on #3080 which is trying to create a common BackendRef struct that all APIs can reuse

sounds good, thanks for the followup. will wait on that one to polish this one up

Signed-off-by: David Alger <davidmalger@gmail.com>

davidalger · 2024-04-11T15:03:54Z

/retest

davidalger · 2024-04-11T15:11:08Z

/retest

davidalger · 2024-04-11T16:14:56Z

/retest

zirain

LGTM

arkodg · 2024-04-15T05:32:09Z

api/v1alpha1/accesslogging_types.go

+	// ResponseHeaders defines response headers to include in log entries sent to the access log service.
+	// +optional
+	ResponseHeaders []string `json:"responseHeaders,omitempty"`
+	// ResponseTrailers defines response trailers to include in log entries sent to the access log service.


do you have a use case for trailers atm ?

arkodg

LGTM thanks !

fix gen-check Signed-off-by: shawnh2 <shawnhxh@outlook.com>

davidalger requested a review from a team as a code owner April 1, 2024 14:45

davidalger mentioned this pull request Apr 1, 2024

feat: gRPC Access Log Service (ALS) sink #3062

Closed

api: gRPC Access Log Service (ALS) logging sink

5b9ebea

Signed-off-by: David Alger <davidmalger@gmail.com>

davidalger force-pushed the algerdev/als-sink-api branch from 6772d1b to 5b9ebea Compare April 1, 2024 14:48

Merge branch 'main' into algerdev/als-sink-api

ae5788b

zirain reviewed Apr 2, 2024

View reviewed changes

api/v1alpha1/accesslogging_types.go Outdated Show resolved Hide resolved

shawnh2 reviewed Apr 2, 2024

View reviewed changes

api/v1alpha1/accesslogging_types.go Show resolved Hide resolved

zhaohuabing reviewed Apr 2, 2024

View reviewed changes

api/v1alpha1/accesslogging_types.go Outdated Show resolved Hide resolved

zhaohuabing reviewed Apr 2, 2024

View reviewed changes

davidalger added 4 commits April 2, 2024 09:14

Merge branch 'main' into algerdev/als-sink-api

1ef9b74

Make type mandatory and remove kubebuilder default for LogName

f5c13a3

Signed-off-by: David Alger <davidmalger@gmail.com>

Add CEL validations

64890e9

Signed-off-by: David Alger <davidmalger@gmail.com>

Cleanup

a32679e

Signed-off-by: David Alger <davidmalger@gmail.com>

davidalger force-pushed the algerdev/als-sink-api branch from c03da7b to a32679e Compare April 2, 2024 16:12

Fix missing type in validation

21ae2a5

Signed-off-by: David Alger <davidmalger@gmail.com>

davidalger requested review from zhaohuabing, zirain and shawnh2 April 2, 2024 16:24

zirain previously approved these changes Apr 3, 2024

View reviewed changes

zhaohuabing reviewed Apr 3, 2024

View reviewed changes

davidalger commented Apr 4, 2024

View reviewed changes

Merge branch 'main' into algerdev/als-sink-api

8d9e4e4

davidalger dismissed zirain’s stale review via 3581b97 April 11, 2024 14:10

Pluralize backendRefs

dd46082

Signed-off-by: David Alger <davidmalger@gmail.com>

davidalger force-pushed the algerdev/als-sink-api branch from 3581b97 to dd46082 Compare April 11, 2024 14:10

davidalger requested review from arkodg and zirain April 11, 2024 14:18

zirain reviewed Apr 12, 2024

View reviewed changes

arkodg reviewed Apr 15, 2024

View reviewed changes

arkodg approved these changes Apr 15, 2024

View reviewed changes

arkodg merged commit 8f2e175 into envoyproxy:main Apr 15, 2024
20 checks passed

arkodg pushed a commit that referenced this pull request Apr 15, 2024

fix gen-check merge race caused by #3078 (#3196)

3392214

fix gen-check Signed-off-by: shawnh2 <shawnhxh@outlook.com>

davidalger deleted the algerdev/als-sink-api branch April 16, 2024 18:27

arkodg added this to the v1.1.0-rc1 milestone Apr 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: gRPC Access Log Service (ALS) sink #3078

api: gRPC Access Log Service (ALS) sink #3078

davidalger commented Apr 1, 2024

codecov bot commented Apr 1, 2024 •

edited

Loading

zhaohuabing Apr 2, 2024 •

edited

Loading

davidalger Apr 2, 2024

zhaohuabing Apr 2, 2024

davidalger commented Apr 2, 2024

zhaohuabing left a comment

davidalger Apr 4, 2024 •

edited

Loading

arkodg Apr 5, 2024

arkodg Apr 10, 2024

davidalger Apr 10, 2024

davidalger commented Apr 11, 2024

davidalger commented Apr 11, 2024

davidalger commented Apr 11, 2024

zirain left a comment

arkodg Apr 15, 2024

arkodg left a comment

api: gRPC Access Log Service (ALS) sink #3078

api: gRPC Access Log Service (ALS) sink #3078

Conversation

davidalger commented Apr 1, 2024

codecov bot commented Apr 1, 2024 • edited Loading

Codecov Report

zhaohuabing Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

davidalger Apr 2, 2024

Choose a reason for hiding this comment

zhaohuabing Apr 2, 2024

Choose a reason for hiding this comment

davidalger commented Apr 2, 2024

zhaohuabing left a comment

Choose a reason for hiding this comment

davidalger Apr 4, 2024 • edited Loading

Choose a reason for hiding this comment

arkodg Apr 5, 2024

Choose a reason for hiding this comment

arkodg Apr 10, 2024

Choose a reason for hiding this comment

davidalger Apr 10, 2024

Choose a reason for hiding this comment

davidalger commented Apr 11, 2024

davidalger commented Apr 11, 2024

davidalger commented Apr 11, 2024

zirain left a comment

Choose a reason for hiding this comment

arkodg Apr 15, 2024

Choose a reason for hiding this comment

arkodg left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 1, 2024 •

edited

Loading

zhaohuabing Apr 2, 2024 •

edited

Loading

davidalger Apr 4, 2024 •

edited

Loading