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

Metrics vertx_http_client_active_requests guage can't decrease zero #205

Closed
Bugxyb opened this issue Nov 9, 2023 · 9 comments
Closed

Metrics vertx_http_client_active_requests guage can't decrease zero #205

Bugxyb opened this issue Nov 9, 2023 · 9 comments
Assignees
Labels
invalid This doesn't seem right

Comments

@Bugxyb
Copy link

Bugxyb commented Nov 9, 2023

Version

Vert.x 4.4.6

Context

we use vert.x metrics to count http client active request, and we see the gauge value increase. I stop send traffic to server instance, when the active connections is zero, the vertx_http_client_active_requests guage is not zero.

Extra

image

  1. we scrape the vertx_http_client_active_request and vertx_http_client_active_connections.
  2. find the vertx_http_client_active_connections is around 900, but vertx_http_client_active_request continue to increase.
  3. we stop traffic when vertx_http_client_active_connections to zero, but vertx_http_client_active_request is equal 4873.

It seems these requests did not successfully call the metrics decrease.

Relate

we open the issue in eclipse-vertx/vert.x#4936

@Bugxyb Bugxyb added the bug Something isn't working label Nov 9, 2023
@tsegismont tsegismont self-assigned this Nov 9, 2023
@tsegismont tsegismont added this to the 4.5.0 milestone Nov 9, 2023
@tsegismont
Copy link
Contributor

Have you been able to create a small reproducer?

@Bugxyb
Copy link
Author

Bugxyb commented Nov 9, 2023

Have you been able to create a small reproducer?

it seems difficult to create a small reproduce, those data is provided by online service. but we use the http client to create the request.

    protected Future<WebClientResponse> execute(String url,
                                                WebClientEntity entity,
                                                boolean responseBody) {
        final long time = TIME.get();
        final Promise<WebClientResponse> promise = Promise.promise();
        final RequestOptions requestOptions = new RequestOptions()
                .setMethod(entity.httpMethod())
                .setTimeout(entity.connectTimeoutMs())
                .setAbsoluteURI(url);

        httpClient.request(requestOptions)
                .onComplete(ignored -> recordTimer(this.conncetTimer, time))
                .onSuccess(httpRequest -> request(promise, httpRequest, entity, responseBody))
                .onFailure(throwable -> handleConnectFailed(throwable, promise));

        return promise.future();
    }

    private void request(Promise<WebClientResponse> promise, HttpClientRequest request,
                         WebClientEntity entity, Boolean responseBody) {
        final long time = TIME.get();
        request.headers().addAll(entity.headers());
        request.setTimeout(entity.timeoutMs());
        final Future<HttpClientResponse> httpClientResponse;
        final TimerPromise<WebClientResponse> timerPromise;

        httpClientResponse = entity.withBody() ? request.send(entity.getBody()) : request.send();
        timerPromise = TimerPromise.promise(vertx, entity.timeoutMs(), request::reset);
        httpClientResponse
                .onSuccess(response -> handleResponse(timerPromise, response, responseBody))
                .onFailure(timerPromise::tryFail);
        timerPromise.future()
                .onComplete(ignored -> recordTimer(this.socketTimer, time))
                .onSuccess(promise::complete)
                .onFailure(throwable -> handleFailed(throwable, promise));
    }

timerPromise is timeout to cancel request(using request reset)

@tsegismont
Copy link
Contributor

I wrote this test:

  @Test
  public void shouldDecrementActiveRequestsWhenRequestIsReset(TestContext ctx) {
    vertx = vertx(ctx);
    int numRequests = 10;
    Async doneLatch = ctx.async(numRequests * 2);
    httpServer = vertx.createHttpServer()
      .requestHandler(req -> {
        req.pause();
        vertx.setTimer(500, l -> {
          req.end().onComplete(ctx.asyncAssertSuccess(v1 -> {
            req.response().end().onComplete(v2 -> {
              doneLatch.countDown();
            });
          }));
          req.resume();
        });
      });
    Async listenLatch = ctx.async();
    httpServer
      .listen(9195, "127.0.0.1")
      .onComplete(ctx.asyncAssertSuccess(s -> listenLatch.complete()));
    listenLatch.awaitSuccess(20_000);
    HttpClient client = vertx.createHttpClient();
    for (int i = 0; i < numRequests; i++) {
      client.request(HttpMethod.POST, 9195, "127.0.0.1", "/")
        .onComplete(ctx.asyncAssertSuccess(req -> {
          req
            .response()
            .compose(HttpClientResponse::body);
          req.end("chunk").onComplete(ctx.asyncAssertSuccess(v -> {
            vertx.setTimer(250, l-> {
              ctx.assertTrue(req.reset());
              doneLatch.countDown();
            });
          }));
        }));
    }
    doneLatch.awaitSuccess(20_000);
    List<Datapoint> res = listDatapoints(startsWith("vertx.http.client.active.requests"));
    assertThat(res).extracting(Datapoint::value).contains(0.0);
  }

Tried with both HTTP1 and HTTP2 and failed to reproduce

@tsegismont
Copy link
Contributor

Is it HTTP1 or HTTP2 you're working with?

@Bugxyb
Copy link
Author

Bugxyb commented Nov 11, 2023

Is it HTTP1 or HTTP2 you're working with?

use HTTP1

@tsegismont
Copy link
Contributor

tsegismont commented Nov 14, 2023

@Bugxyb if you can't create a reproducer, would you be able to create a byteman rule and check if there as many calls to io.vertx.core.spi.metrics.ClientMetrics#requestBegin as the sum of calls to io.vertx.core.spi.metrics.ClientMetrics#requestEnd, io.vertx.core.spi.metrics.ClientMetrics#requestReset and io.vertx.core.spi.metrics.ClientMetrics#responseBegin.

That would tell us if the problem comes from Vert.x core or if it's in Vert.x Micrometer.

@vietj vietj modified the milestones: 4.5.0, 4.5.1 Nov 15, 2023
@tsegismont
Copy link
Contributor

@Bugxyb any news about this?

@Bugxyb
Copy link
Author

Bugxyb commented Nov 23, 2023

@Bugxyb any news about this?

sry, I don't have time to deal it with byteman recently.

@vietj vietj modified the milestones: 4.5.1, 4.5.2 Dec 13, 2023
@tsegismont tsegismont added invalid This doesn't seem right and removed bug Something isn't working labels Jan 30, 2024
@tsegismont tsegismont removed this from the 4.5.2 milestone Jan 30, 2024
@tsegismont
Copy link
Contributor

Closing, please reopen in you can provide a reproducer

@tsegismont tsegismont closed this as not planned Won't fix, can't repro, duplicate, stale Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Development

No branches or pull requests

3 participants