pkg icon indicating copy to clipboard operation
pkg copied to clipboard

[wip] Webhook OTel changes

Open dprotaso opened this issue 6 months ago • 3 comments

This stacks onto the shared main PR - https://github.com/knative/pkg/pull/3190

You can review the webhook changes here

  • webhook changes for observability
  • go mod
  • vendor

dprotaso avatar Jun 25 '25 13:06 dprotaso

/assign @Cali0707 @evankanderson @skonto

dprotaso avatar Jun 25 '25 13:06 dprotaso

Codecov Report

Attention: Patch coverage is 88.69565% with 13 lines in your changes missing coverage. Please review.

Project coverage is 76.00%. Comparing base (7a5377f) to head (624cd94). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
webhook/conversion.go 68.57% 8 Missing and 3 partials :warning:
webhook/metrics.go 91.66% 1 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3189      +/-   ##
==========================================
- Coverage   76.05%   76.00%   -0.05%     
==========================================
  Files         205      205              
  Lines       11751    11710      -41     
==========================================
- Hits         8937     8900      -37     
+ Misses       2541     2540       -1     
+ Partials      273      270       -3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Jun 26 '25 04:06 codecov[bot]

This is ready for a review cc @evankanderson @Cali0707

Based on the feedback from the OTel folks I removed the package variable instruments. This makes unit tests easier to write and FYI the instruments are de-duped by the otel-go sdk when multiple webhooks are created (excluding the pointers in our webhook struct)

dprotaso avatar Jun 30 '25 18:06 dprotaso

rebased

dprotaso avatar Jul 04 '25 14:07 dprotaso

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Cali0707, dprotaso, evankanderson

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~~ [Cali0707,dprotaso]

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

knative-prow[bot] avatar Jul 04 '25 14:07 knative-prow[bot]

/hold cancel

@evankanderson if you have any follow up comments let me know and i can do them in a follow up PR next week.

dprotaso avatar Jul 04 '25 14:07 dprotaso