seongsu shin created HTTPCLIENT-2438:
----------------------------------------
Summary: AsyncIoByteCounterExec keeps tags from previous requests
Key: HTTPCLIENT-2438
URL: https://issues.apache.org/jira/browse/HTTPCLIENT-2438
Project: HttpComponents HttpClient
Issue Type: Bug
Affects Versions: 5.6.5, 5.6.4, 5.6.3
Reporter: seongsu shin
h2. Summary
With the IO metric set on the async client, the request.bytes / response.bytes
counters can get tags from an earlier request. If one request gets a tag that
the others don't have, all requests after it are counted with that tag too. The
classic client is fine.
h2. Cause
AsyncIoByteCounterExec creates the two Counter.Builder instances once in the
constructor and keeps them in fields. record() then does this for every request:
{code:java}
reqBuilder.tags(tags).tags(mc.commonTags).register(meterRegistry).increment(reqBytes);
{code}
Counter.Builder.tags() adds to the tags the builder already has, it doesn't
replace them. method and status are on every request so they just get
overwritten, but any other tag stays on the builder and is used for every
request after that.
IoByteCounterExec creates a new builder on each call, and there is a comment
there saying it's done to avoid tag accumulation, so I guess the async one was
just missed.
h2. Why it matters
If you use a tagCustomizer that adds a tag only for some requests (tenant,
endpoint name, etc.), the byte counters for that tag end up including traffic
that has nothing to do with it. Nothing fails or gets logged, so the numbers
just look normal but are wrong.
Also the same builder is shared across all requests of the client and
Counter.Builder isn't thread safe, so concurrent requests could mix their tags
as well. I didn't try to reproduce that part.
h2. How to reproduce
Async client, IO metric set, and a tagCustomizer that adds a tag only for /a:
{code:java}
ObservingOptions opts = ObservingOptions.builder()
.metrics(EnumSet.of(ObservingOptions.MetricSet.IO))
.tagCustomizer((tags, method, status, protocol, target, uri) -> {
List<Tag> out = new ArrayList<>(tags);
if (uri.startsWith("/a")) {
out.add(Tag.of("only_for_a", "yes"));
}
return out;
})
.build();
HttpClientObservationSupport.enable(builder, null, meterRegistry, opts,
MetricConfig.DEFAULT);
{code}
Send GET /a (200), then GET /b (404), and look at the response.bytes counters.
Did the same with the classic client to compare.
h2. Observed
{noformat}
async:
total=4 [method=GET, only_for_a=yes, status=200]
total=2 [method=GET, only_for_a=yes, status=404]
classic:
total=4 [method=GET, only_for_a=yes, status=200]
total=2 [method=GET, status=404]
{noformat}
/b has only_for_a=yes on the async client.
h2. Expected
/b should only have its own tags, same as classic.
h2. Notes
Creating the builders inside record() like IoByteCounterExec does should fix
it. I can open a PR with a test if that sounds ok.
PR #920 also changes this class (uri tag) but not this part.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
