metrics decoration improvements about gardener-metrics-exporter HOT 8 CLOSED

Also if these improvements are ok for you we are willing to implement these (@alcasim), just let us know.

from gardener-metrics-exporter.

Thanks @bergerx for the proposal.

FYI: We plan to provide some dashboards for the metrics of the exporter. The PR is already filed #6. May this fits already to some your needs. Please feel free to try out and provide feedback.

Basically I'm fine with extending the metrics in the manner you suggested. Are there other reason for the extension than just avoiding joins? In the provided dashboards we are also using joins and they seems to work well.

from gardener-metrics-exporter.

Wow that's cool we'll check it shortly

Are there other reason for the extension than just avoiding joins?

Not really, current set of tags could work for a purposely designed dashboards. But we still want to show shoot/seed/iaas related information also in other dashboards without having to run extra queries if possible.

from gardener-metrics-exporter.

#10 seems to be solving some part but this one slipped:

garden_shoot_operation_states should have a state tag that tells about how the last operation has resulted.

@fsniper can you have a look?

from gardener-metrics-exporter.

Hmm, I checked the metrics again, actually, #10 doesn't address those, but somehow we adopted the join method @dkistner suggested :)

So I was checking what's really missing and found that only the garden_shoot_condition has missing potentially valuable labels.

I was checking what actually the garden_shoot_condition metric is pointing, and I'm kind of confused. This metric seems to be a composition from apparently 2 independent fields under status (lastOperation and conditions):

Here are 2 garden_shoot_condition metrics for a cluster:

garden_shoot_condition{condition="APIServerAvailable",mail_to="",name="testbed",operation="Reconcile",project="test",purpose=""} 1
garden_shoot_condition{condition="ControlPlaneHealthy",mail_to="",name="testbed",operation="Reconcile",project="test",purpose=""} 1

Here is the shoot's related fields:

  status:
    conditions:
    - lastTransitionTime: "2019-01-11T21:30:57Z"
      lastUpdateTime: "2019-01-30T12:38:52Z"
      message: Shoot API server /healthz endpoint responded with success status code.
        [response_time:3ms]
      reason: HealthzRequestSucceeded
      status: "True"
      type: APIServerAvailable
    - lastTransitionTime: "2019-01-27T07:11:33Z"
      lastUpdateTime: "2019-01-30T12:38:52Z"
      message: All control plane components are healthy.
      reason: ControlPlaneRunning
      status: "True"
      type: ControlPlaneHealthy
...
    lastOperation:
      description: Shoot cluster state has been successfully reconciled.
      lastUpdateTime: "2019-01-30T12:11:04Z"
      progress: 100
      state: Succeeded
      type: Reconcile

So for the garden_shoot_condition metric i'd expect to reflect the keys under the conditions as labels. Maybe not all but at least the .conditions[].status fields should be in the metric labels. And optionally the reason as well, but I dont know if the reason field could have different values ever.

from gardener-metrics-exporter.

The .conditions[].status is reflected by value of the metric see here: https://github.com/gardener/gardener-metrics-exporter/blob/master/pkg/metrics/utils.go#L31-L42.
The value of the .conditions[].reason field is at least specific per condition and would lead to high label cardinality and its therefore not a good idea to have it as label, because we want to archive one timeseries per condition and we would get for every reason a new timeseries. It's the same for the .conditions[].message. The operation field of the condition goes in the same direction, but it could produce only a small cardinality (Create|Delete|Reconcile). But I have to check if this is still required.

from gardener-metrics-exporter.

Hmmm, encoding distinct values to a metric doesnt seem right, will require us to copy the enum value decode logic to dashboards we use this metrics.
But at least we have the info available to consume.

So maybe the reason would be the only field that would be added. I don't know if it would make sense. I'm closing this issue anyway we can create another one if we need.

Thanks for the comments.

from gardener-metrics-exporter.

Hmmm, encoding distinct values to a metric doesnt seem right, will require us to copy the enum value decode logic to dashboards we use this metrics.

Yeah that's right unfortunately you need to duplicate the enum value within a dashboard. The value of the metric garden_shoot_condition should reflect the current status of a condition. The status of a condition could have the value True|False|Unknown. Prometheus allow only to store floats for metric values (that's the reason why we need this mapping). Labels are basically meant as identifiers and for selection of those metrics and should therefore not contain any status information.

from gardener-metrics-exporter.