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

deprecate request_count/request_duration_seconds and document in release notes #1010

Closed
sozercan opened this issue Dec 7, 2020 · 6 comments · Fixed by #1428
Closed

deprecate request_count/request_duration_seconds and document in release notes #1010

sozercan opened this issue Dec 7, 2020 · 6 comments · Fixed by #1428
Assignees
@julianKatz
Labels
enhancement New feature or request
Milestone
Mutation Beta

Comments

@sozercan
Copy link
Member

sozercan commented Dec 7, 2020 •
edited

Describe the solution you'd like
As discussed in https://github.com/open-policy-agent/gatekeeper/pull/976/files#r529165013, we will be deprecating request_count and request_duration_seconds metrics (which is validation only) to validation_request_count/validation_request_duration_seconds and mutation_request_count/mutation_request_duration_seconds.

We will publish all 3 metrics initially (no prefix, validation_ and mutation_ prefixes). No prefix and validation_ prefixes will be reporting same values. After deprecation period ends, request_count and request_duration_seconds metrics will be removed.

Document this in next release notes and metrics docs.

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

  • Gatekeeper version:
  • Kubernetes version: (use kubectl version):
@sozercan sozercan added the enhancement New feature or request label Dec 7, 2020
@sozercan sozercan mentioned this issue Dec 7, 2020
Merged
@sozercan sozercan mentioned this issue Jun 9, 2021
Closed
@ritazh ritazh added this to the Mutation Beta milestone Jun 16, 2021
@julianKatz
Copy link
Contributor

Per this comment thread, I believe we can narrow the scope of this bug to only adding the following metrics:

  • validation_request_count
  • validation_request_duration_seconds

Per @sozercan's initial comment, these will live alongside request_count and request_duration_seconds (both of which apply to validation requests only, despite the general naming.

The mutation_ metrics will be added as part of #1214. See the doc for the full list of proposed mutation metrics.

As the mutation_ metrics are out of scope for this bug, the required change is decoupled from #1214 and can be done simultaneously.

@sozercan
Copy link
Member Author

sozercan commented Jun 30, 2021 •
edited

SGTM. Just want to make sure it's visible so we call out in the release notes. Then in a future release we can remove request_ ones to complete transition.

@julianKatz
Copy link
Contributor

julianKatz commented Jul 2, 2021 •
edited

I now realize that I misunderstood this bug.

Both validation_request... and mutation_request... are already implemented in the webhook.

All that's left to do to close out this bug is remove the request_count and request_duration_seconds metrics, leaving behind:

  • validation_request_count
  • validation_request_duration_seconds
  • mutation_request_count
  • mutation_request_duration_seconds

All of which are already implemented in stats_reporter.go.

@sozercan, do you know when the "deprecation period" ends? Can we remove request_count and request_duration_seconds yet?

@sozercan
Copy link
Member Author

sozercan commented Jul 2, 2021

Looks like we already announced this in v3.4.0 notes: https://github.com/open-policy-agent/gatekeeper/releases/tag/v3.4.0 I think it's okay to remove them in next release (v3.6). @maxsmythe thoughts?

@maxsmythe
Copy link
Contributor

SGTM!

@julianKatz julianKatz self-assigned this Jul 8, 2021
julianKatz added a commit to julianKatz/gatekeeper that referenced this issue Jul 8, 2021
@julianKatz
Remove non-specific webhook request metrics
5af16dc 
As g8r once had only a validation webhook, certain metrics
(request_count, request_count_duration_seconds) were implemented without
qualifiers.  i.e., the fact that they applied to the validation webhook
was _assumed_.

As g8r now has both validation and mutation webhooks, separate metrics
for both are required.  These metrics have already been implemented, and
have been released since v3.4.0.  The original metrics were deprecated
(but retained) at the time of that release.

As we are approaching v3.6, it is finally time to remove these metrics.
This PR removes the code and updates the metrics README.

Fixes open-policy-agent#1010

Signed-off-by: juliankatz <juliankatz@google.com>
@julianKatz julianKatz mentioned this issue Jul 8, 2021
Merged
@julianKatz
Copy link
Contributor

@sozercan @maxsmythe I've put up a PR to remove these. PTAL

julianKatz added a commit that referenced this issue Jul 13, 2021
@julianKatz
Remove non-specific webhook request metrics (#1428)
786e2db 
As g8r once had only a validation webhook, certain metrics
(request_count, request_count_duration_seconds) were implemented without
qualifiers.  i.e., the fact that they applied to the validation webhook
was _assumed_.

As g8r now has both validation and mutation webhooks, separate metrics
for both are required.  These metrics have already been implemented, and
have been released since v3.4.0.  The original metrics were deprecated
(but retained) at the time of that release.

As we are approaching v3.6, it is finally time to remove these metrics.
This PR removes the code and updates the metrics README.

Fixes #1010

Signed-off-by: juliankatz <juliankatz@google.com>
julianKatz added a commit to julianKatz/gatekeeper that referenced this issue Jul 27, 2021
@julianKatz
Remove non-specific webhook request metrics (open-policy-agent#1428)
a0e7c16 
As g8r once had only a validation webhook, certain metrics
(request_count, request_count_duration_seconds) were implemented without
qualifiers.  i.e., the fact that they applied to the validation webhook
was _assumed_.

As g8r now has both validation and mutation webhooks, separate metrics
for both are required.  These metrics have already been implemented, and
have been released since v3.4.0.  The original metrics were deprecated
(but retained) at the time of that release.

As we are approaching v3.6, it is finally time to remove these metrics.
This PR removes the code and updates the metrics README.

Fixes open-policy-agent#1010

Signed-off-by: juliankatz <juliankatz@google.com>
@fanny-jiang fanny-jiang mentioned this issue Sep 21, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@julianKatz julianKatz

Labels
enhancement New feature or request
Projects
None yet
Milestone
Mutation Beta
Development

Successfully merging a pull request may close this issue.

Remove non-specific webhook request metrics julianKatz/gatekeeper
4 participants
@sozercan @ritazh @julianKatz @maxsmythe