Skip to content

add event interval from rest client metric broken down by source#27976

Closed
tkashem wants to merge 1 commit intoopenshift:masterfrom
tkashem:client-view
Closed

add event interval from rest client metric broken down by source#27976
tkashem wants to merge 1 commit intoopenshift:masterfrom
tkashem:client-view

Conversation

@tkashem
Copy link
Contributor

@tkashem tkashem commented Jun 10, 2023

No description provided.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 10, 2023
@openshift-ci openshift-ci bot requested review from bparees and jwforres June 10, 2023 12:15
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 10, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tkashem
Once this PR has been reviewed and has the lgtm label, please assign bparees for approval. For more information see the Kubernetes Code Review Process.

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

Details 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


func (g *gatherer) query(ctx context.Context) (prometheustypes.Value, error) {
// TODO: should we include 5xx? "429|5..|<error>"
query := `rest_client_requests_total{code =~ "429|5..|<error>"}`
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can add DNS resolve error - or it won't be recorded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, when Michal's upstream PR merges, we can incorporate it.

Copy link
Contributor Author

@tkashem tkashem Jun 13, 2023

Choose a reason for hiding this comment

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

to be more clear, if dns error happens code=<error>, any other network error, code=<error> also. (we can't make the distinction today with rest_client_requests_total. Michal's PR adds a new metric just to capture dns error.

@dinhxuanvu
Copy link
Member

The upstream PR: kubernetes/kubernetes#115357

@tkashem tkashem changed the title [WIP] add event interval from rest client metric broken down by source add event interval from rest client metric broken down by source Jun 12, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 12, 2023
@tkashem tkashem force-pushed the client-view branch 2 times, most recently from 3ea8c6a to e7cc6c2 Compare June 13, 2023 23:04
@tkashem
Copy link
Contributor Author

tkashem commented Jun 14, 2023

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 14, 2023

@tkashem: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openstack-ovn 2c7ac51 link false /test e2e-openstack-ovn
ci/prow/e2e-aws-ovn-single-node-serial 2c7ac51 link false /test e2e-aws-ovn-single-node-serial
ci/prow/e2e-agnostic-ovn-cmd 2c7ac51 link false /test e2e-agnostic-ovn-cmd
ci/prow/e2e-aws-ovn-upgrade 2c7ac51 link false /test e2e-aws-ovn-upgrade
ci/prow/e2e-aws-ovn-single-node-upgrade 2c7ac51 link false /test e2e-aws-ovn-single-node-upgrade

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@tkashem
Copy link
Contributor Author

tkashem commented Jun 15, 2023

/retest-required

@dinhxuanvu
Copy link
Member

@p0lyn0mial @dgrisonnet Would like to have your inputs on this PR. Abu indicates this PR is ready for merge. We can either merge it and I can create subsequent PRs to address feedbacks or incorporate those changes into linked Vadim's PR.

@dgrisonnet
Copy link
Member

I think we should close this one and wrap up the work on the PR Vadim opened and improved.

@dinhxuanvu
Copy link
Member

I think we should close this one and wrap up the work on the PR Vadim opened and improved.

Sounds fair. Let do that.

@vrutkovs
Copy link
Contributor

/close

@openshift-ci openshift-ci bot closed this Jun 21, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 21, 2023

@vrutkovs: Closed this PR.

Details

In response to this:

/close

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants