Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Add timeout delivery metrics #1578

Merged
merged 3 commits into from
Aug 14, 2020
Merged

Conversation

grac3gao-zz
Copy link
Contributor

@grac3gao-zz grac3gao-zz commented Aug 13, 2020

Fixes #1522

Proposed Changes

  • If the delivery is cancelled because of timeout, report event dispatch time without resp status code. In metric explorer, the dispatch time will look like: (trigger-actor-0 only has timeout event, trigger-actor-1 only has 2xx event)
    Screen Shot 2020-08-13 at 9 53 40 AM

  • Add and update some UT

Release Note

🎁 add timeout delivery metrics to broker fanout and retry 

Docs

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grac3gao

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

@google-cla google-cla bot added the cla: yes (override cla status due to multiple authors bug) label Aug 13, 2020
Copy link
Member

@yolocs yolocs left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

Maybe add Ian for a look.

pkg/metrics/delivery_reporter.go Outdated Show resolved Hide resolved
@knative-test-reporter-robot

The following jobs failed:

Test name Triggers Retries
pull-google-knative-gcp-wi-tests 0/3

Failed non-flaky tests preventing automatic retry of pull-google-knative-gcp-wi-tests:

test/e2e.TestSmokeChannel
test/e2e.TestSmokeCloudPubSubSourceV1beta1
test/e2e.TestSmokeCloudSchedulerSourceV1beta1
test/e2e.TestCloudAuditLogsSourceWithGCPBroker

@grac3gao-zz
Copy link
Contributor Author

/retest

Copy link
Member

@yolocs yolocs left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold
In case @ian-mi has comments

@knative-metrics-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-google-knative-gcp-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/broker/handler/processors/deliver/processor.go 80.0% 80.2% 0.2
pkg/metrics/delivery_reporter.go 81.8% 82.6% 0.8

@ian-mi
Copy link
Contributor

ian-mi commented Aug 14, 2020

/lgtm

@ian-mi
Copy link
Contributor

ian-mi commented Aug 14, 2020

/unhold

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved cla: yes (override cla status due to multiple authors bug) lgtm size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add timeout delivery metrics
7 participants