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

Provide a metric that measures the time for authorizing #5017

Closed
minwoox opened this issue Jul 10, 2023 · 2 comments · Fixed by #5074
Closed

Provide a metric that measures the time for authorizing #5017

minwoox opened this issue Jul 10, 2023 · 2 comments · Fixed by #5074

Comments

@minwoox
Copy link
Member

minwoox commented Jul 10, 2023

Usually, it takes a long to authorize a request because the authorizer mostly needs to call another backend.
It would be nice if we provide a timer for the AuthService so that users easily get to know how long it takes.

@erie0210
Copy link

Can you specify how users get to know the timer? (How to provide time for the user.) Should it be like return with response field? or would it be better in header? :)

@minwoox
Copy link
Member Author

minwoox commented Jul 11, 2023

Hi!
We can create a timer using Micrometer API and register it to MeterRegistry.
(You might want to refer to these lines of code)

public static Timer newTimer(MeterRegistry registry, String name, Iterable<Tag> tags) {
requireNonNull(registry, "registry");
requireNonNull(name, "name");
requireNonNull(tags, "tags");
final Double maxExpectedValueNanos = distStatCfg.getMaximumExpectedValueAsDouble();
final Double minExpectedValueNanos = distStatCfg.getMinimumExpectedValueAsDouble();
final Duration maxExpectedValue =
maxExpectedValueNanos != null ? Duration.ofNanos(maxExpectedValueNanos.longValue()) : null;
final Duration minExpectedValue =
minExpectedValueNanos != null ? Duration.ofNanos(minExpectedValueNanos.longValue()) : null;
return Timer.builder(name)
.tags(tags)
.maximumExpectedValue(maxExpectedValue)
.minimumExpectedValue(minExpectedValue)
.publishPercentiles(distStatCfg.getPercentiles())
.publishPercentileHistogram(distStatCfg.isPercentileHistogram())
.percentilePrecision(distStatCfg.getPercentilePrecision())
.distributionStatisticBufferLength(distStatCfg.getBufferLength())
.distributionStatisticExpiry(distStatCfg.getExpiry())
.register(registry);
}

If so, we can get the information using by looking at the registry. We don't have to return to the client or whoever.
Usually, the collected registy is sent to the collector such as Prometheus, so that we can investigate if something happens later.

If you are not familiar with metric, I recommend you to read this document:
https://micrometer.io/docs/concepts

After that, we can create a timer in the AuthService and measure the duration here:

public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exception {
return HttpResponse.from(AuthorizerUtil.authorizeAndSupplyHandlers(authorizer, ctx, req)
.handleAsync((result, cause) -> {
try {

Then, we can call this API to record it:
timer.record(duration, TimeUnit.NANOSECONDS)

Please, let me know if you have any further questions. 😉

jrhee17 added a commit that referenced this issue Mar 28, 2024
Motivation:

Fixing an open improvement issue.
#5017

Modifications:

- Modified the `AuthService.java` file to use `MoreMeters.newTimer`.
- Updated the tests. I think some are still remaining to be fixed, so
opening the PR as draft.
  - Any feedback is welcome as it's my first time! 😄  

Result:

- Closes #5017. (If this resolves the issue.)
- Describe the consequences that a user will face after this PR is
merged.

<!--
Visit this URL to learn more about how to write a pull request
description:

https://armeria.dev/community/developer-guide#how-to-write-pull-request-description
-->
The metrics would look like:
```
# HELP armeria_server_auth_seconds  
# TYPE armeria_server_auth_seconds summary
armeria_server_auth_seconds_count 1.0
armeria_server_auth_seconds_sum 0.018437417
# HELP armeria_server_auth_seconds_max  
# TYPE armeria_server_auth_seconds_max gauge
armeria_server_auth_seconds_max 0.018437417
```

---------

Co-authored-by: jrhee17 <guins_j@guins.org>
Co-authored-by: songmw725 <songmw725@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants