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.

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

feat: improve configurability of prometheus metrics #450

Conversation

claudio-benfatto
Copy link
Contributor

Related issue

Proposed changes

This PR introduces a new boolean configuration parameter collapse_request_paths in the prometheus configuration stanza of the oathkeeper.yml file:

serve:
  prometheus:
    port: 9000
    host: localhost
    metrics_path: /metrics
    collapse_request_paths: true

The parameter is optional and if not set it defaults to true.

When set to true it modifies the behaviour of the metrics Middleware so that for all the request metrics that contain the request label (which is equal to the request context path) its value is collapsed to only the first element of the segment. Eg.

curl http://localhost:4456/decisions/service1/users
curl http://localhost:4456/decisions/service1/topics

will result in the following request metrics (trimmed for reading convenience):

# TYPE ory_oathkeeper_request_duration_seconds histogram
ory_oathkeeper_request_duration_seconds_count{method="GET",request="/decisions",service="oathkeeper-api",status_code="404"} 2
# HELP ory_oathkeeper_requests_total Total number of requests
# TYPE ory_oathkeeper_requests_total counter
ory_oathkeeper_requests_total{method="GET",request="/decisions",service="oathkeeper-api",status_code="404"} 2

When collapse_request_paths is set to false the metrics will keep the path previous fine granularity:

# HELP ory_oathkeeper_request_duration_seconds Time spent serving requests.
# TYPE ory_oathkeeper_request_duration_seconds histogram
ory_oathkeeper_request_duration_seconds_count{method="GET",request="/decisions/service1/topics",service="oathkeeper-api",status_code="404"} 1
ory_oathkeeper_request_duration_seconds_count{method="GET",request="/decisions/service1/users",service="oathkeeper-api",status_code="404"} 1
# HELP ory_oathkeeper_requests_total Total number of requests
# TYPE ory_oathkeeper_requests_total counter
ory_oathkeeper_requests_total{method="GET",request="/decisions/service1/topics",service="oathkeeper-api",status_code="404"} 1
ory_oathkeeper_requests_total{method="GET",request="/decisions/service1/users",service="oathkeeper-api",status_code="404"} 1

Checklist

  • I have read the contributing guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further comments

I'm happy to modify any logic or naming I somehow arbitrarily chosen!

the collapse_request_paths parameter will collapse the context path
for the request label in the prometheus metrics to a single segment
add a CollapsePaths method to the metrics.Middleware.
When set to true collapse the request path to only the first segment.
@claudio-benfatto claudio-benfatto changed the title Feat improve configurability of prometheus metrics feat: improve configurability of prometheus metrics May 28, 2020
@FredrikAppelros
Copy link

Is there anything missing from this PR that blocks it from being reviewed? Anything we can do to help speed it up?

@aeneasr
Copy link
Member

aeneasr commented Jun 26, 2020

No, my bad, I just completely forgot to review it, sorry!

@aeneasr aeneasr merged commit ddcb226 into ory:master Jun 26, 2020
@aeneasr
Copy link
Member

aeneasr commented Jun 26, 2020

Thank you for your hard work!

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.

3 participants