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
[CC-4365] HCP Metrics xDS Config for Envoy #16585
[CC-4365] HCP Metrics xDS Config for Envoy #16585
Conversation
395ce5d
to
d0c67da
Compare
agent/proxycfg/connect_proxy.go
Outdated
DestinationNamespace: acl.DefaultNamespaceName, | ||
DestinationPartition: s.proxyID.PartitionOrDefault(), | ||
DestinationName: api.HCPMetricsCollectorName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The collector will be deployed in the default namespace of each admin partition using the well-known api.HCPMetricsCollectorName
agent/proxycfg/connect_proxy.go
Outdated
DestinationName: api.HCPMetricsCollectorName, | ||
LocalBindSocketPath: path, | ||
Config: map[string]interface{}{ | ||
"protocol": "grpc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Envoy's metrics service stats sink uses gRPC: https://github.com/envoyproxy/envoy/blob/main/api/envoy/service/metrics/v3/metrics_service.proto#L23
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": { | ||
"@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions", | ||
"explicitHttpConfig": { | ||
"http2ProtocolOptions": {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected http2 protocol options since it is a gRPC service
agent/proxycfg/connect_proxy.go
Outdated
s.logger.Error("failed to parse connect.proxy.config", "error", err) | ||
} | ||
|
||
if hcpCfg.HCPMetricsBindSocketDir != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. We should invert this so that it performs an early return when the value is empty.
307c579
to
8cda1df
Compare
8c65c58
to
4eb82e1
Compare
f739e60
to
d6fadc3
Compare
59d4378
into
cc-4361/hcp-metrics-bootstrap-config
Description
#16511 adds bootstrap configuration so that Envoy is configured to send its telemetry to a statically defined local socket.
This PR builds on top of the previous one by configuring a dynamic listener at that statically configured socket. The changes below inject the HCP metrics collector as an upstream for connect proxies since it is intended to be a mesh service deployed onto the user's cluster.
Why is there dynamic configuration when the cluster is statically defined at bootstrap time?
Why define the collector as an upstream in
proxycfg
?xds
package.Testing & Reproduction steps
PR Checklist