Add a new stat metric in queue to prevent double counting

[hohaichi]

Currently a request pending in both the activator and queue proxy is counted twice, once in the activator, and one in the queue proxy. This change fixes it by letting queue report a concurrency metric for proxied requests and letting the autoscaler discount such concurrency when it calculates the total concurrency for scaling decision.

Fixes #3301

Proposed Changes

• Add a new Knative header for tracking where a request goes through. And let the activator use it to mark proxied requests.
• Let queue report a new metric for the average concurrency of requests proxied through activator.
• Autoscaler discount the average proxied concurrency from the average concurrency from queue when it calculate the total concurrency for scaling decision.

Release Note

[knative-prow-robot]

@hohaichi: 0 warnings.

In response to this:

Currently a request pending in both the activator and queue proxy is counted twice, once in the activator, and one in the queue proxy. This change fixes it by letting queue report a concurrency metric for proxied requests and letting the autoscaler discount such concurrency when it calculates the total concurrency for scaling decision.

Fixes #3301

Proposed Changes

• Add a new Knative header for tracking where a request goes through. And let the activator use it to mark proxied requests.
• Let queue report a new metric for the average concurrency of requests proxied through activator.
• Autoscaler discount the average proxied concurrency from the average concurrency from queue when it calculate the total concurrency for scaling decision.

Release Note

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vagababov]

In general looks fine.
But it still has the same attack vector that the underlying issue talks about.
Perhaps the header should be dynamic?

[markusthoemmes]

Very neat execution, thanks! Left a few comments throughout.

[hohaichi]

In general looks fine.
But it still has the same attack vector that the underlying issue talks about.
Perhaps the header should be dynamic?
I think an attacker can attack the same way using 'k-network-probe' header. It's probably not a real thread, or we should have the same solution for both.

[hohaichi]

Updated with tests and rebased.

[hohaichi]

/assign @mdemirhan
/assign @markusthoemmes

[vagababov]

A few nits.
Rest is fine with me.

[markusthoemmes]

Generally LGTM but a few comments on naming and adding comments describing the values.

[hohaichi]

/assign @yanweiguo

[knative-metrics-robot]

The following is the coverage report on pkg/.
Say /test pull-knative-serving-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/autoscaler/autoscaler.go	97.2%	97.0%	-0.2
pkg/autoscaler/stats_scraper.go	83.7%	90.3%	6.6
pkg/queue/stats_reporter.go	87.8%	87.5%	-0.3

[yanweiguo]

/lgtm

[markusthoemmes]

/approve
/lgtm

Thanks for all the adjustments 🎉

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hohaichi, markusthoemmes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• cmd/queue/OWNERS [markusthoemmes]
• pkg/activator/OWNERS [markusthoemmes]
• pkg/autoscaler/OWNERS [markusthoemmes]
• pkg/network/OWNERS [markusthoemmes]
• pkg/queue/OWNERS [markusthoemmes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hohaichi]

/retest