kiali / kiali

Kiali project, observability for the Istio service mesh
https://www.kiali.io
Apache License 2.0
3.41k stars 494 forks source link

[K-charted] Handle unit scaling when input already scaled #1715

Closed jotak closed 5 years ago

jotak commented 5 years ago

(This is an issue to be confirmed) Related: https://github.com/kiali/kiali/pull/1711

I believe a unit in charts model set as "ms" for millisecond is going to be scaled as "kms" when x1000.

Solution: add a "scale" field in model that tells how it is already scaled in prometheus. E.g. , instead of the current:

    {
        Chart: kmodel.Chart{
            Name:  "Request duration",
            Unit:  "ms",
            Spans: 6,
        },
        refName: "request_duration_millis",
    },

we would have something like:

    {
        Chart: kmodel.Chart{
            Name:  "Request duration",
            Unit:  "seconds",
            Scale: 0.001,
            Spans: 6,
        },
        refName: "request_duration_millis",
    },
jotak commented 5 years ago

cc @jshaughn

jshaughn commented 5 years ago

What I don't understand is the x1000 comment, because the numbers in prom are in ms, and it is passed to k-charted in ms, so where is the manual scaling coming into play? In my local tests of the charts for https://github.com/kiali/kiali/pull/1711, the chart looked good to me.

jotak commented 5 years ago

But @jshaughn in your tests did you had some high response times, like over a second? That's the scenario where I think we will see this bug.

PS: look here: https://www.kiali.io/documentation/runtimes-monitoring/#units "Other units will fall into the default case and be scaled using SI standard. For instance, unit: "m" for meter can be scaled up to km."