Repository: knative/pkg
Pull request #1963 (2020-12-15 14:35:25)

[0.19] Backport "Add host config option to the prometheus exporter (#1882)"
Backporting #1882, cherry-picked c73d344c89a7e80d9e416cea5b4a797e1195cf0.

Note: This should have been already in via https://github.com/knative/pkg/pull/1945 for some reason it ended up on master (I know it says master for the merge but then how is it possible anyway if this patch was already there). Master already has this patch via https://github.com/knative/pkg/pull/1882. :thinking: 

/cc @vagababov 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1963#" title="Author self-approved">skonto</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **yanweiguo** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @yanweiguo` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[metrics/OWNERS](https://github.com/knative/pkg/blob/release-0.19/metrics/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["yanweiguo"]} -->
knative-prow-robot: Hi @skonto. Thanks for your PR.

I'm waiting for a [knative](https://github.com/orgs/knative/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://github.com/orgs/knative/people) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1963?src=pr&el=h1) Report
> Merging [#1963](https://codecov.io/gh/knative/pkg/pull/1963?src=pr&el=desc) (73dd5a6) into [release-0.19](https://codecov.io/gh/knative/pkg/commit/5514ab0c1fdffe6ad771ab2ff84daf694d1f4ca1?el=desc) (5514ab0) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/1963/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO)](https://codecov.io/gh/knative/pkg/pull/1963?src=pr&el=tree)

```diff
@@               Coverage Diff                @@
##           release-0.19    #1963      +/-   ##
================================================
+ Coverage         69.13%   69.15%   +0.02%     
================================================
  Files               215      215              
  Lines              9088     9095       +7     
================================================
+ Hits               6283     6290       +7     
  Misses             2511     2511              
  Partials            294      294              
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1963?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [metrics/exporter.go](https://codecov.io/gh/knative/pkg/pull/1963/diff?src=pr&el=tree#diff-bWV0cmljcy9leHBvcnRlci5nbw==) | `78.26% <ø> (ø)` | |
| [metrics/config.go](https://codecov.io/gh/knative/pkg/pull/1963/diff?src=pr&el=tree#diff-bWV0cmljcy9jb25maWcuZ28=) | `83.20% <100.00%> (+0.70%)` | :arrow_up: |
| [metrics/prometheus\_exporter.go](https://codecov.io/gh/knative/pkg/pull/1963/diff?src=pr&el=tree#diff-bWV0cmljcy9wcm9tZXRoZXVzX2V4cG9ydGVyLmdv) | `89.28% <100.00%> (ø)` | |
| [reconciler/events.go](https://codecov.io/gh/knative/pkg/pull/1963/diff?src=pr&el=tree#diff-cmVjb25jaWxlci9ldmVudHMuZ28=) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1963?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/1963?src=pr&el=footer). Last update [5514ab0...818f742](https://codecov.io/gh/knative/pkg/pull/1963?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1963#pullrequestreview-552537397" title="Approved">markusthoemmes</a>*, *<a href="https://github.com/knative/pkg/pull/1963#" title="Author self-approved">skonto</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[metrics/OWNERS](https://github.com/knative/pkg/blob/release-0.19/metrics/OWNERS)~~ [markusthoemmes]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/metrics/config.go b/metrics/config.go
index 38ac02e65..d4ae24ad5 100644
--- a/metrics/config.go
+++ b/metrics/config.go
@@ -60,7 +60,9 @@ const (
 	defaultPrometheusPort = 9090
 	maxPrometheusPort     = 65535
 	minPrometheusPort     = 1024
+	defaultPrometheusHost = "0.0.0.0"
 	prometheusPortEnvName = "METRICS_PROMETHEUS_PORT"
+	prometheusHostEnvName = "METRICS_PROMETHEUS_HOST"
 )
 
 // Metrics backend "enum".
@@ -105,6 +107,10 @@ type metricsConfig struct {
 	// format. It defaults to 9090.
 	prometheusPort int
 
+	// prometheusHost is the host where the metrics are exposed in Prometheus
+	// format. It defaults to "0.0.0.0"
+	prometheusHost string
+
 	// ---- Stackdriver specific below ----
 	// True if backendDestination equals to "stackdriver". Store this in a variable
 	// to reduce string comparison operations.
@@ -240,6 +246,7 @@ func createMetricsConfig(ctx context.Context, ops ExporterOptions) (*metricsConf
 		}
 
 		mc.prometheusPort = pp
+		mc.prometheusHost = prometheusHost()
 	case stackdriver:
 		// If stackdriverClientConfig is not provided for stackdriver backend destination, OpenCensus will try to
 		// use the application default credentials. If that is not available, Opencensus would fail to create the
@@ -341,6 +348,17 @@ func prometheusPort() (int, error) {
 	return int(pp), nil
 }
 
+// prometheusHost returns the host configured via the environment
+// for the Prometheus metrics exporter if it's set, a default value otherwise.
+// No validation is done here.
+func prometheusHost() string {
+	phStr := os.Getenv(prometheusHostEnvName)
+	if phStr == "" {
+		return defaultPrometheusHost
+	}
+	return phStr
+}
+
 // JSONToOptions converts a json string to ExporterOptions.
 func JSONToOptions(jsonOpts string) (*ExporterOptions, error) {
 	var opts ExporterOptions
diff --git a/metrics/exporter.go b/metrics/exporter.go
index a7411fccb..0c680106e 100644
--- a/metrics/exporter.go
+++ b/metrics/exporter.go
@@ -66,10 +66,15 @@ type ExporterOptions struct {
 
 	// PrometheusPort is the port to expose metrics if metrics backend is Prometheus.
 	// It should be between maxPrometheusPort and maxPrometheusPort. 0 value means
-	// using the default 9090 value. If is ignored if metrics backend is not
+	// using the default 9090 value. It is ignored if metrics backend is not
 	// Prometheus.
 	PrometheusPort int
 
+	// PrometheusHost is the host to expose metrics on if metrics backend is Prometheus.
+	// The default value is "0.0.0.0". It is ignored if metrics backend is not
+	// Prometheus.
+	PrometheusHost string
+
 	// ConfigMap is the data from config map config-observability. Must be present.
 	// See https://github.com/knative/serving/blob/master/config/config-observability.yaml
 	// for details.
diff --git a/metrics/prometheus_exporter.go b/metrics/prometheus_exporter.go
index 7f49302cc..29b920633 100644
--- a/metrics/prometheus_exporter.go
+++ b/metrics/prometheus_exporter.go
@@ -17,8 +17,8 @@ limitations under the License.
 package metrics
 
 import (
-	"fmt"
 	"net/http"
+	"strconv"
 	"sync"
 
 	prom "contrib.go.opencensus.io/exporter/prometheus"
@@ -50,7 +50,7 @@ func newPrometheusExporter(config *metricsConfig, logger *zap.SugaredLogger) (vi
 	logger.Infof("Created Opencensus Prometheus exporter with config: %v. Start the server for Prometheus exporter.", config)
 	// Start the server for Prometheus scraping
 	go func() {
-		srv := startNewPromSrv(e, config.prometheusPort)
+		srv := startNewPromSrv(e, config.prometheusHost, config.prometheusPort)
 		srv.ListenAndServe()
 	}()
 	return e,
@@ -73,7 +73,7 @@ func resetCurPromSrv() {
 	}
 }
 
-func startNewPromSrv(e *prom.Exporter, port int) *http.Server {
+func startNewPromSrv(e *prom.Exporter, host string, port int) *http.Server {
 	sm := http.NewServeMux()
 	sm.Handle("/metrics", e)
 	curPromSrvMux.Lock()
@@ -82,7 +82,7 @@ func startNewPromSrv(e *prom.Exporter, port int) *http.Server {
 		curPromSrv.Close()
 	}
 	curPromSrv = &http.Server{
-		Addr:    fmt.Sprint(":", port),
+		Addr:    host + ":" + strconv.Itoa(port),
 		Handler: sm,
 	}
 	return curPromSrv


Test patch:
diff --git a/metrics/config_test.go b/metrics/config_test.go
index 8d47a5716..53fa89639 100644
--- a/metrics/config_test.go
+++ b/metrics/config_test.go
@@ -187,6 +187,7 @@ var (
 			backendDestination: prometheus,
 			reportingPeriod:    5 * time.Second,
 			prometheusPort:     defaultPrometheusPort,
+			prometheusHost:     defaultPrometheusHost,
 		},
 		expectedNewExporter: true,
 	}, {
@@ -308,6 +309,7 @@ var (
 			backendDestination: prometheus,
 			reportingPeriod:    5 * time.Second,
 			prometheusPort:     defaultPrometheusPort,
+			prometheusHost:     defaultPrometheusHost,
 		},
 		expectedNewExporter: true,
 	}, {
@@ -349,6 +351,7 @@ var (
 			backendDestination: prometheus,
 			reportingPeriod:    12 * time.Second,
 			prometheusPort:     defaultPrometheusPort,
+			prometheusHost:     defaultPrometheusHost,
 		},
 		expectedNewExporter: true,
 	}, {
@@ -431,6 +434,7 @@ var (
 			backendDestination: prometheus,
 			reportingPeriod:    5 * time.Second,
 			prometheusPort:     defaultPrometheusPort,
+			prometheusHost:     defaultPrometheusHost,
 		},
 		expectedNewExporter: true,
 	}, {
@@ -521,6 +525,7 @@ var (
 			backendDestination: prometheus,
 			reportingPeriod:    5 * time.Second,
 			prometheusPort:     9091,
+			prometheusHost:     defaultPrometheusHost,
 		},
 		expectedNewExporter: true,
 	}}
@@ -595,6 +600,7 @@ func TestGetMetricsConfig_fromEnv(t *testing.T) {
 			backendDestination: prometheus,
 			reportingPeriod:    5 * time.Second,
 			prometheusPort:     defaultPrometheusPort,
+			prometheusHost:     defaultPrometheusHost,
 		},
 	}, {
 		name:     "PrometheusPort from env",
@@ -611,6 +617,7 @@ func TestGetMetricsConfig_fromEnv(t *testing.T) {
 			backendDestination: prometheus,
 			reportingPeriod:    5 * time.Second,
 			prometheusPort:     9999,
+			prometheusHost:     defaultPrometheusHost,
 		},
 	}}
 
@@ -911,12 +918,13 @@ func TestMetricsOptions(t *testing.T) {
 				Domain:         "domain",
 				Component:      "component",
 				PrometheusPort: 9090,
+				PrometheusHost: "0.0.0.0",
 				ConfigMap: map[string]string{
 					"foo":   "bar",
 					"boosh": "kakow",
 				},
 			},
-			want: `{"Domain":"domain","Component":"component","PrometheusPort":9090,"ConfigMap":{"boosh":"kakow","foo":"bar"}}`,
+			want: `{"Domain":"domain","Component":"component","PrometheusPort":9090,"PrometheusHost":"0.0.0.0","ConfigMap":{"boosh":"kakow","foo":"bar"}}`,
 		},
 	}
 	for n, tc := range testCases {
diff --git a/metrics/prometheus_exporter_test.go b/metrics/prometheus_exporter_test.go
index 6a416fb3a..447458e4a 100644
--- a/metrics/prometheus_exporter_test.go
+++ b/metrics/prometheus_exporter_test.go
@@ -13,6 +13,8 @@ limitations under the License.
 package metrics
 
 import (
+	"context"
+	"os"
 	"testing"
 	"time"
 
@@ -31,8 +33,9 @@ func TestNewPrometheusExporter(t *testing.T) {
 			component:          testComponent,
 			backendDestination: prometheus,
 			prometheusPort:     9090,
+			prometheusHost:     "0.0.0.0",
 		},
-		expectedAddr: ":9090",
+		expectedAddr: "0.0.0.0:9090",
 	}, {
 		name: "port 9091",
 		config: metricsConfig{
@@ -40,8 +43,9 @@ func TestNewPrometheusExporter(t *testing.T) {
 			component:          testComponent,
 			backendDestination: prometheus,
 			prometheusPort:     9091,
+			prometheusHost:     "127.0.0.1",
 		},
-		expectedAddr: ":9091",
+		expectedAddr: "127.0.0.1:9091",
 	}}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -57,6 +61,71 @@ func TestNewPrometheusExporter(t *testing.T) {
 	}
 }
 
+func TestNewPrometheusExporter_fromEnv(t *testing.T) {
+	exporterOptions := ExporterOptions{
+		ConfigMap: map[string]string{},
+		Domain:    servingDomain,
+		Component: testComponent,
+	}
+	testCases := []struct {
+		name                   string
+		prometheusPortVarName  string
+		prometheusPortVarValue string
+		prometheusHostVarName  string
+		prometheusHostVarValue string
+		ops                    ExporterOptions
+		expectedAddr           string
+	}{{
+		name:                   "port from env var with no host set",
+		prometheusPortVarName:  prometheusPortEnvName,
+		prometheusPortVarValue: "9092",
+		ops:                    exporterOptions,
+		expectedAddr:           "0.0.0.0:9092",
+	}, {
+		name:                   "no port set with host from env var",
+		prometheusHostVarName:  prometheusHostEnvName,
+		prometheusHostVarValue: "127.0.0.1",
+		ops:                    exporterOptions,
+		expectedAddr:           "127.0.0.1:9090",
+	}, {
+		name:                   "port set and host set to empty string",
+		prometheusPortVarName:  prometheusPortEnvName,
+		prometheusPortVarValue: "",
+		prometheusHostVarName:  prometheusHostEnvName,
+		prometheusHostVarValue: "",
+		ops:                    exporterOptions,
+		expectedAddr:           "0.0.0.0:9090",
+	}, {
+		name:         "no port or host from the env",
+		ops:          exporterOptions,
+		expectedAddr: "0.0.0.0:9090",
+	}}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.prometheusPortVarName != "" {
+				os.Setenv(tc.prometheusPortVarName, tc.prometheusPortVarValue)
+				defer os.Unsetenv(tc.prometheusPortVarName)
+			}
+			if tc.prometheusHostVarName != "" {
+				os.Setenv(tc.prometheusHostVarName, tc.prometheusHostVarValue)
+				defer os.Unsetenv(tc.prometheusHostVarName)
+			}
+			mc, err := createMetricsConfig(context.Background(), tc.ops)
+			if err != nil {
+				t.Fatal("Failed to create the metrics config:", err)
+			}
+			e, _, err := newPrometheusExporter(mc, TestLogger(t))
+			if err != nil {
+				t.Fatal("Failed to create a new Prometheus exporter:", err)
+			}
+			if e == nil {
+				t.Fatal("expected a non-nil metrics exporter")
+			}
+			expectPromSrv(t, tc.expectedAddr)
+		})
+	}
+}
+
 func expectPromSrv(t *testing.T, expectedAddr string) {
 	time.Sleep(200 * time.Millisecond)
 	srv := getCurPromSrv()