Use view.Count() instead of view.Sum() for request count stats

[anniefu]

Proposed Changes

• For stats/metrics that represent integer counter values, use opencensus.io view.Count() instead of view.Sum()
• Because recording an opencensus CountData always converts to a Measurement of 1, remove ability to pass int value into "ReportRequestCount" StatsReporter APIs for pkg/activator and pkg/queue

The CountData is intended for integers and is the appropriate type. This also ensures that Knative request_count metrics are exported to Prometheus as Prometheus "counter" metrics instead of "untyped" (see opencensus-go Supported metric primitives and opencensus-go-exporter-prometheus/prometheus.go).

Example of incorrect prometheus type when using opencensus view.Sum() (from /metrics endpoint)

# HELP activator_request_count The number of requests that are routed to Activator
# TYPE activator_request_count untyped
activator_request_count{configuration_name="helloworld-go",namespace_name="default",num_tries="2",response_code="200",response_code_class="2xx",revision_name="helloworld-go-ls5dq",service_name="helloworld-go"} 1

Example of correct prometheus type when using opencensus view.Count()

# HELP autoscaler_reconcile_count Number of reconcile operations
# TYPE autoscaler_reconcile_count counter
autoscaler_reconcile_count{key="default/helloworld-go",reconciler="KPA-Class Autoscaling",success="true"} 1

Note: This is not an issue for Knative 0.8.0+, because the opencensus-go package has been updated and view.Sum() now converts to Prometheus "counter" type, likely as a side effect of this opencensus-go change.

For future reference when using view.Sum(), there are open issues about the opencensus to prometheus counter type conversion being inconsistent across implementations (see opencensus-spec Exported type/kind conversion for different aggregates and opencensus-csharp Counter type conversion inconsistency in Prometheus exporter).

[knative-prow-robot]

@anniefu: 0 warnings.

In response to this:

Proposed Changes

• For stats/metrics that represent integer counter values, use opencensus.io view.Count() instead of view.Sum()

The CountData is intended for integers and is the appropriate type. This also ensures that Knative request_count metrics are exported to Prometheus as Prometheus "counter" metrics instead of "untyped" (see opencensus-go Supported metric primitives and opencensus-go-exporter-prometheus/prometheus.go).

Example of incorrect prometheus type when using opencensus view.Sum() (from /metrics endpoint)

# HELP activator_request_count The number of requests that are routed to Activator
# TYPE activator_request_count untyped
activator_request_count{configuration_name="helloworld-go",namespace_name="default",num_tries="2",response_code="200",response_code_class="2xx",revision_name="helloworld-go-ls5dq",service_name="helloworld-go"} 1

Example of correct prometheus type when using opencensus view.Count()

# HELP autoscaler_reconcile_count Number of reconcile operations
# TYPE autoscaler_reconcile_count counter
autoscaler_reconcile_count{key="default/helloworld-go",reconciler="KPA-Class Autoscaling",success="true"} 1

Note: This is not an issue for Knative 0.8.0+, because the opencensus-go package has been updated and view.Sum() now converts to Prometheus "counter" type, likely as a side effect of this opencensus-go change.

For future reference when using view.Sum(), there are open issues about the opencensus to prometheus counter type conversion being inconsistent across implementations (see opencensus-spec Exported type/kind conversion for different aggregates and opencensus-csharp Counter type conversion inconsistency in Prometheus exporter).

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.

[anniefu]

/assign @mdemirhan
/assign @yanweiguo
/assign @ssmall
/assign @trshafer

[yanweiguo]

unit tests need update

[anniefu]

unit tests need update

For view.Count()/CountData, passing in a non-1 value at record time is equivalent to passing in a 1, so I removed the value parameter from the ReportRequestCount functions. I edited the PR description to reflect this.

[ssmall]

/lgtm

[yanweiguo]

/lgtm

[mdemirhan]

/lgtm
/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anniefu, mdemirhan

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:

• pkg/activator/OWNERS [mdemirhan]
• pkg/queue/OWNERS [mdemirhan]

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