Skip to content

Adding responseCode and responseCodeClass to metrics#1784

Merged
knative-prow-robot merged 11 commits into
knative:masterfrom
nachocano:cloudevents
Sep 3, 2019
Merged

Adding responseCode and responseCodeClass to metrics#1784
knative-prow-robot merged 11 commits into
knative:masterfrom
nachocano:cloudevents

Conversation

@nachocano
Copy link
Copy Markdown
Contributor

Helps with #1693

Proposed Changes

  • Updating cloudevents sdk version to read the http response status code
  • Tagging certain metrics with the responseCode and the responseCodeClass

Release Note

NONE

@knative-prow-robot knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 1, 2019
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Sep 1, 2019
@knative-prow-robot knative-prow-robot added the area/test-and-release Test infrastructure, tests or release label Sep 1, 2019
@nachocano
Copy link
Copy Markdown
Contributor Author

@sayanh here is the change to add responseCode and responseCodeClass.
If you can follow this one while addressing the comments on the ApiServerSource as well as the changes for CronJobSource, it'd be great.

Comment thread pkg/broker/filter/filter_handler.go Outdated
Comment thread pkg/broker/filter/filter_handler.go Outdated
Comment thread pkg/broker/filter/filter_handler_test.go Outdated
Comment thread pkg/broker/filter/filter_handler.go
Comment thread pkg/broker/filter/stats_reporter.go
@n3wscott
Copy link
Copy Markdown
Contributor

n3wscott commented Sep 3, 2019

/approve

Some questions about status codes.

@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n3wscott, nachocano

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:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 3, 2019
# Conflicts:
#	pkg/broker/ingress/stats_reporter.go
@knative-metrics-robot
Copy link
Copy Markdown

The following is the coverage report on pkg/.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/broker/filter/filter_handler.go 78.9% 79.1% 0.2
pkg/broker/filter/stats_reporter.go 78.8% 77.8% -1.1
pkg/broker/ingress/ingress_handler.go 62.7% 63.3% 0.6
pkg/broker/ingress/stats_reporter.go 77.8% 77.5% -0.3
pkg/utils/utils.go 78.8% 79.4% 0.6

@n3wscott
Copy link
Copy Markdown
Contributor

n3wscott commented Sep 3, 2019

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2019
@knative-prow-robot knative-prow-robot merged commit 4a5e6e2 into knative:master Sep 3, 2019
aslom pushed a commit to aslom/eventing that referenced this pull request Sep 5, 2019
* Stats reporter refactor

* updating lock

* updates after code review

* go imports

* not working

* updates

* compilation problems

* updates after code review
pierDipi pushed a commit to pierDipi/eventing that referenced this pull request Jul 12, 2022
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. area/test-and-release Test infrastructure, tests or release cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants