design(docs/api): control plane metrics monitoring #1981

Xunzhuo · 2023-10-17T08:31:09Z

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

design(docs/api): control plane metrics monitoring

Part of: #700

codecov · 2023-10-17T08:48:56Z

Codecov Report

Merging #1981 (57a2ffa) into main (a858547) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1981      +/-   ##
==========================================
- Coverage   65.25%   65.22%   -0.04%     
==========================================
  Files          97       97              
  Lines       14199    14213      +14     
==========================================
+ Hits         9266     9270       +4     
- Misses       4359     4366       +7     
- Partials      574      577       +3

Files	Coverage Δ
api/v1alpha1/validation/envoygateway_validate.go	`86.66% <100.00%> (+1.76%)`	⬆️
api/v1alpha1/validation/envoyproxy_validate.go	`75.00% <100.00%> (+1.14%)`	⬆️

... and 2 files with indirect coverage changes

site/content/en/latest/design/eg-metrics.md

arkodg · 2023-10-17T16:13:58Z

api/v1alpha1/envoygateway_helpers.go

+// DefaultEnvoyGatewayPrometheus returns a new EnvoyGatewayMetrics with default configuration parameters.
+func DefaultEnvoyGatewayPrometheus() *EnvoyGatewayPrometheusProvider {
+	return &EnvoyGatewayPrometheusProvider{
+		// Enable prometheus pull by default.


we dont enable prom by default in Envoy Proxy, I think we should be consistent and do the same here, and not enable it by default

As we discussed in Slack, we should enable prometheus in default, let us Use disable for it.

arkodg · 2023-10-18T17:02:23Z

site/content/en/latest/install/install-helm.md

+| Xds EnvoyProxy Server  | 0.0.0.0   | 18000  |       Yes      |
+| Xds RateLimit Server   | 0.0.0.0   | 18001  |       Yes      |
+| Admin Server           | 127.0.0.1 | 19000  |       Yes      |
+| Metrics Server         |  0.0.0.0  | 19001  |       Yes      |


none of these are configurable atm, can we add No and later move it to Yes ?

Admin Server and Metrics Server can be configured by EnvoyGateway Config.

arkodg · 2023-10-18T17:03:05Z

hey @Xunzhuo can you rm the png from the diff, thanks

arkodg · 2023-10-18T17:37:04Z

@Xunzhuo this PR mostly LGTM from my end but it also has the design doc which includes the specific metric names as well as the design for the internal metrics lib which is being discussed in #1982. Suggest either only keeping the API in the design doc and adding the metrics lib design later or moving the design doc out of this PR

Xunzhuo · 2023-10-20T08:03:30Z

@Xunzhuo this PR mostly LGTM from my end but it also has the design doc which includes the specific metric names as well as the design for the internal metrics lib which is being discussed in #1982. Suggest either only keeping the API in the design doc and adding the metrics lib design later or moving the design doc out of this PR

Removed from the docs.

api/v1alpha1/envoygateway_metrics_types.go

arkodg

LGTM thanks !

zirain

Great work, leave some comments.

zirain · 2023-10-23T07:06:59Z

site/content/en/latest/design/eg-metrics.md

@@ -0,0 +1,238 @@
+---
+date: 2023-10-10


Remove, consistent with other documents

No, we should set date from now now, it sorted docs with date instead of naming.

The design doc needs to be kept updated, use alphabetical order is fine.

zirain · 2023-10-23T07:07:05Z

site/content/en/latest/design/eg-metrics.md

+author: Xunzhuo Liu
+linkTitle: "Control Plane Observability: Metrics"


Remove, consistent with other documents

zirain · 2023-10-23T07:07:12Z

site/content/en/latest/design/eg-metrics.md

+{{% alert title="State" color="warning" %}}
+
+ Author: [Xunzhuo Liu](https://github.com/Xunzhuo)
+ Affiliation: Tencent
+ Data: 2023-10-12
+ Status: Done
+{{% /alert %}}


Remove, consistent with other documents

zirain · 2023-10-23T07:08:20Z

site/content/en/latest/design/eg-metrics.md

+{{% alert title="Note" color="secondary" %}}
+**Data plane** observability (while important) is outside of scope for this document.
+{{% /alert %}}


replaced with right link, For dataplane, see xxxxxx

zirain · 2023-10-23T07:08:58Z

site/content/en/latest/design/eg-metrics.md

+## Current State
+
+At present, the Envoy Gateway control plane provides logs and controller-runtime metrics, without traces. Logs are managed through our proprietary library (`internal/logging`, a shim to `zap`) and are written to `/dev/stdout`.
+
+The absence of comprehensive and robust control plane metrics observability hinders the effective monitoring of Envoy Gateway in a production environment, a critical requirement before deploying Envoy Gateway into production.


Prefer to removed this paragraph.

zirain · 2023-10-23T07:17:12Z

site/content/en/latest/design/eg-metrics.md

+
+### Standards
+
+Our metrics, and traces in the future, will be built upon the [OpenTelemetry](https://opentelemetry.io/) standards. All metrics will be configured via the [OpenTelemetry SDK](https://opentelemetry.io/docs/specs/otel/metrics/sdk/), which offers neutral libraries that can be connected to various backends.


let's focus on metrics, remove traces.

zirain · 2023-10-23T07:20:03Z

site/content/en/latest/design/eg-metrics.md

+
+#### Example
+
+ The following is an example to enable prometheus metric.


prometheus is enable by default

zirain · 2023-10-23T07:20:52Z

site/content/en/latest/design/eg-metrics.md

+ ***Deprecated***: a metric that is intended to be phased out.
+ ***Experimental***: a metric that is off by default.
+ ***Alpha***: a metric that is on by default.


how can you categorize these?

zirain · 2023-10-23T07:23:51Z

site/content/en/latest/design/eg-metrics.md

+Our objectives include:
+
+ Supporting **PULL** mode for Prometheus metrics and exposing these metrics on the admin address.
+ Supporting **PUSH** mode for Prometheus metrics, thereby sending metrics to the Open Telemetry Stats sink.


You don't mention HTTP or gRPC here, it seems that you use HTTP by default, so what about gRPC?

zirain · 2023-10-23T07:33:23Z

site/content/en/latest/design/eg-metrics.md

+
+The OTEL libraries allow for the registration of Providers/Handlers. While we will offer the default ones (PULL via Prometheus, PUSH via OTEL HTTP metrics exporter) mentioned in Envoy Gateway's extensibility, we can easily allow custom builds of Envoy Gateway to plug in alternatives if the default options don't meet their needs.
+
+For instance, users may prefer to write metrics over the OTLP gRPC metrics exporter instead of the HTTP metrics exporter. This is perfectly acceptable -- and almost impossible to prevent. The OTEL has ways to register their providers/exporters, and Envoy Gateway can ensure its usage is such that it's not overly difficult to swap out a different provider/exporter.


does this doc spend too much paragraph on what's OTel, how OTel/OTel-collector work?

Signed-off-by: bitliu <bitliu@tencent.com>

zirain · 2023-10-24T11:23:00Z

please do not use force push, it's hard to track the changes for a large PR.

zirain · 2023-10-24T22:00:58Z

site/content/en/latest/design/eg-metrics.md

+	Host string `json:"host"`
+	// Protocol define the sink service protocol.
+	// +kubebuilder:validation:Enum=grpc;http
+	Protocol string `json:"protocol"`


from the default value of port, I suspect default value for this field is grpc.

imo its fine to start w/o a default

zirain

Let's merge this first

Xunzhuo requested a review from a team as a code owner October 17, 2023 08:31

Xunzhuo force-pushed the design-cp-obs-metrics-docs-with-api branch from d99b201 to 2fd0dd1 Compare October 17, 2023 08:32

Xunzhuo mentioned this pull request Oct 17, 2023

feat: add control plane metrics library #1982

Merged

Xunzhuo added the area/observability Observability related issues label Oct 17, 2023

Xunzhuo force-pushed the design-cp-obs-metrics-docs-with-api branch from 2fd0dd1 to 9cf7048 Compare October 17, 2023 08:46

zirain reviewed Oct 17, 2023

View reviewed changes

site/content/en/latest/design/eg-metrics.md Outdated Show resolved Hide resolved

arkodg reviewed Oct 17, 2023

View reviewed changes

Xunzhuo force-pushed the design-cp-obs-metrics-docs-with-api branch 2 times, most recently from a496868 to 58e6504 Compare October 18, 2023 08:57

arkodg reviewed Oct 18, 2023

View reviewed changes

Xunzhuo force-pushed the design-cp-obs-metrics-docs-with-api branch from 58e6504 to b066956 Compare October 20, 2023 08:01

Xunzhuo commented Oct 20, 2023

View reviewed changes

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

Xunzhuo force-pushed the design-cp-obs-metrics-docs-with-api branch 5 times, most recently from fdb9469 to c56f83a Compare October 20, 2023 23:57

arkodg previously approved these changes Oct 21, 2023

View reviewed changes

arkodg requested review from zirain, a team, kflynn, Alice-Lilith and chauhanshubham and removed request for a team October 21, 2023 00:28

Xunzhuo dismissed arkodg’s stale review via a6e3c99 October 23, 2023 06:58

Xunzhuo force-pushed the design-cp-obs-metrics-docs-with-api branch 2 times, most recently from a6e3c99 to 0e31051 Compare October 23, 2023 06:59

Xunzhuo requested a review from arkodg October 23, 2023 07:19

zirain reviewed Oct 23, 2023

View reviewed changes

Xunzhuo added 3 commits October 24, 2023 14:12

design(docs/api): control plane metrics monitoring

aa336ab

Signed-off-by: bitliu <bitliu@tencent.com>

remove prometheues address

d4c012f

Signed-off-by: bitliu <bitliu@tencent.com>

update

77ea45f

Signed-off-by: bitliu <bitliu@tencent.com>

Xunzhuo force-pushed the design-cp-obs-metrics-docs-with-api branch from 0e31051 to 77ea45f Compare October 24, 2023 07:10

update

57a2ffa

Signed-off-by: bitliu <bitliu@tencent.com>

Xunzhuo requested a review from zirain October 24, 2023 07:39

arkodg approved these changes Oct 24, 2023

View reviewed changes

zirain reviewed Oct 24, 2023

View reviewed changes

zirain approved these changes Oct 24, 2023

View reviewed changes

arkodg merged commit 78f2d4a into envoyproxy:main Oct 24, 2023
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

design(docs/api): control plane metrics monitoring #1981

design(docs/api): control plane metrics monitoring #1981

Xunzhuo commented Oct 17, 2023 •

edited

Loading

codecov bot commented Oct 17, 2023 •

edited

Loading

arkodg Oct 17, 2023

Xunzhuo Oct 18, 2023

arkodg Oct 18, 2023

Xunzhuo Oct 20, 2023

arkodg commented Oct 18, 2023

arkodg commented Oct 18, 2023

Xunzhuo commented Oct 20, 2023

arkodg left a comment

zirain left a comment

zirain Oct 23, 2023

Xunzhuo Oct 24, 2023

zirain Oct 24, 2023

zirain Oct 23, 2023

zirain Oct 23, 2023

zirain Oct 23, 2023

zirain Oct 23, 2023

zirain Oct 23, 2023

zirain Oct 23, 2023

zirain Oct 23, 2023

zirain Oct 23, 2023

zirain Oct 23, 2023

zirain commented Oct 24, 2023

zirain Oct 24, 2023

arkodg Oct 24, 2023

zirain left a comment

		author: Xunzhuo Liu
		linkTitle: "Control Plane Observability: Metrics"


		### Standards

		Our metrics, and traces in the future, will be built upon the [OpenTelemetry](https://opentelemetry.io/) standards. All metrics will be configured via the [OpenTelemetry SDK](https://opentelemetry.io/docs/specs/otel/metrics/sdk/), which offers neutral libraries that can be connected to various backends.


		#### Example

		+ The following is an example to enable prometheus metric.


		The OTEL libraries allow for the registration of Providers/Handlers. While we will offer the default ones (PULL via Prometheus, PUSH via OTEL HTTP metrics exporter) mentioned in Envoy Gateway's extensibility, we can easily allow custom builds of Envoy Gateway to plug in alternatives if the default options don't meet their needs.

		For instance, users may prefer to write metrics over the OTLP gRPC metrics exporter instead of the HTTP metrics exporter. This is perfectly acceptable -- and almost impossible to prevent. The OTEL has ways to register their providers/exporters, and Envoy Gateway can ensure its usage is such that it's not overly difficult to swap out a different provider/exporter.

design(docs/api): control plane metrics monitoring #1981

design(docs/api): control plane metrics monitoring #1981

Conversation

Xunzhuo commented Oct 17, 2023 • edited Loading

codecov bot commented Oct 17, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arkodg commented Oct 18, 2023

arkodg commented Oct 18, 2023

Xunzhuo commented Oct 20, 2023

arkodg left a comment

Choose a reason for hiding this comment

zirain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zirain commented Oct 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zirain left a comment

Choose a reason for hiding this comment

Xunzhuo commented Oct 17, 2023 •

edited

Loading

codecov bot commented Oct 17, 2023 •

edited

Loading