Repository: knative/pkg
Pull request #2584 (2022-08-25 20:22:22)

Update: report stats for request
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
-->

- 🧹  uncomment reporting of stats for conversion requests

<!--
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 cleanup

<!--
*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
NONE
```

**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/2584#" title="Author self-approved">k4leung4</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign mattmoor for approval 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:

- **[webhook/OWNERS](https://github.com/knative/pkg/blob/main/webhook/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"]} -->
k4leung4: /assign @mattmoor 
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 | fe614e18bb864a69e5bcd8e1fafd53a13286c929 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562898206486958080) | true | `/test unit-tests_pkg_main`

[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 -->
k4leung4: /test unit-tests_pkg_main
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 | fe614e18bb864a69e5bcd8e1fafd53a13286c929 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562899500312301568) | true | `/test unit-tests_pkg_main`

[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 -->
knative-prow[bot]: @k4leung4: The following tests **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 | fe614e18bb864a69e5bcd8e1fafd53a13286c929 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562899500312301568) | true | `/test unit-tests_pkg_main`
build-tests_pkg_main | fe614e18bb864a69e5bcd8e1fafd53a13286c929 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/build-tests_pkg_main/1562898206352740352) | true | `/test build-tests_pkg_main`

[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 -->
knative-prow[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2584#" 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:

- **[webhook/OWNERS](https://github.com/knative/pkg/blob/main/webhook/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]: @k4leung4: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
build-tests_pkg_main | fe614e18bb864a69e5bcd8e1fafd53a13286c929 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/build-tests_pkg_main/1562898206352740352) | true | `/test build-tests_pkg_main`
unit-tests_pkg_main | 290065e3ef4f6c2e099277762e9c3b0287ddb462 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562904492330979328) | true | `/test unit-tests_pkg_main`

[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 -->
knative-prow[bot]: @k4leung4: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
build-tests_pkg_main | 290065e3ef4f6c2e099277762e9c3b0287ddb462 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/build-tests_pkg_main/1562904492268064768) | true | `/test build-tests_pkg_main`
unit-tests_pkg_main | 4ec04489fc1dcef8c9c8c7f189f5c9cbd25b1a24 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562937912352313344) | true | `/test unit-tests_pkg_main`

[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 -->
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 | 28bb58ec678107d3d1f5b8e28802c65b97349ead | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562940386651934720) | true | `/test unit-tests_pkg_main`

[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 -->
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 | 89331697eb51f1adb281dae8baaad15829d61cc0 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562953076334137344) | true | `/test unit-tests_pkg_main`

[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/2584?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) Report
> Merging [#2584](https://codecov.io/gh/knative/pkg/pull/2584?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (8933169) into [main](https://codecov.io/gh/knative/pkg/commit/a6afcab5bd78047384d858c913826af80dd7dcd8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (a6afcab) will **increase** coverage by `0.04%`.
> The diff coverage is `89.28%`.

> :exclamation: Current head 8933169 differs from pull request most recent head 481678c. Consider uploading reports for the commit 481678c to get more accurate results

```diff
@@            Coverage Diff             @@
##             main    #2584      +/-   ##
==========================================
+ Coverage   81.23%   81.28%   +0.04%     
==========================================
  Files         162      162              
  Lines        9774     9798      +24     
==========================================
+ Hits         7940     7964      +24     
  Misses       1595     1595              
  Partials      239      239              
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/2584?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) | Coverage Δ | |
|---|---|---|
| [webhook/stats\_reporter.go](https://codecov.io/gh/knative/pkg/pull/2584/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-d2ViaG9vay9zdGF0c19yZXBvcnRlci5nbw==) | `85.13% <85.71%> (-0.32%)` | :arrow_down: |
| [webhook/admission.go](https://codecov.io/gh/knative/pkg/pull/2584/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-d2ViaG9vay9hZG1pc3Npb24uZ28=) | `84.61% <100.00%> (ø)` | |
| [webhook/conversion.go](https://codecov.io/gh/knative/pkg/pull/2584/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-d2ViaG9vay9jb252ZXJzaW9uLmdv) | `88.57% <100.00%> (+1.90%)` | :arrow_up: |
| [controller/controller.go](https://codecov.io/gh/knative/pkg/pull/2584/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-Y29udHJvbGxlci9jb250cm9sbGVyLmdv) | `88.78% <0.00%> (+0.71%)` | :arrow_up: |

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)

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 | 481678cbdf4c5c0e71ea35101d3576e59a73689f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562954222234767360) | true | `/test unit-tests_pkg_main`

[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 -->
k4leung4: /test unit-tests_pkg_main
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 | 481678cbdf4c5c0e71ea35101d3576e59a73689f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2584/unit-tests_pkg_main/1562957446903762944) | true | `/test unit-tests_pkg_main`

[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 -->
k4leung4: /retest
k4leung4: /retest
k4leung4: /retest

Patch:
diff --git a/webhook/admission.go b/webhook/admission.go
index 857987ad3..db131b902 100644
--- a/webhook/admission.go
+++ b/webhook/admission.go
@@ -17,9 +17,11 @@ limitations under the License.
 package webhook
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"strings"
 	"time"
@@ -91,10 +93,12 @@ func admissionHandler(rootLogger *zap.SugaredLogger, stats StatsReporter, c Admi
 		logger.Infof("Webhook ServeHTTP request=%#v", r)
 
 		var review admissionv1.AdmissionReview
-		if err := json.NewDecoder(r.Body).Decode(&review); err != nil {
+		bodyBuffer := bytes.Buffer{}
+		if err := json.NewDecoder(io.TeeReader(r.Body, &bodyBuffer)).Decode(&review); err != nil {
 			http.Error(w, fmt.Sprint("could not decode body:", err), http.StatusBadRequest)
 			return
 		}
+		r.Body = io.NopCloser(&bodyBuffer)
 
 		logger = logger.With(
 			logkey.Kind, review.Request.Kind.String(),
@@ -154,7 +158,7 @@ func admissionHandler(rootLogger *zap.SugaredLogger, stats StatsReporter, c Admi
 
 		if stats != nil {
 			// Only report valid requests
-			stats.ReportRequest(review.Request, response.Response, time.Since(ttStart))
+			stats.ReportAdmissionRequest(review.Request, response.Response, time.Since(ttStart))
 		}
 	}
 }
diff --git a/webhook/conversion.go b/webhook/conversion.go
index 201ff41f4..2e876c14f 100644
--- a/webhook/conversion.go
+++ b/webhook/conversion.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"time"
 
 	"go.uber.org/zap"
 	apixv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -37,8 +38,9 @@ type ConversionController interface {
 	Convert(context.Context, *apixv1.ConversionRequest) *apixv1.ConversionResponse
 }
 
-func conversionHandler(rootLogger *zap.SugaredLogger, _ StatsReporter, c ConversionController) http.HandlerFunc {
+func conversionHandler(rootLogger *zap.SugaredLogger, stats StatsReporter, c ConversionController) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
+		var ttStart = time.Now()
 		logger := rootLogger
 		logger.Infof("Webhook ServeHTTP request=%#v", r)
 
@@ -69,10 +71,9 @@ func conversionHandler(rootLogger *zap.SugaredLogger, _ StatsReporter, c Convers
 			return
 		}
 
-		// TODO(dprotaso) - figure out what metrics we want reported
-		// if stats != nil {
-		// 	// Only report valid requests
-		// 	stats.ReportRequest(review.Request, response.Response, time.Since(ttStart))
-		// }
+		if stats != nil {
+			// Only report valid requests
+			stats.ReportConversionRequest(review.Request, response.Response, time.Since(ttStart))
+		}
 	}
 }
diff --git a/webhook/stats_reporter.go b/webhook/stats_reporter.go
index 9d64634fe..1fe30e8af 100644
--- a/webhook/stats_reporter.go
+++ b/webhook/stats_reporter.go
@@ -25,6 +25,7 @@ import (
 	"go.opencensus.io/stats/view"
 	"go.opencensus.io/tag"
 	admissionv1 "k8s.io/api/admission/v1"
+	apixv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"knative.dev/pkg/metrics"
 )
 
@@ -57,11 +58,17 @@ var (
 	resourceResourceKey  = tag.MustNewKey("resource_resource")
 	resourceNamespaceKey = tag.MustNewKey("resource_namespace")
 	admissionAllowedKey  = tag.MustNewKey("admission_allowed")
+
+	desiredAPIVersionKey = tag.MustNewKey("desired_api_version")
+	resultStatusKey      = tag.MustNewKey("result_status")
+	resultReasonKey      = tag.MustNewKey("result_reason")
+	resultCodeKey        = tag.MustNewKey("result_code")
 )
 
 // StatsReporter reports webhook metrics
 type StatsReporter interface {
-	ReportRequest(request *admissionv1.AdmissionRequest, response *admissionv1.AdmissionResponse, d time.Duration) error
+	ReportAdmissionRequest(request *admissionv1.AdmissionRequest, response *admissionv1.AdmissionResponse, d time.Duration) error
+	ReportConversionRequest(request *apixv1.ConversionRequest, response *apixv1.ConversionResponse, d time.Duration) error
 }
 
 // reporter implements StatsReporter interface
@@ -82,7 +89,7 @@ func NewStatsReporter() (StatsReporter, error) {
 }
 
 // Captures req count metric, recording the count and the duration
-func (r *reporter) ReportRequest(req *admissionv1.AdmissionRequest, resp *admissionv1.AdmissionResponse, d time.Duration) error {
+func (r *reporter) ReportAdmissionRequest(req *admissionv1.AdmissionRequest, resp *admissionv1.AdmissionResponse, d time.Duration) error {
 	ctx, err := tag.New(
 		r.ctx,
 		tag.Insert(requestOperationKey, string(req.Operation)),
@@ -105,6 +112,25 @@ func (r *reporter) ReportRequest(req *admissionv1.AdmissionRequest, resp *admiss
 	return nil
 }
 
+// Captures req count metric, recording the count and the duration
+func (r *reporter) ReportConversionRequest(req *apixv1.ConversionRequest, resp *apixv1.ConversionResponse, d time.Duration) error {
+	ctx, err := tag.New(
+		r.ctx,
+		tag.Insert(desiredAPIVersionKey, req.DesiredAPIVersion),
+		tag.Insert(resultStatusKey, resp.Result.Status),
+		tag.Insert(resultReasonKey, string(resp.Result.Reason)),
+		tag.Insert(resultCodeKey, strconv.Itoa(int(resp.Result.Code))),
+	)
+	if err != nil {
+		return err
+	}
+
+	metrics.RecordBatch(ctx, requestCountM.M(1),
+		// Convert time.Duration in nanoseconds to milliseconds
+		responseTimeInMsecM.M(float64(d.Milliseconds())))
+	return nil
+}
+
 func RegisterMetrics() {
 	tagKeys := []tag.Key{
 		requestOperationKey,
@@ -115,7 +141,11 @@ func RegisterMetrics() {
 		resourceVersionKey,
 		resourceResourceKey,
 		resourceNamespaceKey,
-		admissionAllowedKey}
+		admissionAllowedKey,
+		desiredAPIVersionKey,
+		resultStatusKey,
+		resultReasonKey,
+		resultCodeKey}
 
 	if err := view.Register(
 		&view.View{


Test patch:
diff --git a/webhook/admission_integration_test.go b/webhook/admission_integration_test.go
index 8e143c918..caf176097 100644
--- a/webhook/admission_integration_test.go
+++ b/webhook/admission_integration_test.go
@@ -60,6 +60,32 @@ func (fac *fixedAdmissionController) Admit(ctx context.Context, req *admissionv1
 	return fac.response
 }
 
+type readBodyTwiceAdmissionController struct {
+	path     string
+	response *admissionv1.AdmissionResponse
+}
+
+var _ AdmissionController = (*readBodyTwiceAdmissionController)(nil)
+
+func (rbtac *readBodyTwiceAdmissionController) Path() string {
+	return rbtac.path
+}
+
+func (rbtac *readBodyTwiceAdmissionController) Admit(ctx context.Context, req *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
+	r := apis.GetHTTPRequest(ctx)
+	if r == nil {
+		panic("nil request!")
+	} else if r.URL.Path != rbtac.path {
+		panic("wrong path!")
+	}
+
+	var review admissionv1.AdmissionReview
+	if err := json.NewDecoder(r.Body).Decode(&review); err != nil {
+		panic("body closed!")
+	}
+	return rbtac.response
+}
+
 func TestAdmissionEmptyRequestBody(t *testing.T) {
 	c := &fixedAdmissionController{
 		path:     "/bazinga",
@@ -547,3 +573,128 @@ func TestAdmissionWarningResponseForResource(t *testing.T) {
 		}
 	}
 }
+
+func TestAdmissionValidResponseForRequestBody(t *testing.T) {
+	ac := &readBodyTwiceAdmissionController{
+		path:     "/bazinga",
+		response: &admissionv1.AdmissionResponse{},
+	}
+	wh, serverURL, ctx, cancel, err := testSetupNoTLS(t, ac)
+	if err != nil {
+		t.Fatal("testSetup() =", err)
+	}
+
+	eg, _ := errgroup.WithContext(ctx)
+	eg.Go(func() error { return wh.Run(ctx.Done()) })
+	defer func() {
+		cancel()
+		if err := eg.Wait(); err != nil {
+			t.Error("Unable to run controller:", err)
+		}
+	}()
+
+	pollErr := waitForServerAvailable(t, serverURL, testTimeout)
+	if pollErr != nil {
+		t.Fatal("waitForServerAvailable() =", err)
+	}
+	client := createNonTLSClient()
+
+	admissionreq := &admissionv1.AdmissionRequest{
+		Operation: admissionv1.Create,
+		Kind: metav1.GroupVersionKind{
+			Group:   "pkg.knative.dev",
+			Version: "v1alpha1",
+			Kind:    "Resource",
+		},
+	}
+	testRev := createResource("testrev")
+	marshaled, err := json.Marshal(testRev)
+	if err != nil {
+		t.Fatal("Failed to marshal resource:", err)
+	}
+
+	admissionreq.Resource.Group = "pkg.knative.dev"
+	admissionreq.Object.Raw = marshaled
+	rev := &admissionv1.AdmissionReview{
+		Request: admissionreq,
+	}
+
+	reqBuf := new(bytes.Buffer)
+	err = json.NewEncoder(reqBuf).Encode(&rev)
+	if err != nil {
+		t.Fatal("Failed to marshal admission review:", err)
+	}
+
+	u, err := url.Parse("http://" + serverURL)
+	if err != nil {
+		t.Fatal("bad url", err)
+	}
+
+	u.Path = path.Join(u.Path, ac.Path())
+
+	req, err := http.NewRequest("GET", u.String(), reqBuf)
+	if err != nil {
+		t.Fatal("http.NewRequest() =", err)
+	}
+	req.Header.Add("Content-Type", "application/json")
+
+	doneCh := make(chan struct{})
+	launchedCh := make(chan struct{})
+	go func() {
+		defer close(doneCh)
+
+		close(launchedCh)
+		response, err := client.Do(req)
+		if err != nil {
+			t.Error("Failed to get response", err)
+			return
+		}
+
+		if got, want := response.StatusCode, http.StatusOK; got != want {
+			t.Errorf("Response status code = %v, wanted %v", got, want)
+			return
+		}
+
+		defer response.Body.Close()
+		responseBody, err := ioutil.ReadAll(response.Body)
+		if err != nil {
+			t.Error("Failed to read response body", err)
+			return
+		}
+
+		reviewResponse := admissionv1.AdmissionReview{}
+
+		err = json.NewDecoder(bytes.NewReader(responseBody)).Decode(&reviewResponse)
+		if err != nil {
+			t.Error("Failed to decode response:", err)
+			return
+		}
+
+		if diff := cmp.Diff(rev.TypeMeta, reviewResponse.TypeMeta); diff != "" {
+			t.Errorf("expected the response typeMeta to be the same as the request (-want, +got)\n%s", diff)
+			return
+		}
+	}()
+
+	// Wait for the goroutine to launch.
+	<-launchedCh
+
+	// Check that Admit calls block when they are initiated before informers sync.
+	select {
+	case <-time.After(100 * time.Millisecond):
+	case <-doneCh:
+		t.Fatal("Admit was called before informers had synced.")
+	}
+
+	// Signal the webhook that informers have synced.
+	wh.InformersHaveSynced()
+
+	// Check that after informers have synced that things start completing immediately (including outstanding requests).
+	select {
+	case <-doneCh:
+	case <-time.After(5 * time.Second):
+		t.Error("Timed out waiting on Admit to complete after informers synced.")
+	}
+
+	metricstest.CheckStatsReported(t, requestCountName, requestLatenciesName)
+}
diff --git a/webhook/stats_reporter_test.go b/webhook/stats_reporter_test.go
index 755c14cfb..e3911b4d4 100644
--- a/webhook/stats_reporter_test.go
+++ b/webhook/stats_reporter_test.go
@@ -22,12 +22,13 @@ import (
 	"time"
 
 	admissionv1 "k8s.io/api/admission/v1"
+	apixv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/metrics/metricstest"
 	_ "knative.dev/pkg/metrics/testing"
 )
 
-func TestWebhookStatsReporter(t *testing.T) {
+func TestWebhookStatsReporterAdmission(t *testing.T) {
 	setup()
 	req := &admissionv1.AdmissionRequest{
 		UID:       "705ab4f5-6393-11e8-b7cc-42010a800002",
@@ -58,8 +59,45 @@ func TestWebhookStatsReporter(t *testing.T) {
 		admissionAllowedKey.Name():  strconv.FormatBool(resp.Allowed),
 	}
 
-	r.ReportRequest(req, resp, time.Duration(shortTime)*time.Millisecond)
-	r.ReportRequest(req, resp, time.Duration(longTime)*time.Millisecond)
+	if err := r.ReportAdmissionRequest(req, resp, time.Duration(shortTime)*time.Millisecond); err != nil {
+		t.Fatalf("ReportAdmissionRequest() = %v", err)
+	}
+	if err := r.ReportAdmissionRequest(req, resp, time.Duration(longTime)*time.Millisecond); err != nil {
+		t.Fatalf("ReportAdmissionRequest() = %v", err)
+	}
+
+	metricstest.CheckCountData(t, requestCountName, expectedTags, 2)
+	metricstest.CheckDistributionData(t, requestLatenciesName, expectedTags, 2, shortTime, longTime)
+}
+
+func TestWebhookStatsReporterConversion(t *testing.T) {
+	setup()
+	req := &apixv1.ConversionRequest{
+		UID:               "705ab4f5-6393-11e8-b7cc-42010a800003",
+		DesiredAPIVersion: "knative.dev/v1",
+	}
+
+	resp := &apixv1.ConversionResponse{
+		UID:    req.UID,
+		Result: metav1.Status{Status: "Failure", Reason: metav1.StatusReasonNotFound, Code: 404},
+	}
+
+	r, _ := NewStatsReporter()
+
+	shortTime, longTime := 1100.0, 9100.0
+	expectedTags := map[string]string{
+		desiredAPIVersionKey.Name(): req.DesiredAPIVersion,
+		resultStatusKey.Name():      resp.Result.Status,
+		resultReasonKey.Name():      string(resp.Result.Reason),
+		resultCodeKey.Name():        strconv.Itoa(int(resp.Result.Code)),
+	}
+
+	if err := r.ReportConversionRequest(req, resp, time.Duration(shortTime)*time.Millisecond); err != nil {
+		t.Fatalf("ReportConversionRequest() = %v", err)
+	}
+	if err := r.ReportConversionRequest(req, resp, time.Duration(longTime)*time.Millisecond); err != nil {
+		t.Fatalf("ReportConversionRequest() = %v", err)
+	}
 
 	metricstest.CheckCountData(t, requestCountName, expectedTags, 2)
 	metricstest.CheckDistributionData(t, requestLatenciesName, expectedTags, 2, shortTime, longTime)