Skip to content

[CC-4365] HCP Metrics xDS Config for Envoy#16585

Merged
freddygv merged 2 commits into
cc-4361/hcp-metrics-bootstrap-configfrom
cc-4365/hcp-metrics-dynamic-listener
Mar 10, 2023
Merged

[CC-4365] HCP Metrics xDS Config for Envoy#16585
freddygv merged 2 commits into
cc-4361/hcp-metrics-bootstrap-configfrom
cc-4365/hcp-metrics-dynamic-listener

Conversation

@freddygv
Copy link
Copy Markdown
Contributor

@freddygv freddygv commented Mar 9, 2023

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?

  • We want to secure the metrics stream using TLS, but the stats sink can only be defined in bootstrap config. With dynamic listeners/clusters we can use certificates issued by the connect CA, which aren't available at bootstrap time.
  • We want to intelligently route to the HCP collector. Configuring its address at bootstrap time limits our flexibility routing-wise compared to providing clusters/endpoints dynamically using xDS. More on this below.

Why define the collector as an upstream in proxycfg?

  • Service discovery and routing logic is automatically taken care of, meaning that no code changes are required in the xds package.
  • Certificate management is taken care of. Each proxy will dial using the certificate of the proxy it represents, and the HCP collector can present its own certificate as well as check intentions.
  • Custom routing rules can be added for the collector using discovery chain config entries. Initially the collector is expected to be deployed to each admin partition, but in the future could be deployed centrally in the default partition. These config entries could be managed by HCP itself.

Testing & Reproduction steps

  • Added unit tests at the proxycfg and xds layers.
  • Tested against live Consul client agent and consul-dataplane instance, and verified that metrics do get sent to a collector service registered with the expected name.

PR Checklist

  • updated test coverage
  • external facing docs updated
  • not a security concern

@freddygv freddygv requested review from a team, hashi-derek and zalimeni and removed request for a team March 9, 2023 17:14
@github-actions github-actions Bot added theme/api Relating to the HTTP API interface theme/cli Flags and documentation for the CLI interface theme/envoy/xds Related to Envoy support labels Mar 9, 2023
@freddygv freddygv force-pushed the cc-4365/hcp-metrics-dynamic-listener branch from 395ce5d to d0c67da Compare March 9, 2023 17:16
Comment thread agent/proxycfg/connect_proxy.go Outdated
Comment on lines 662 to 664
Copy link
Copy Markdown
Contributor Author

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

Comment thread agent/proxycfg/connect_proxy.go Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

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

Comment thread agent/proxycfg/connect_proxy.go Outdated
Copy link
Copy Markdown
Contributor

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.

@Achooo Achooo force-pushed the cc-4361/hcp-metrics-bootstrap-config branch from 307c579 to 8cda1df Compare March 10, 2023 13:30
@Achooo Achooo requested a review from a team as a March 10, 2023 13:30
@Achooo Achooo force-pushed the cc-4361/hcp-metrics-bootstrap-config branch from 8c65c58 to 4eb82e1 Compare March 10, 2023 19:28
@freddygv freddygv force-pushed the cc-4365/hcp-metrics-dynamic-listener branch from f739e60 to d6fadc3 Compare March 10, 2023 20:00
@freddygv freddygv merged commit 59d4378 into cc-4361/hcp-metrics-bootstrap-config Mar 10, 2023
@freddygv freddygv deleted the cc-4365/hcp-metrics-dynamic-listener branch March 10, 2023 20:16
@hashicorp hashicorp deleted a comment from sas65-wed May 17, 2023
@hashicorp hashicorp deleted a comment from sas65-wed May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme/api Relating to the HTTP API interface theme/cli Flags and documentation for the CLI interface theme/envoy/xds Related to Envoy support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants