Enable metrics by Danil-Grigorev · Pull Request #324 · openshift/cluster-api-provider-aws

Danil-Grigorev · 2020-05-21T14:46:21Z

Working on https://issues.redhat.com/browse/OCPCLOUD-784

This PR is adding support for reporting following prometheus metrics and also starting controller-runtime metrics server to make these metrics available for prometheus servers:

mapi_instance_create_failed: Total count of "create" cloud api errors
mapi_instance_update_failed: Total count of "update" cloud api errors
mapi_instance_delete_failed: Total count of "delete" cloud api errors

Labels on these metrics (for AWS):

prometheus.Labels{
  "name": machine.Name,
  "namespace": machine.Namespace,
  "reason": error.Reason
}

Depends on PR openshift/machine-api-operator#609 introducing metrics support

openshift-ci-robot · 2020-05-21T14:46:36Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Danil-Grigorev · 2020-05-22T14:14:55Z

Waits for PR openshift/machine-api-operator#590 to merge.

JoelSpeed · 2020-06-04T14:13:18Z

go.mod

I'd prefer this to be moved back to master openshift before merge
Hold until we can identify whether this is absolutely necessary
/hold

enxebre · 2020-06-05T11:53:04Z

pkg/actuators/machine/reconciler.go

why is this ignoring this error now?

The method always returns nil as an error.

how is this related to this commit?

It is not, but I didn't like it 😅

enxebre · 2020-06-05T12:29:53Z

pkg/actuators/machine/instances.go

how are this metrics actually being exposed?
wouldn't this need to metrics.Registry.MustRegister(failedInstanceCreateCount) or anything?
https://github.com/kubernetes-sigs/controller-runtime/blob/c0438568a706ec61de31b92f4d76e7fb7e1007b9/pkg/internal/controller/metrics/metrics.go#L50

This happens inside the init in https://github.com/openshift/machine-api-operator/pull/590/files#diff-7cbe8e056d62a2de30c7066e359bd9c9R68

JoelSpeed

Did we need to update the vendor directory as part of this PR?

@enxebre Did you want to separate the providerID change so that it's a separate commit/PR to make it more obvious?

Danil-Grigorev · 2020-06-17T13:44:09Z

Did we need to update the vendor directory as part of this PR?

@enxebre Did you want to separate the providerID change so that it's a separate commit/PR to make it more obvious?

Created #331

Danil-Grigorev · 2020-06-17T14:30:06Z

/hold Waiting on openshift/machine-api-operator#590

JoelSpeed · 2020-06-18T15:29:14Z

/lgtm

JoelSpeed · 2020-06-19T12:02:16Z

/retest

Danil-Grigorev · 2020-06-19T14:47:00Z

/retest

elmiko

/lgtm

enxebre · 2020-07-01T08:39:37Z

go.mod

why is this needed at all? gcp is not even a fork

enxebre · 2020-07-01T08:42:01Z

Can you please include a link to the mao counter PR in the PR desc and the commit desc?
Other than the above and #324 (comment) lgtm

- Report instance creation and deletion failures - Report loadBalancer operation failures

- Include PR openshift/machine-api-operator#609 introducing metrics support

Danil-Grigorev · 2020-07-07T10:40:38Z

@enxebre @JoelSpeed @alexander-demichev I'd like to merge those today. Please approve and LGTM

elmiko

/lgtm

JoelSpeed

/approve

openshift-ci-robot · 2020-07-08T11:38:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed

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~~ [JoelSpeed]

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

openshift-bot · 2020-07-08T13:28:33Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-07-08T13:41:30Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 21, 2020

openshift-ci-robot requested review from enxebre and michaelgugino May 21, 2020 14:46

Danil-Grigorev marked this pull request as ready for review May 21, 2020 15:25

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 21, 2020

enxebre added the release/4.6 label May 22, 2020

Danil-Grigorev force-pushed the enable-metrics branch 7 times, most recently from 0d6bac3 to 889f784 Compare May 22, 2020 14:11

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 30, 2020

Danil-Grigorev force-pushed the enable-metrics branch 3 times, most recently from 80b7172 to 2303036 Compare June 4, 2020 10:10

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 4, 2020

JoelSpeed reviewed Jun 4, 2020

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 4, 2020

Danil-Grigorev force-pushed the enable-metrics branch from 2303036 to 43cfb33 Compare June 4, 2020 15:52

enxebre reviewed Jun 5, 2020

View reviewed changes

JoelSpeed reviewed Jun 17, 2020

View reviewed changes

Danil-Grigorev force-pushed the enable-metrics branch from 43cfb33 to 905a7b2 Compare June 17, 2020 13:48

Danil-Grigorev mentioned this pull request Jun 17, 2020

Enable prometheus metrics for controllers openshift/machine-api-operator#609

Merged

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2020

Danil-Grigorev requested review from JoelSpeed and enxebre June 18, 2020 15:24

Danil-Grigorev mentioned this pull request Jun 18, 2020

Enable metrics to support updated MAO metrics integration openshift/cluster-api-provider-openstack#100

Closed

openshift-ci-robot assigned JoelSpeed Jun 18, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 18, 2020

elmiko approved these changes Jun 30, 2020

View reviewed changes

openshift-ci-robot assigned elmiko Jun 30, 2020

enxebre reviewed Jul 1, 2020

View reviewed changes

go.mod Outdated

Copy link

Member

enxebre Jul 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed at all? gcp is not even a fork

Danil-Grigorev force-pushed the enable-metrics branch from 675a823 to 73c2229 Compare July 2, 2020 08:55

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2020

Danil-Grigorev force-pushed the enable-metrics branch from 73c2229 to 0f8fe0b Compare July 2, 2020 09:04

Danil-Grigorev added 2 commits July 6, 2020 19:01

Add metrics for machine interactions

0d9743e

- Report instance creation and deletion failures - Report loadBalancer operation failures

Revendor MAO

fa7eef7

- Include PR openshift/machine-api-operator#609 introducing metrics support

Danil-Grigorev force-pushed the enable-metrics branch from 0f8fe0b to fa7eef7 Compare July 7, 2020 07:59

Danil-Grigorev requested review from elmiko and enxebre July 7, 2020 10:39

elmiko approved these changes Jul 7, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 7, 2020

JoelSpeed reviewed Jul 8, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2020

openshift-merge-robot merged commit 34dd2b6 into openshift:master Jul 8, 2020

Conversation

Danil-Grigorev commented May 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented May 21, 2020

Uh oh!

Danil-Grigorev commented May 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Danil-Grigorev commented Jun 17, 2020

Uh oh!

Danil-Grigorev commented Jun 17, 2020

Uh oh!

JoelSpeed commented Jun 18, 2020

Uh oh!

JoelSpeed commented Jun 19, 2020

Uh oh!

Danil-Grigorev commented Jun 19, 2020

Uh oh!

elmiko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enxebre commented Jul 1, 2020

Uh oh!

Danil-Grigorev commented Jul 7, 2020

Uh oh!

elmiko left a comment

Choose a reason for hiding this comment

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Jul 8, 2020

Uh oh!

openshift-bot commented Jul 8, 2020

Uh oh!

openshift-bot commented Jul 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Danil-Grigorev commented May 21, 2020 •

edited

Loading