Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1768483: Limit Cardinality of Marketplace quay metrics #259

Conversation

awgreene
Copy link
Member

@awgreene awgreene commented Oct 28, 2019

This commit introduces a series of record rules that lump http status codes returned by Default AppRegistries into buckets of 100.

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /docs
  • Commit messages sensible and descriptive

Solves #264

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 28, 2019
@kevinrizza
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2019
@awgreene
Copy link
Member Author

/hold

Waiting for someone from telemeter to comment before merging.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 28, 2019
manifests/12_prometheus_rule.yaml Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2019
manifests/12_prometheus_rule.yaml Outdated Show resolved Hide resolved
manifests/12_prometheus_rule.yaml Outdated Show resolved Hide resolved
@awgreene
Copy link
Member Author

@brancz should be all set for a review

@awgreene
Copy link
Member Author

/retest

4 similar comments
@awgreene
Copy link
Member Author

/retest

@awgreene
Copy link
Member Author

/retest

@awgreene
Copy link
Member Author

/retest

@awgreene
Copy link
Member Author

/retest

Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

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

manifests/12_prometheus_rule.yaml Outdated Show resolved Hide resolved
manifests/12_prometheus_rule.yaml Outdated Show resolved Hide resolved
manifests/12_prometheus_rule.yaml Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 31, 2019
@awgreene
Copy link
Member Author

This file should be removed then? Or at least the content of it? https://github.com/operator-framework/operator-marketplace/blob/c72dc6c093fa8592b83a74ab2839d65fa59f58a3/deploy/prometheus_rule.yaml

Good point @lilic, I have updated the contents of the file.

Copy link

@brancz brancz left a comment

Choose a reason for hiding this comment

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

One last thing, then lgtm.

- alert: CommunityOperatorConnectionErrors
annotations:
message: Unable to connect to the default OperatorSource community-operators AppRegistry for {{ $value }}% of requests.
expr: increase(app_registry_request_total{code!~"2..",opsrc="community-operators"}[5m]) / increase(app_registry_request_total{opsrc="community-operators"}[5m]) > 0.5
Copy link

Choose a reason for hiding this comment

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

Increase is a rather unusual use for this, rate is typically used for the purpose that we use it here for.

Copy link
Member Author

Choose a reason for hiding this comment

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

@brancz Addressed in the latest version of this PR, but would you mind sharing the reasoning?

@awgreene
Copy link
Member Author

awgreene commented Nov 4, 2019

/retest

@awgreene awgreene changed the title [metrics] Update metrics reported by marketplace Bug 1768483: Limit Cardinality of Marketplace quay metrics Nov 4, 2019
@openshift-ci-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Nov 4, 2019
@openshift-ci-robot
Copy link
Contributor

@awgreene: This pull request references Bugzilla bug 1768483, which is invalid:

  • expected the bug to target the "4.3.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1768483: Limit Cardinality of Marketplace quay metrics

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.

@awgreene
Copy link
Member Author

awgreene commented Nov 4, 2019

/bugzilla refresh

@openshift-ci-robot
Copy link
Contributor

@awgreene: This pull request references Bugzilla bug 1768483, which is invalid:

  • expected the bug to target the "4.3.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

@awgreene
Copy link
Member Author

awgreene commented Nov 4, 2019

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Nov 4, 2019
@openshift-ci-robot
Copy link
Contributor

@awgreene: This pull request references Bugzilla bug 1768483, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

/bugzilla refresh

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.

@openshift-ci-robot openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Nov 4, 2019
@awgreene
Copy link
Member Author

awgreene commented Nov 4, 2019

/retest

4 similar comments
@awgreene
Copy link
Member Author

awgreene commented Nov 4, 2019

/retest

@awgreene
Copy link
Member Author

awgreene commented Nov 4, 2019

/retest

@awgreene
Copy link
Member Author

awgreene commented Nov 4, 2019

/retest

@awgreene
Copy link
Member Author

awgreene commented Nov 4, 2019

/retest

@lilic
Copy link
Member

lilic commented Nov 5, 2019

/retest

Think the upstream failure should be fixed already.

@awgreene
Copy link
Member Author

awgreene commented Nov 5, 2019

/retest

@anik120
Copy link
Contributor

anik120 commented Nov 5, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 5, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awgreene, gallettilance, kevinrizza

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:

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

@awgreene
Copy link
Member Author

awgreene commented Nov 5, 2019

/retest

level=info msg="Cluster operator ingress Available is False with IngressUnavailable: Not all ingress controllers are available."```

@awgreene
Copy link
Member Author

awgreene commented Nov 5, 2019

/retest

1105 17:32:03.615061     825 factory_object_mapping.go:423] Failed to download OpenAPI (the server could not find the requested resource), falling back to swagger
6) Interacting with an `AllNamespaces` install mode Operator (Jaeger)
   ✖ displays subscription creation form for selected Operator (1 failure)
A Jasmine spec timed out. Resetting the WebDriver Control Flow.
   • selects all namespaces for Operator subscription
   • displays Operator as subscribed in OperatorHub
   • displays Operator in "Cluster Service Versions" view for "test-cttoo" namespace
   • creates Operator `Deployment`
   • recreates Operator `Deployment` if manually deleted

@awgreene
Copy link
Member Author

awgreene commented Nov 5, 2019

/bugzilla refresh

@openshift-ci-robot
Copy link
Contributor

@awgreene: This pull request references Bugzilla bug 1768483, which is valid.

In response to this:

/bugzilla refresh

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.

@awgreene
Copy link
Member Author

awgreene commented Nov 5, 2019

/test e2e-aws-console-olm

@openshift-merge-robot openshift-merge-robot merged commit 530c85d into operator-framework:master Nov 5, 2019
@openshift-ci-robot
Copy link
Contributor

@awgreene: All pull requests linked via external trackers have merged. Bugzilla bug 1768483 has been moved to the MODIFIED state.

In response to this:

Bug 1768483: Limit Cardinality of Marketplace quay metrics

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants