Repository: knative/pkg
Pull request #2610 (2022-10-12 18:54:15)

Support fetching observability from ctx.
Signed-off-by: Kenny Leung <kleung@chainguard.dev>

<!-- Thanks for sending a pull request! -->

# Changes

<!-- 
Describe your changes here- ideally you can get that description straight from
your descriptive commit message(s)! 

- :gift: Add new feature
- :bug: Fix bug
- :broom: Update or clean up current behavior
- :wastebasket: Remove feature or internal logic
-->

This enables callers of sharedmain to infuse ctx with a metrics.ObservabilityConfig instead of relying on the API server for it.

- 🎁  Add new feature

<!--
In addition, categorize the changes you're making using the "/kind" Prow command, example:

/kind <kind>

Supported kinds are: api-change, bug, cleanup, deprecation, removal, documentation, enhancement, performance

-->
/kind enhancement

<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

<!-- Please include the 'why' behind your changes if no issue exists -->

**Release Note**

<!--
:page_facing_up: If this change has user-visible impact, write a release note in the block
below. Include the string "action required" if additional action is required of
users switching to the new release, for example in case of a breaking change.

Write as if you are speaking to users, not other Knative contributors. If this
change has no user-visible impact, no release-note is needed.
-->
```release-note
The observability configurations can now be configured with sharedmain.WithObservabilityConfig.

```

**Docs**

<!--
:book: If this change has user-visible impact, link to an issue or PR in
https://github.com/knative/docs.

See here for guidance on getting permanent links to files: https://help.github.com/en/articles/getting-permanent-links-to-files

Please use the following format for linking documentation:
- [knative/docs]: <issue or pr link>
- [Feature Track]: <link>
- [Usage]: <link>
- [Other doc]: <link>
-->
```docs

```


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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2610#" title="Author self-approved">k4leung4</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign mattmoor, yanweiguo for approval by writing `/assign @mattmoor @yanweiguo` in a comment. For more information see:[The Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

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:

- **[injection/OWNERS](https://github.com/knative/pkg/blob/main/injection/OWNERS)**
- **[metrics/OWNERS](https://github.com/knative/pkg/blob/main/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":["mattmoor","yanweiguo"]} -->
knative-prow[bot]: @k4leung4: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit-tests_pkg_main | 68e8eb607f4d0d3e2cfb296777cd6d48d9e57555 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2610/unit-tests_pkg_main/1580270645361512448) | true | `/test unit-tests`

[Your PR dashboard](https://prow.knative.dev/pr).

<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. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/2610?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) Report
Base: **81.33**% // Head: **81.28**% // Decreases project coverage by **`-0.04%`** :warning:
> Coverage data is based on head [(`73328ad`)](https://codecov.io/gh/knative/pkg/pull/2610?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) compared to base [(`714b763`)](https://codecov.io/gh/knative/pkg/commit/714b7630a836ef3952826849f31fe9bc16493218?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).
> Patch coverage: 61.90% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #2610      +/-   ##
==========================================
- Coverage   81.33%   81.28%   -0.05%     
==========================================
  Files         162      162              
  Lines        9824     9845      +21     
==========================================
+ Hits         7990     8003      +13     
- Misses       1595     1603       +8     
  Partials      239      239              
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/2610?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) | Coverage Δ | |
|---|---|---|
| [metrics/prometheus\_exporter.go](https://codecov.io/gh/knative/pkg/pull/2610/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-bWV0cmljcy9wcm9tZXRoZXVzX2V4cG9ydGVyLmdv) | `90.24% <ø> (ø)` | |
| [metrics/config\_observability.go](https://codecov.io/gh/knative/pkg/pull/2610/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-bWV0cmljcy9jb25maWdfb2JzZXJ2YWJpbGl0eS5nbw==) | `80.35% <61.90%> (-11.08%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/knative/pkg/pull/2610?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).

vagababov: /lgtm
k4leung4: /assign @mattmoor @yanweiguo
k4leung4: /retest
knative-prow[bot]: New changes are detected. LGTM label has been removed.
knative-prow[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2610#" title="Author self-approved">k4leung4</a>*
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from mattmoor by writing `/assign @mattmoor` in a comment. For more information see:[The Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

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:

- **[injection/OWNERS](https://github.com/knative/pkg/blob/main/injection/OWNERS)**
- **[metrics/OWNERS](https://github.com/knative/pkg/blob/main/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":["mattmoor"]} -->
knative-prow[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2610#" title="Author self-approved">k4leung4</a>*, *<a href="https://github.com/knative/pkg/pull/2610#pullrequestreview-1142714554" title="Approved">mattmoor</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:

- ~~[injection/OWNERS](https://github.com/knative/pkg/blob/main/injection/OWNERS)~~ [mattmoor]
- ~~[metrics/OWNERS](https://github.com/knative/pkg/blob/main/metrics/OWNERS)~~ [mattmoor]

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/injection/sharedmain/main.go b/injection/sharedmain/main.go
index 3b8ab3f40..8af9eb5d1 100644
--- a/injection/sharedmain/main.go
+++ b/injection/sharedmain/main.go
@@ -104,6 +104,25 @@ func GetLeaderElectionConfig(ctx context.Context) (*leaderelection.Config, error
 	return leaderelection.NewConfigFromConfigMap(leaderElectionConfigMap)
 }
 
+// GetObservabilityConfig gets the observability config from the (in order):
+// 1. provided context,
+// 2. reading from the API server,
+// 3. defaults (if not found).
+func GetObservabilityConfig(ctx context.Context) (*metrics.ObservabilityConfig, error) {
+	if cfg := metrics.GetObservabilityConfig(ctx); cfg != nil {
+		return cfg, nil
+	}
+
+	observabilityConfigMap, err := kubeclient.Get(ctx).CoreV1().ConfigMaps(system.Namespace()).Get(ctx, metrics.ConfigMapName(), metav1.GetOptions{})
+	if apierrors.IsNotFound(err) {
+		return metrics.NewObservabilityConfigFromConfigMap(nil)
+	}
+	if err != nil {
+		return nil, err
+	}
+	return metrics.NewObservabilityConfigFromConfigMap(observabilityConfigMap)
+}
+
 // EnableInjectionOrDie enables Knative Injection and starts the informers.
 // Both Context and Config are optional.
 // Deprecated: use injection.EnableInjectionOrDie
@@ -249,6 +268,8 @@ func MainWithConfig(ctx context.Context, component string, cfg *rest.Config, cto
 			leaderElectionConfig.GetComponentConfig(component))
 	}
 
+	SetupObservabilityOrDie(ctx, component, logger, profilingHandler)
+
 	controllers, webhooks := ControllersAndWebhooksFromCtors(ctx, cmw, ctors...)
 	WatchLoggingConfigOrDie(ctx, cmw, logger, atomicLevel, component)
 	WatchObservabilityConfigOrDie(ctx, cmw, profilingHandler, logger, component)
@@ -327,6 +348,18 @@ func SetupLoggerOrDie(ctx context.Context, component string) (*zap.SugaredLogger
 	return l, level
 }
 
+// SetupObservabilityOrDie sets up the observability using the config from the given context
+// or dies by calling log.Fatalf.
+func SetupObservabilityOrDie(ctx context.Context, component string, logger *zap.SugaredLogger, profilingHandler *profiling.Handler) {
+	observabilityConfig, err := GetObservabilityConfig(ctx)
+	if err != nil {
+		logger.Fatal("Error loading observability configuration: ", err)
+	}
+	observabilityConfigMap := observabilityConfig.GetConfigMap()
+	metrics.ConfigMapWatcher(ctx, component, SecretFetcher(ctx), logger)(&observabilityConfigMap)
+	profilingHandler.UpdateFromConfigMap(&observabilityConfigMap)
+}
+
 // CheckK8sClientMinimumVersionOrDie checks that the hosting Kubernetes cluster
 // is at least the minimum allowable version or dies by calling log.Fatalw.
 func CheckK8sClientMinimumVersionOrDie(ctx context.Context, logger *zap.SugaredLogger) {
diff --git a/metrics/config_observability.go b/metrics/config_observability.go
index 0a17e0f03..f95d6bc8c 100644
--- a/metrics/config_observability.go
+++ b/metrics/config_observability.go
@@ -17,8 +17,10 @@ limitations under the License.
 package metrics
 
 import (
+	"context"
 	"fmt"
 	"os"
+	"strconv"
 	texttemplate "text/template"
 
 	corev1 "k8s.io/api/core/v1"
@@ -81,6 +83,22 @@ type ObservabilityConfig struct {
 	MetricsCollectorAddress string
 }
 
+type ocfg struct{}
+
+// WithConfig associates a observability configuration with the context.
+func WithConfig(ctx context.Context, cfg *ObservabilityConfig) context.Context {
+	return context.WithValue(ctx, ocfg{}, cfg)
+}
+
+// GetObservability gets the observability config from the provided context.
+func GetObservabilityConfig(ctx context.Context) *ObservabilityConfig {
+	untyped := ctx.Value(ocfg{})
+	if untyped == nil {
+		return nil
+	}
+	return untyped.(*ObservabilityConfig)
+}
+
 func defaultConfig() *ObservabilityConfig {
 	return &ObservabilityConfig{
 		LoggingURLTemplate:    DefaultLogURLTemplate,
@@ -120,6 +138,21 @@ func NewObservabilityConfigFromConfigMap(configMap *corev1.ConfigMap) (*Observab
 	return oc, nil
 }
 
+func (oc *ObservabilityConfig) GetConfigMap() corev1.ConfigMap {
+	return corev1.ConfigMap{
+		Data: map[string]string{
+			"logging.enable-var-log-collection":           strconv.FormatBool(oc.EnableVarLogCollection),
+			"logging.revision-url-template":               oc.LoggingURLTemplate,
+			ReqLogTemplateKey:                             oc.RequestLogTemplate,
+			EnableReqLogKey:                               strconv.FormatBool(oc.EnableRequestLog),
+			EnableProbeReqLogKey:                          strconv.FormatBool(oc.EnableProbeRequestLog),
+			"metrics.request-metrics-backend-destination": oc.RequestMetricsBackend,
+			"profiling.enable":                            strconv.FormatBool(oc.EnableProfiling),
+			"metrics.opencensus-address":                  oc.MetricsCollectorAddress,
+		},
+	}
+}
+
 // ConfigMapName gets the name of the metrics ConfigMap
 func ConfigMapName() string {
 	if cm := os.Getenv(configMapNameEnv); cm != "" {


Test patch:
diff --git a/injection/sharedmain/main_test.go b/injection/sharedmain/main_test.go
index d739fe2cf..e32a53f15 100644
--- a/injection/sharedmain/main_test.go
+++ b/injection/sharedmain/main_test.go
@@ -26,6 +26,7 @@ import (
 	"knative.dev/pkg/injection"
 	"knative.dev/pkg/leaderelection"
 	"knative.dev/pkg/logging"
+	"knative.dev/pkg/metrics"
 )
 
 func TestEnabledControllers(t *testing.T) {
@@ -103,3 +104,25 @@ func TestWithLeaderElectionConfig(t *testing.T) {
 		t.Errorf("(-got, +want) = %s", diff)
 	}
 }
+
+func TextWithObservabilityConfig(t *testing.T) {
+	want := &metrics.ObservabilityConfig{
+		EnableVarLogCollection:  false,
+		LoggingURLTemplate:      "url-template",
+		RequestLogTemplate:      "log-template",
+		EnableProbeRequestLog:   true,
+		RequestMetricsBackend:   "prometheus",
+		EnableProfiling:         true,
+		EnableRequestLog:        false,
+		MetricsCollectorAddress: "localhost:9090",
+	}
+	ctx := metrics.WithConfig(context.Background(), want)
+
+	got, err := GetObservabilityConfig(ctx)
+	if err != nil {
+		t.Fatalf("GetObservabilityConfig() = %v", err)
+	}
+	if diff := cmp.Diff(got, want); diff != "" {
+		t.Errorf("(-got, +want) = %s", diff)
+	}
+}
diff --git a/metrics/config_observability_test.go b/metrics/config_observability_test.go
index 5338f510b..1388ffe94 100644
--- a/metrics/config_observability_test.go
+++ b/metrics/config_observability_test.go
@@ -154,3 +154,55 @@ func TestConfigMapName(t *testing.T) {
 		t.Errorf("ConfigMapName = %q, want: %q", got, want)
 	}
 }
+
+func TestObservabilityConfigMap(t *testing.T) {
+	observabilityConfigMapTests := []struct {
+		name              string
+		config            *ObservabilityConfig
+		wantConfigMapData map[string]string
+	}{{
+		name: "observability configuration with all inputs",
+		config: &ObservabilityConfig{
+			EnableProbeRequestLog:  true,
+			EnableProfiling:        true,
+			EnableVarLogCollection: true,
+			EnableRequestLog:       true,
+			LoggingURLTemplate:     "https://logging.io",
+			RequestLogTemplate:     `{"requestMethod": "{{.Request.Method}}"}`,
+			RequestMetricsBackend:  "opencensus",
+		},
+		wantConfigMapData: map[string]string{
+			EnableProbeReqLogKey:                          "true",
+			"logging.enable-var-log-collection":           "true",
+			ReqLogTemplateKey:                             `{"requestMethod": "{{.Request.Method}}"}`,
+			"logging.revision-url-template":               "https://logging.io",
+			EnableReqLogKey:                               "true",
+			"metrics.request-metrics-backend-destination": "opencensus",
+			"profiling.enable":                            "true",
+			"metrics.opencensus-address":                  "",
+		},
+	}, {
+		name:   "observability configuration default config",
+		config: defaultConfig(),
+		wantConfigMapData: map[string]string{
+			EnableProbeReqLogKey:                          "false",
+			"logging.enable-var-log-collection":           "false",
+			ReqLogTemplateKey:                             `{"httpRequest": {"requestMethod": "{{.Request.Method}}", "requestUrl": "{{js .Request.RequestURI}}", "requestSize": "{{.Request.ContentLength}}", "status": {{.Response.Code}}, "responseSize": "{{.Response.Size}}", "userAgent": "{{js .Request.UserAgent}}", "remoteIp": "{{js .Request.RemoteAddr}}", "serverIp": "{{.Revision.PodIP}}", "referer": "{{js .Request.Referer}}", "latency": "{{.Response.Latency}}s", "protocol": "{{.Request.Proto}}"}, "traceId": "{{index .Request.Header "X-B3-Traceid"}}"}`,
+			"logging.revision-url-template":               "",
+			EnableReqLogKey:                               "false",
+			"metrics.request-metrics-backend-destination": "prometheus",
+			"profiling.enable":                            "false",
+			"metrics.opencensus-address":                  "",
+		},
+	}}
+
+	for _, tt := range observabilityConfigMapTests {
+		t.Run(tt.name, func(t *testing.T) {
+			cm := tt.config.GetConfigMap()
+
+			if got, want := cm.Data, tt.wantConfigMapData; !cmp.Equal(got, want) {
+				t.Errorf("Got = %v, want: %v, diff(-want,+got)\n%s", got, want, cmp.Diff(want, got))
+			}
+		})
+	}
+}