From 61757f5e65bc380b67b8accc1b13b701d8e273bc Mon Sep 17 00:00:00 2001
From: Kenneth Jenkins <51246568+kenjenkins@users.noreply.github.com>
Date: Thu, 30 Nov 2023 16:14:24 -0800
Subject: [PATCH] metrics: explicitly set Accept header (#4774)

If a request is made to the Pomerium metrics endpoint with an Accept
header requesting the Prometheus protobuf exposition format, some
metrics will be missing from the response.

These missing metrics are obtained by replaying the incoming request to
an OpenCensus metrics exporter. This exporter honors the request for the
protobuf format, however Pomerium expects this response to be in the
text format.

We can avoid this mismatch by explicitly requesting the text format from
the OpenCensus exporter, regardless of the incoming request's Accept
header.

(Note: the Pomerium metrics endpoint always responds with text format
metrics, even if the protobuf format is requested.)
---
 internal/telemetry/metrics/providers.go      |  2 ++
 internal/telemetry/metrics/providers_test.go | 24 +++++++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/internal/telemetry/metrics/providers.go b/internal/telemetry/metrics/providers.go
index 5c1baf315..8319a4545 100644
--- a/internal/telemetry/metrics/providers.go
+++ b/internal/telemetry/metrics/providers.go
@@ -196,6 +196,8 @@ func ocExport(name string, exporter *ocprom.Exporter, r *http.Request, labels []
 	return func(context.Context) promProducerResult {
 		// Ensure we don't get entangled with compression from ocprom
 		r.Header.Del("Accept-Encoding")
+		// Request metrics in text format.
+		r.Header.Set("Accept", "text/plain")
 
 		rec := httptest.NewRecorder()
 		exporter.ServeHTTP(rec, r)
diff --git a/internal/telemetry/metrics/providers_test.go b/internal/telemetry/metrics/providers_test.go
index aa795401c..9cc2e8afc 100644
--- a/internal/telemetry/metrics/providers_test.go
+++ b/internal/telemetry/metrics/providers_test.go
@@ -28,12 +28,15 @@ envoy_server_initialization_time_ms_bucket{le="1000"} 1
 	}
 }
 
-func getMetrics(t *testing.T, envoyURL *url.URL) []byte {
+func getMetrics(t *testing.T, envoyURL *url.URL, header http.Header) []byte {
 	h, err := PrometheusHandler([]ScrapeEndpoint{{Name: "envoy", URL: *envoyURL}}, "test_installation_id", time.Second*20)
 	if err != nil {
 		t.Fatal(err)
 	}
 	req := httptest.NewRequest(http.MethodGet, "http://test.local/metrics", nil)
+	if header != nil {
+		req.Header = header
+	}
 	rec := httptest.NewRecorder()
 	h.ServeHTTP(rec, req)
 
@@ -48,7 +51,7 @@ func getMetrics(t *testing.T, envoyURL *url.URL) []byte {
 
 func Test_PrometheusHandler(t *testing.T) {
 	t.Run("no envoy", func(t *testing.T) {
-		b := getMetrics(t, &url.URL{})
+		b := getMetrics(t, &url.URL{}, nil)
 
 		if m, _ := regexp.Match(`(?m)^# HELP .*`, b); !m {
 			t.Errorf("Metrics endpoint did not contain any help messages: %s", b)
@@ -58,7 +61,22 @@ func Test_PrometheusHandler(t *testing.T) {
 	t.Run("with envoy", func(t *testing.T) {
 		fakeEnvoyMetricsServer := httptest.NewServer(newEnvoyMetricsHandler())
 		envoyURL, _ := url.Parse(fakeEnvoyMetricsServer.URL)
-		b := getMetrics(t, envoyURL)
+		b := getMetrics(t, envoyURL, nil)
+
+		if m, _ := regexp.Match(`(?m)^go_.*`, b); !m {
+			t.Errorf("Metrics endpoint did not contain internal metrics: %s", b)
+		}
+		if m, _ := regexp.Match(`(?m)^# TYPE envoy_.*`, b); !m {
+			t.Errorf("Metrics endpoint did not contain envoy metrics: %s", b)
+		}
+	})
+
+	t.Run("with envoy, request protobuf format", func(t *testing.T) {
+		fakeEnvoyMetricsServer := httptest.NewServer(newEnvoyMetricsHandler())
+		envoyURL, _ := url.Parse(fakeEnvoyMetricsServer.URL)
+		header := http.Header{}
+		header.Set("Accept", "application/vnd.google.protobuf;proto=io.prometheus.client.MetricFamily;encoding=delimited")
+		b := getMetrics(t, envoyURL, header)
 
 		if m, _ := regexp.Match(`(?m)^go_.*`, b); !m {
 			t.Errorf("Metrics endpoint did not contain internal metrics: %s", b)