Eventing TLS: Add scheme label to metrics #7581

sadath-12 · 2024-01-15T13:47:10Z

Fixes #6838

Proposed Changes

Added scheme label on ReporterArgs to be set by url.scheme in the request if it is empty http will be set

Pre-review Checklist

At least 80% unit test coverage
E2E tests for any new behavior
Docs PR for any user-facing impact
Spec PR for any new API feature
Conformance test for any change to the spec

Release Note

Docs

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

knative-prow · 2024-01-15T13:47:19Z

Welcome @sadath-12! It looks like this is your first PR to knative/eventing 🎉

knative-prow · 2024-01-15T13:47:20Z

Hi @sadath-12. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

sadath-12 · 2024-01-15T13:47:31Z

PR is WIP

pierDipi · 2024-01-15T15:31:33Z

@sadath-12 when the PR is WIP you can add [WIP] in the title so that we and the CI bots know, for example:

[WIP] Eventing TLS: Add scheme label to metrics

sadath-12 · 2024-01-15T15:34:47Z

@sadath-12 when the PR is WIP you can add [WIP] in the title so that we and the CI bots know, for example:
[WIP] Eventing TLS: Add scheme label to metrics

thanks for letting know @pierDipi 😊

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

sadath-12 · 2024-01-15T16:20:49Z

ready for review

Cali0707 · 2024-01-15T16:23:48Z

/ok-to-test

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

sadath-12 · 2024-01-15T16:50:17Z

/retest

vendor/github.com/cloudevents/sdk-go/v2/event/event_reader.go

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

pierDipi · 2024-01-18T11:21:40Z

Can you check the test failures https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_eventing/7581/unit-tests_eventing_main/1747823117209702400 and code style errors https://github.com/knative/eventing/actions/runs/7564936038/job/20612089237?pr=7581 ?

You can run unit tests locally with ./test/presubmit-tests.sh --unit-tests

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

codecov · 2024-01-18T14:19:02Z

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (c7520cc) 74.56% compared to head (94af9d0) 74.44%.
Report is 13 commits behind head on main.

❗ Current head 94af9d0 differs from pull request most recent head f1db6a6. Consider uploading reports for the commit f1db6a6 to get more accurate results

Files	Patch %	Lines
pkg/broker/filter/filter_handler.go	13.33%	11 Missing and 2 partials ⚠️
pkg/channel/fanout/fanout_event_handler.go	71.79%	7 Missing and 4 partials ⚠️
pkg/broker/ingress/ingress_handler.go	62.50%	1 Missing and 2 partials ⚠️
pkg/channel/event_receiver.go	40.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7581      +/-   ##
==========================================
- Coverage   74.56%   74.44%   -0.12%     
==========================================
  Files         262      262              
  Lines       14951    15041      +90     
==========================================
+ Hits        11148    11198      +50     
- Misses       3215     3243      +28     
- Partials      588      600      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

pkg/adapter/v2/cloudevents.go

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

pkg/broker/filter/filter_handler.go

pkg/broker/ingress/ingress_handler.go

pkg/channel/event_receiver.go

pkg/channel/fanout/fanout_event_handler.go

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

pkg/channel/fanout/fanout_event_handler.go

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

pierDipi · 2024-01-22T13:48:07Z

/assign @Cali0707

pierDipi · 2024-01-22T13:58:33Z

@sadath-12 see code style errors https://github.com/knative/eventing/actions/runs/7611074750/job/20729046441?pr=7581 and https://github.com/knative/eventing/actions/runs/7611074750/job/20729047741?pr=7581

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

pierDipi · 2024-01-22T15:05:46Z

A few more https://github.com/knative/eventing/actions/runs/7612814283/job/20731946087?pr=7581

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

sadath-12 · 2024-01-22T15:30:07Z

Thanks @pierDipi @Cali0707 lot of lessons learned from this pr

Cali0707

/lgtm
/cc @pierDipi
For a final approval on this one, as I'm less familiar with metrics reporting code

pierDipi · 2024-01-22T16:03:07Z

/approve

Thanks for the contribution @sadath-12

knative-prow · 2024-01-22T16:03:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi, sadath-12

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:

~~OWNERS~~ [pierDipi]

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

Eventing TLS: Add scheme label to metrics

bf03b7d

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

knative-prow bot requested review from Cali0707 and odacremolbap January 15, 2024 13:47

knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 15, 2024

sadath-12 changed the title ~~Eventing TLS: Add scheme label to metrics~~ [WIP] Eventing TLS: Add scheme label to metrics Jan 15, 2024

knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 15, 2024

setting the scheme

bca035b

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 15, 2024

sadath-12 changed the title ~~[WIP] Eventing TLS: Add scheme label to metrics~~ Eventing TLS: Add scheme label to metrics Jan 15, 2024

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 15, 2024

knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 15, 2024

sadath-12 added 2 commits January 15, 2024 22:08

fix scheme

e4524cd

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

fix minors

66d9178

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

Cali0707 requested changes Jan 15, 2024

View reviewed changes

vendor/github.com/cloudevents/sdk-go/v2/event/event_reader.go Outdated Show resolved Hide resolved

knative-prow bot assigned Cali0707 Jan 15, 2024

vendor undo

8d0583c

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 15, 2024

minorfix

8993e45

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

scheme fix

84753b3

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

sadath-12 requested a review from Cali0707 January 18, 2024 03:28

null pointer fix

3debf6d

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

lintfix

eca3d13

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

pierDipi reviewed Jan 18, 2024

View reviewed changes

pkg/adapter/v2/cloudevents.go Outdated Show resolved Hide resolved

replace scheme place

3fa4797

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

sadath-12 requested a review from pierDipi January 18, 2024 17:17

Cali0707 requested changes Jan 19, 2024

View reviewed changes

sadath-12 added 3 commits January 20, 2024 07:52

minor advances

07906ca

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

minoradvc

9fb2b05

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

minor adv

4ad255e

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

sadath-12 requested a review from Cali0707 January 20, 2024 02:29

pierDipi reviewed Jan 22, 2024

View reviewed changes

pkg/channel/fanout/fanout_event_handler.go Outdated Show resolved Hide resolved

restructure subscription handling

faf3071

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

sadath-12 requested a review from pierDipi January 22, 2024 12:26

lintfix

94af9d0

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

lintfix

f1db6a6

Signed-off-by: sadath-12 <sadathsadu2002@gmail.com>

Cali0707 reviewed Jan 22, 2024

View reviewed changes

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2024

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 2024

knative-prow bot merged commit 44ff98b into knative:main Jan 22, 2024
38 of 40 checks passed

maschmid mentioned this pull request Apr 24, 2024

imc dispatcher metrics show duplicate entries with event_scheme="" #7869

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eventing TLS: Add scheme label to metrics #7581

Eventing TLS: Add scheme label to metrics #7581

sadath-12 commented Jan 15, 2024 •

edited

Loading

knative-prow bot commented Jan 15, 2024

knative-prow bot commented Jan 15, 2024

sadath-12 commented Jan 15, 2024

pierDipi commented Jan 15, 2024 •

edited

Loading

sadath-12 commented Jan 15, 2024

sadath-12 commented Jan 15, 2024

Cali0707 commented Jan 15, 2024

sadath-12 commented Jan 15, 2024 •

edited

Loading

pierDipi commented Jan 18, 2024 •

edited

Loading

codecov bot commented Jan 18, 2024 •

edited

Loading

pierDipi commented Jan 22, 2024

pierDipi commented Jan 22, 2024

pierDipi commented Jan 22, 2024

sadath-12 commented Jan 22, 2024 •

edited

Loading

Cali0707 left a comment

pierDipi commented Jan 22, 2024

knative-prow bot commented Jan 22, 2024

Eventing TLS: Add scheme label to metrics #7581

Eventing TLS: Add scheme label to metrics #7581

Conversation

sadath-12 commented Jan 15, 2024 • edited Loading

Proposed Changes

Pre-review Checklist

knative-prow bot commented Jan 15, 2024

knative-prow bot commented Jan 15, 2024

sadath-12 commented Jan 15, 2024

pierDipi commented Jan 15, 2024 • edited Loading

sadath-12 commented Jan 15, 2024

sadath-12 commented Jan 15, 2024

Cali0707 commented Jan 15, 2024

sadath-12 commented Jan 15, 2024 • edited Loading

pierDipi commented Jan 18, 2024 • edited Loading

codecov bot commented Jan 18, 2024 • edited Loading

Codecov Report

pierDipi commented Jan 22, 2024

pierDipi commented Jan 22, 2024

pierDipi commented Jan 22, 2024

sadath-12 commented Jan 22, 2024 • edited Loading

Cali0707 left a comment

Choose a reason for hiding this comment

pierDipi commented Jan 22, 2024

knative-prow bot commented Jan 22, 2024

sadath-12 commented Jan 15, 2024 •

edited

Loading

pierDipi commented Jan 15, 2024 •

edited

Loading

sadath-12 commented Jan 15, 2024 •

edited

Loading

pierDipi commented Jan 18, 2024 •

edited

Loading

codecov bot commented Jan 18, 2024 •

edited

Loading

sadath-12 commented Jan 22, 2024 •

edited

Loading