Support eventing metrics #688
Conversation
|
Hi @skonto. Thanks for your PR. I'm waiting for a knative-sandbox member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
knative-prow-robot
added
size/M
knative-prow-robot
added
the
do-not-merge/work-in-progress
|
Need to update eventing to make metric description more generic as it is incorrect. |
Codecov Report
@@ Coverage Diff @@
## main #688 +/- ##
==========================================
- Coverage 75.17% 74.93% -0.24%
==========================================
Files 132 132
Lines 5784 5801 +17
==========================================
- Hits 4348 4347 -1
- Misses 1215 1234 +19
+ Partials 221 220 -1
Continue to review full report at Codecov.
|
| // Ideally this should be moved to Eventing | ||
| func parseDispatchResultAndReportMetrics(info *eventingchannels.DispatchExecutionInfo, reporter eventingchannels.StatsReporter, reportArgs eventingchannels.ReportArgs, dispatchErr error) error { |
There was a problem hiding this comment.
I am going to create a new PR pointing to this one showing that I need to copy the same code. This is to justify the change only, will go away. That is why this is in wip ;)
There was a problem hiding this comment.
Also will fix test coverage once code is eliminated and see what is left.
|
Thanks!
On Mon 7. Jun 2021 at 20:33, Stavros Kontopoulos ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkg/channel/consolidated/dispatcher/consumer_message_handler.go
<#688 (comment)>
:
> +// Ideally this should be moved to Eventing
+func parseDispatchResultAndReportMetrics(info *eventingchannels.DispatchExecutionInfo, reporter eventingchannels.StatsReporter, reportArgs eventingchannels.ReportArgs, dispatchErr error) error {
I am going to create a new PR pointing to this one showing that I need to
copy the same code. This is to justify the change only, will go away. That
is why this is in wip ;)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#688 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABGPTVIJ7RR4NQCEYOY2QLTRUGG5ANCNFSM46H3MIHQ>
.
--
Sent from Gmail Mobile
|
|
When updates from Eventing land here I will update the PR. |
@skonto they are in 😄 |
skonto
force-pushed
the
fix_eventing_metrics
branch
from
8f11edc
to
2a85d15
Compare
knative-prow-robot
removed
the
do-not-merge/work-in-progress
|
@matzew this is ready. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: matzew, skonto 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 |
knative-prow-robot
added
the
approved
|
/ok-to-test |
knative-prow-robot
added
ok-to-test
|
The following is the coverage report on the affected files.
|
* support eventing metrics * lint * imports * update with latest deps
* patch vendor * Support eventing metrics (knative-extensions#688) * support eventing metrics * lint * imports * update with latest deps * use transformers in dispatchWithRetries instead of copying * fix use of transformers * updates * pass transformers to executeRequest
…ons#234) * patch vendor * Support eventing metrics (knative-extensions#688) * support eventing metrics * lint * imports * update with latest deps * use transformers in dispatchWithRetries instead of copying * fix use of transformers * updates * pass transformers to executeRequest
Fixes #683
Sample output:
Emit metrics when they are consumed successfully by the subscriber.