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

Expose the server-side metrics programmatically #4992

Open
ikhoon opened this issue Jun 28, 2023 · 6 comments · May be fixed by #5627
Open

Expose the server-side metrics programmatically #4992

ikhoon opened this issue Jun 28, 2023 · 6 comments · May be fixed by #5627
Labels
new feature sprint Issues for OSS Sprint participants
Milestone
1.29.0

Comments

@ikhoon
Copy link
Contributor

ikhoon commented Jun 28, 2023

Metrics related to the server, especially connection and request, are not exposed as API. If these values are exposed, users will be able to implement more sophisticated graceful shutdowns such as stopping the server when there are no pending requests.

public class ServerMetrics {

    public int pendingRequests();
 
    public int activeRequests();

    public int activeConnections();
    ....
}

Server server = ...;
ServerMetrics metrics = server.serverMetrics();

Pending requests can increment the headers when they are received.

armeria/core/src/main/java/com/linecorp/armeria/server/Http2RequestDecoder.java

Lines 102 to 103 in 145bc0c

public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers nettyHeaders, int padding,
boolean endOfStream) throws Http2Exception {

The value will be decreased when they are handed over to the service.

armeria/core/src/main/java/com/linecorp/armeria/server/HttpServerHandler.java

Lines 362 to 363 in 145bc0c

serviceResponse = service.serve(reqCtx, req);
} catch (Throwable cause) {

In the case of ExchangeType.UNARY or ExchangeType.RESPONSE_STREAMING, buffering is performed until the entire body is received, so it will be counted as pending requests until END_OF_STREAM is received.
@ikhoon ikhoon added this to the 1.25.0 milestone Jun 28, 2023
@seonWKim
Copy link
Contributor

seonWKim commented Jul 1, 2023 •
edited

Hi @ikhoon, thank you for the detailed explanation.
I wonder if this feature should consider Http1RequestDecorder as well 🤔 . To my understanding, armeria collects all the request related objects if the request is an aggregating request(referenced the code below)

Http1RequestDocoder.java

// An aggregating request will be fired after all objects are collected.
if (!req.needsAggregation()) {
    ctx.fireChannelRead(req);
}

So if http1 request is a request that requires aggregation and has not collected all the objects, can we consider the state of the request as pending ? And if we can, should we consider http1 while implementing this feature ?

@ikhoon
Copy link
Contributor Author

ikhoon commented Jul 1, 2023

Right. The link to Http2RequestDecoder was an example. We should consider both HTTP/1 and HTTP/2.

@seonWKim
Copy link
Contributor

seonWKim commented Jul 1, 2023

@ikhoon ! Thank you for the confirmation. May I work on this issue ?

@jrhee17 jrhee17 modified the milestones: 1.25.0, 1.26.0 Jul 18, 2023
@minwoox minwoox modified the milestones: 1.26.0, 1.27.0 Oct 11, 2023
@ikhoon ikhoon modified the milestones: 1.27.0, 1.28.0 Jan 5, 2024
@trustin trustin changed the title Add ServerMetrics Expose the server-side metrics programmatically Mar 11, 2024
@jrhee17 jrhee17 modified the milestones: 1.28.0, 1.29.0 Mar 19, 2024
@minwoox
Copy link
Member

minwoox commented Mar 27, 2024

activeConnections:

armeria/core/src/main/java/com/linecorp/armeria/server/ConnectionLimitingHandler.java

Line 50 in 9061e24

private final AtomicInteger numConnections = new AtomicInteger();

@minwoox
Copy link
Member

minwoox commented Mar 28, 2024

Sorry @seonwoo960000, this issue has been assigned to a participant for the internal sprint that you've experienced. 😆

@seonWKim
Copy link
Contributor

seonWKim commented Mar 28, 2024 •
edited

@minwoox that's a great news. Hope to see a new contributor is added to Armeria 😄

@jrhee17 jrhee17 added the sprint Issues for OSS Sprint participants label Apr 1, 2024
@seongbeenkim seongbeenkim linked a pull request Apr 22, 2024 that will close this issue
Open
seongbeenkim added a commit to seongbeenkim/armeria that referenced this issue Apr 26, 2024
@seongbeenkim
Merge branch 'main' of https://github.com/seongbeenkim/armeria into f…
cfc9d80 
…eature/line#4992/metric
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
new feature sprint Issues for OSS Sprint participants
Projects
None yet
Milestone
1.29.0
Development

Successfully merging a pull request may close this issue.

Add ServerMetrics to collect metrics related server seongbeenkim/armeria
4 participants
@ikhoon @jrhee17 @minwoox @seonWKim