Repository navigation
QueryCountStatsMonitor: emit query/count - #6473
Conversation
| Map<String, Long> diff = keyedDiff.to( | ||
| "queryCountStats", | ||
| ImmutableMap.of("query/success/count", statsProvider.getSuccessfulQueryCount(), | ||
| ImmutableMap.of("query/total/count", statsProvider.getSuccessfulQueryCount() + statsProvider.getFailedQueryCount(), |
There was a problem hiding this comment.
I think this should include interrupted too. It looks like "failed" and "interrupted" are exclusive.
For consistency I'd also suggest retrieving the successful/failed/interrupted counts first, then summing them into the total. This is because the counts are always getting updated in the background, and if you call getSuccessfulQueryCount() more than once, the two won't necessarily be the same.
There was a problem hiding this comment.
I think metric names should add slashes when some different options are imaginable after the slash. With this, I think just query/count or query/count/total (with another option of query/count/delta) is better
leventov
changed the title
| ImmutableMap.of("query/success/count", statsProvider.getSuccessfulQueryCount(), | ||
| "query/failed/count", statsProvider.getFailedQueryCount(), | ||
| "query/interrupted/count", statsProvider.getInterruptedQueryCount() | ||
| ImmutableMap.of("query/count", successfulQueryCount + failedQueryCount + interruptedQueryCount, |
There was a problem hiding this comment.
Should break "query/count", successfulQueryCount + failedQueryCount + interruptedQueryCount, on the next line, as well as the following lines
| "query/interrupted/count", interruptedQueryCount | ||
|
|
||
| ImmutableMap.of( | ||
| "query/count", |
There was a problem hiding this comment.
You could keep corresponding keys and values on the same line, e. g.
ImmutableMap.of(
key1, value1,
key2, value2,
...
)I think it's more readable and recognised as an exception from the general rule here: https://github.com/apache/incubator-druid/pull/6461/files#diff-b26be0568e2252c7501570f97d7a91e8R212
There was a problem hiding this comment.
you mean like below?
Map<String, Long> diff = keyedDiff.to(
"queryCountStats",
ImmutableMap.of(
"query/count", successfulQueryCount + failedQueryCount + interruptedQueryCount,
"query/success/count", successfulQueryCount,
"query/failed/count", failedQueryCount,
"query/interrupted/count", interruptedQueryCount
)
);
Let `QueryCountStatsMonitor` emit `query/count`, then I can monitor QPS of my services, or I have to count it by myself.
Let
QueryCountStatsMonitoremitquery/count, then I can monitor QPS of my services, or I have to count it by myself.