Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

feat(metrics): generalize admission webhook metrics #4597

Merged
merged 4 commits into from
Mar 23, 2022
Merged

feat(metrics): generalize admission webhook metrics #4597

merged 4 commits into from
Mar 23, 2022

Conversation

nojnhuh
Copy link
Contributor

@nojnhuh nojnhuh commented Mar 16, 2022

Description:
This change adds the osm_admission_webhook_response_total metric to
track all mutating and validating webhook responses by Kubernetes
resource kind and whether the request was successful or not.

Example:

osm_admission_webhook_response_total{kind="TrafficTarget",success="true"} 4

Part of #4568

Testing done:

  • Updated unit tests
  • Did some manual smoke testing with the whole control plane running

Affected area:

Functional Area
New Functionality [X]
CI System [ ]
CLI Tool [ ]
Certificate Management [ ]
Control Plane [ ]
Demo [ ]
Documentation [ ]
Egress [ ]
Ingress [ ]
Install [ ]
Networking [ ]
Observability [X]
Performance [ ]
SMI Policy [ ]
Security [ ]
Sidecar Injection [X]
Tests [ ]
Upgrade [ ]
Other [ ]

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project? No

    • Did you notify the maintainers and provide attribution?
  2. Is this a breaking change? Yes, release notes updated

  3. Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)? Yes, add webhook metric osm-docs#341

nojnhuh added 2 commits March 16, 2022 14:27
This change adds the `osm_admission_webhook_response_total` metric to
track all mutating and validating webhook responses by Kubernetes
resource kind and whether the request was successful or not.

Example:

    osm_admission_webhook_response_total{kind="TrafficTarget",success="true"} 4

Part of #4568

Signed-off-by: Jon Huhn <[email protected]>
Signed-off-by: Jon Huhn <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Mar 16, 2022

Codecov Report

Merging #4597 (4998456) into main (caaa189) will decrease coverage by 0.03%.
The diff coverage is 72.41%.

@@            Coverage Diff             @@
##             main    #4597      +/-   ##
==========================================
- Coverage   68.73%   68.69%   -0.04%     
==========================================
  Files         217      216       -1     
  Lines       14954    14982      +28     
==========================================
+ Hits        10278    10292      +14     
- Misses       4624     4639      +15     
+ Partials       52       51       -1     
Flag Coverage Δ
unittests 68.69% <72.41%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/osm-controller/osm-controller.go 15.31% <0.00%> (-0.07%) ⬇️
pkg/metricsstore/metricsstore.go 91.90% <36.36%> (-4.25%) ⬇️
pkg/httpserver/httpserver.go 84.61% <100.00%> (-1.83%) ⬇️
pkg/injector/webhook.go 87.95% <100.00%> (-0.36%) ⬇️
pkg/validator/server.go 77.08% <100.00%> (+0.32%) ⬆️
pkg/webhook/webhook.go 94.73% <100.00%> (+1.87%) ⬆️
pkg/validator/validators.go 86.79% <0.00%> (-2.46%) ⬇️
pkg/envoy/cds/cluster.go 92.83% <0.00%> (-0.42%) ⬇️
pkg/trafficpolicy/trafficpolicy.go 97.46% <0.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update caaa189...4998456. Read the comment docs.

None
The following capabilities have been deprecated and cannot be used.

- The `osm_injector_injector_sidecar_count` and `osm_injector_injector_rq_time` metrics have been removed. The `osm_admission_webhook_response_total` and `osm_http_response_duration` metrics should be used instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

With this change, the osm_injector_injector_rq_time and osm_injector_injector_sidecar_count metrics can be removed from the OSM Mesh and Envoy Details Grafana dashboard.

Copy link
Contributor Author

@nojnhuh nojnhuh Mar 17, 2022

Choose a reason for hiding this comment

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

I took a pass at editing the dashboards to use the new metrics instead of removing the dashboards completely.

@nojnhuh nojnhuh merged commit 02e6d7d into openservicemesh:main Mar 23, 2022
@nojnhuh nojnhuh deleted the adm-webhook-metrics branch March 23, 2022 16:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants