Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ext_authz and ratelimiter metrics to OpenMetrics implementation #12451

Merged
merged 6 commits into from
Jul 5, 2022
Merged

Add ext_authz and ratelimiter metrics to OpenMetrics implementation #12451

merged 6 commits into from
Jul 5, 2022

Conversation

alopezz
Copy link
Contributor

@alopezz alopezz commented Jul 5, 2022

What does this PR do?

  • It adds some ratelimit and external authorization config to the integration test harness so that we can trigger at least some associated metrics for testing. It also adds requests to the dummy services when testing to ensure the triggering.
    • Fixes errors on mock services while doing it.
  • It completes missing metadata from Add ext_authz metrics to envoy integration #12374.
  • It adds ext_authz and ratelimiter metrics to the OpenMetricsV2 implementation.

Motivation

QA for #12374.

Additional Notes

I only got to trigger .error and .failure_mode_allowed metrics, as triggering the rest would require understanding those features better and a more elaborate setup (probably with more mock services).

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@alopezz alopezz requested a review from a team as a code owner July 5, 2022 11:01
@ghost ghost added the integration/envoy label Jul 5, 2022
@alopezz alopezz added category/improvement For use during Agent Release period changelog/Added labels Jul 5, 2022
@codecov
Copy link

codecov bot commented Jul 5, 2022 •
edited
Loading

Codecov Report

Merging #12451 (e9ece21) into master (7bf7924) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Flag Coverage Δ
envoy 94.32% <100.00%> (+0.07%) ⬆️

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

@ofek ofek merged commit 55391c0 into master Jul 5, 2022
@ofek ofek deleted the alopez/envoy/qa-ext-authz-ratelimiter branch July 5, 2022 14:30
ofek pushed a commit that referenced this pull request Jul 6, 2022
@alopezz @ofek
Add ext_authz and ratelimiter metrics to OpenMetrics implementation (#…
4a5c939 
…12451)

* Improve integration testing

* Improve integration testing: external authorization

And add missing metadata

* Simplify ratelimiter and external authorization test setup

* Add external authorization metrics to OpenMetricsV2 implementation

* Add ratelimit metrics to OpenMetrics V2 implementation

* Fix test running for api v2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hithwen hithwen hithwen approved these changes

Assignees
No one assigned
Labels
category/improvement For use during Agent Release period integration/envoy
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alopezz @hithwen @ofek