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

Report event_count metric in Broker Ingress for invalid events #1798

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Report event_count metric in Broker Ingress for invalid events #1798

merged 1 commit into from
Oct 7, 2020

Conversation

AlexandraRoatis
Copy link
Contributor

@AlexandraRoatis AlexandraRoatis commented Oct 5, 2020 •
edited
Loading

Fixes #1706

Proposed Changes

  • Report event_count metric in Broker Ingress for invalid events

Release Note

Invalid Broker Ingress events now produce the event_count metric with event type _invalid_cloud_event_.

Docs

@google-cla google-cla bot added the cla: yes (override cla status due to multiple authors bug) label Oct 5, 2020
zhongduo
zhongduo reviewed Oct 5, 2020
View reviewed changes
pkg/metrics/tags.go Outdated
@@ -58,6 +58,7 @@ var (
allowedEventTypes = map[string]struct{}{
"e2e-dummy-event-type": {},
"e2e-testing-resp-event-type-dummy": {},
"_invalid_cloud_event_": {},
Copy link
Contributor

Choose a reason for hiding this comment

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

The alignment of {} suggests that your editor is not set up with auto formatting properly. Make sure you either set it up or add a commit hook to your git repo.

Harwayne
Harwayne reviewed Oct 5, 2020
View reviewed changes
Copy link
Contributor

@Harwayne Harwayne left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlexandraRoatis, Harwayne

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

@knative-test-reporter-robot
Copy link

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.TestSmokeCloudPubSubSourceWithDeletion
test/e2e.TestCloudSchedulerSourceWithTargetTestImpl

@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/ingress/handler.go 94.0% 94.1% 0.1

@AlexandraRoatis AlexandraRoatis changed the title Report event_count metric in Broker Ingress for invalid events WIP: Report event_count metric in Broker Ingress for invalid events Oct 6, 2020
@AlexandraRoatis AlexandraRoatis changed the title WIP: Report event_count metric in Broker Ingress for invalid events Report event_count metric in Broker Ingress for invalid events Oct 6, 2020
Harwayne
Harwayne reviewed Oct 7, 2020
View reviewed changes
Copy link
Contributor

@Harwayne Harwayne left a comment

Choose a reason for hiding this comment

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

/lgtm

@knative-prow-robot knative-prow-robot merged commit c1d24f9 into google:master Oct 7, 2020
@AlexandraRoatis AlexandraRoatis deleted the fix-1706 branch October 13, 2020 14:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Harwayne Harwayne Harwayne left review comments

@zhongduo zhongduo zhongduo left review comments

@grantr grantr Awaiting requested review from grantr

@yolocs yolocs Awaiting requested review from yolocs

Assignees

@Harwayne Harwayne

Labels
approved cla: yes (override cla status due to multiple authors bug) lgtm size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Report event_count metric in Broker Ingress when request is not a valid event
6 participants
@AlexandraRoatis @knative-prow-robot @knative-test-reporter-robot @knative-metrics-robot @Harwayne @zhongduo