Instrumenting follow-up: metrics labelling w/ finer granularity by yue9944882 · Pull Request #1171 · kubernetes-client/java

yue9944882 · 2020-08-19T07:20:15Z

the new classes are picked from open-telemetry/opentelemetry-java-instrumentation#651, except for those groovy unit tests..

a sample output from the prometheus example will be:

k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.005",} 0.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.01",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.025",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.05",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.075",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.1",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.25",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.5",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="0.75",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="1.0",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="2.5",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="5.0",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="7.5",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="10.0",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",le="+Inf",} 2.0
k8s_java_resource_request_latency_seconds_count{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",} 2.0
k8s_java_resource_request_latency_seconds_sum{http_response_code="404",api_group="",api_version="v1",resource="pods",subresource="",api_verb="get",namespace="bar",} 0.014409807
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.005",} 0.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.01",} 0.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.025",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.05",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.075",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.1",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.25",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.5",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="0.75",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="1.0",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="2.5",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="5.0",} 1.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="7.5",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="10.0",} 2.0
k8s_java_resource_request_latency_seconds_bucket{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",le="+Inf",} 2.0
k8s_java_resource_request_latency_seconds_count{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",} 2.0
k8s_java_resource_request_latency_seconds_sum{http_response_code="200",api_group="",api_version="v1",resource="pods",subresource="",api_verb="list",namespace="",} 5.198464195000001

brendandburns · 2020-08-19T16:01:50Z

I don't think we want to remove the overall histogram for request latencies, given the fine grained histograms it's impossible to re-assemble the broader histograms.

Likewise, it's a little weird to write a query like rate(k8s_java_resource_request_latency_seconds_count{code="500"}) to get the rate of errors. I get that we might not want to have extra Counter metrics, but I feel like it makes more sense than overloading the histogram metrics.

So I would retain both of the Counters and also the all-up Histogram.

The rest of the additional labels LGTM.

yue9944882 · 2020-08-20T04:46:31Z

given the fine grained histograms it's impossible to re-assemble the broader histograms.

@brendandburns if i understand correctly, we can always aggregate finer-grained histogram/counter to overall histogram/counter by promQL functions --- generally the fine-grained metrics is a superset of overall metrics:

static final Counter requests =
Counter.build().name(PREFIX + "_requests_total").help("Total requests").register();

will be equal to

sum(rate(k8s_java_resource_request_latency_seconds_sum[10m]) by (code)

Histogram.build() Histogram.build()
.name(PREFIX + "_request_latency_seconds")

can be aggregated by

histogram_quantile(0.9, sum (rate(k8s_java_resource_request_latency_seconds_bucket[10m]) by (le)))

am i missing anything?

brendandburns · 2020-08-20T17:01:10Z

a) I believe that the first calculation is a histogram of the average latency, not the actual latency, so it's lossy.

b) The second calculation is valid, but I think it's hard for a person to understand, since why would I sum latency count to get # of requests. So while the calculation works, I think it adds complexity.

yue9944882 · 2020-08-20T17:39:22Z

a) I believe that the first calculation is a histogram of the average latency, not the actual latency, so it's lossy.

oh oversight! i was referring to k8s_java_resource_request_latency_seconds_count to replace the counter.

b) The second calculation is valid, but I think it's hard for a person to understand, since why would I sum latency count to get # of requests. So while the calculation works, I think it adds complexity.

yeah i agree the aggregation is complex and not intuitive enough, even tho they're overlapping mathematically. will keep counter and all-up histogram

yue9944882 · 2020-08-21T07:32:29Z

@brendandburns i added the request-counter and response-code-counter back, PTAL

brendandburns · 2020-08-21T22:13:48Z

+    this.name = name;
+  }
+
+  private final String apiGroup;


TODO: update to group/version/kind object.

i did the TODO right now

brendandburns · 2020-08-21T22:14:56Z

+    }
+
+    codes.labels(Integer.toString(response.code())).inc();
+    ;


Stray semi-colon.

brendandburns · 2020-08-21T22:15:25Z

One TODO for the future (unless you feel like doing it now) and one typo, then LGTM.

brendandburns · 2020-08-26T04:39:23Z

/lgtm
/approve

k8s-ci-robot · 2020-08-26T04:39:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, yue9944882

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [brendandburns,yue9944882]

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

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 19, 2020

k8s-ci-robot requested review from brendandburns and mbohlool August 19, 2020 07:20

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 19, 2020

metrics labelling w/ finer granularity

b2cf0a0

yue9944882 force-pushed the follow-up-prom branch from f3cf9a9 to b2cf0a0 Compare August 21, 2020 07:31

brendandburns reviewed Aug 21, 2020

View reviewed changes

addressing review comments

007f613

k8s-ci-robot assigned brendandburns Aug 26, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 26, 2020

k8s-ci-robot merged commit 4bd712d into kubernetes-client:master Aug 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Instrumenting follow-up: metrics labelling w/ finer granularity#1171

Instrumenting follow-up: metrics labelling w/ finer granularity#1171
k8s-ci-robot merged 2 commits into
kubernetes-client:masterfrom
yue9944882:follow-up-prom

yue9944882 commented Aug 19, 2020 •

edited

Loading

Uh oh!

brendandburns commented Aug 19, 2020

Uh oh!

yue9944882 commented Aug 20, 2020

Uh oh!

brendandburns commented Aug 20, 2020

Uh oh!

yue9944882 commented Aug 20, 2020

Uh oh!

yue9944882 commented Aug 21, 2020

Uh oh!

brendandburns Aug 21, 2020

Uh oh!

yue9944882 Aug 24, 2020

Uh oh!

brendandburns Aug 21, 2020

Uh oh!

yue9944882 Aug 24, 2020

Uh oh!

brendandburns commented Aug 21, 2020

Uh oh!

brendandburns commented Aug 26, 2020

Uh oh!

k8s-ci-robot commented Aug 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

yue9944882 commented Aug 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brendandburns commented Aug 19, 2020

Uh oh!

yue9944882 commented Aug 20, 2020

Uh oh!

brendandburns commented Aug 20, 2020

Uh oh!

yue9944882 commented Aug 20, 2020

Uh oh!

yue9944882 commented Aug 21, 2020

Uh oh!

brendandburns Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

yue9944882 Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

brendandburns Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

yue9944882 Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

brendandburns commented Aug 21, 2020

Uh oh!

brendandburns commented Aug 26, 2020

Uh oh!

k8s-ci-robot commented Aug 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yue9944882 commented Aug 19, 2020 •

edited

Loading