Skip to content

Add metrics for topologyInjector webhook#5835

Merged
zirain merged 8 commits intoenvoyproxy:mainfrom
jukie:topology-injector-metrics
Apr 29, 2025
Merged

Add metrics for topologyInjector webhook#5835
zirain merged 8 commits intoenvoyproxy:mainfrom
jukie:topology-injector-metrics

Conversation

@jukie
Copy link
Copy Markdown
Contributor

@jukie jukie commented Apr 27, 2025

What type of PR is this?

What this PR does / why we need it:
Folllow-up to #5352

Which issue(s) this PR fixes:

Fixes #

Release Notes: No

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@jukie jukie marked this pull request as ready for review April 27, 2025 17:46
@jukie jukie requested a review from a team as a code owner April 27, 2025 17:46
@jukie
Copy link
Copy Markdown
Contributor Author

jukie commented Apr 27, 2025

CC @zirain

zirain
zirain previously approved these changes Apr 27, 2025
Copy link
Copy Markdown
Contributor

@shawnh2 shawnh2 left a comment

Choose a reason for hiding this comment

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

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 28, 2025

Codecov Report

Attention: Patch coverage is 23.07692% with 10 lines in your changes missing coverage. Please review.

Project coverage is 65.35%. Comparing base (ba034fb) to head (a7b9695).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/provider/kubernetes/topology_injector.go 23.07% 10 Missing ⚠️

❌ Your patch status has failed because the patch coverage (23.07%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5835      +/-   ##
==========================================
+ Coverage   65.31%   65.35%   +0.03%     
==========================================
  Files         221      221              
  Lines       35314    35321       +7     
==========================================
+ Hits        23066    23084      +18     
+ Misses      10820    10814       -6     
+ Partials     1428     1423       -5     

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

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

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@zirain zirain force-pushed the topology-injector-metrics branch from c09c948 to d21b0c7 Compare April 28, 2025 05:23
jukie added 2 commits April 28, 2025 10:35
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@jukie
Copy link
Copy Markdown
Contributor Author

jukie commented Apr 28, 2025

/retest

@jukie
Copy link
Copy Markdown
Contributor Author

jukie commented Apr 28, 2025

Not sure how to go about adding tests for the metrics change. Is that required for merging?

Copy link
Copy Markdown
Contributor

@shawnh2 shawnh2 left a comment

Choose a reason for hiding this comment

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

lgtm

@zirain
Copy link
Copy Markdown
Member

zirain commented Apr 29, 2025

Not sure how to go about adding tests for the metrics change. Is that required for merging?

it would be better to have, but not a blocker.

it shouldn't be very hard to add in the existing zone e2e.

@zirain zirain merged commit cf11963 into envoyproxy:main Apr 29, 2025
36 of 38 checks passed
@jukie jukie deleted the topology-injector-metrics branch April 29, 2025 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants