[HWKMETRICS-180] findMetricsWithTags supports now MatchType (ANY / ALL)

[burmanm]

Implements logical AND for filtering with tags. Also adds a MatchType enum to be used, which has a following meaning:

• ANY = logical OR
• ALL = logical AND

[burmanm]

Do not merge yet - work in progress for tests and REST-API refactoring (and some reusability refactoring in the MetricsServiceImpl)

[burmanm]

Ready for review

[jsanda]

I can see that there is support for logical AND and OR for tag values, but what about tag names? It looks like an AND is applied implicitly. Do you think we should support OR as well? And possibly negation, NOT?

[jsanda]

With the changes for HWKMetrics-114, which are in master now, can we just MetricId instead of the MetricIndex class?

Ah, I see the comment in the javadoc to that effect :)

[burmanm]

OR for tag names and negation are possible options, just like the regexp-filtering of values. Syntax might on the other hand be a bit more problematic. The reason I set with implicit AND in this case is that the OR can be substituted by making several queries and just combining the results.

I'd rather do these changes on another PR to get these things going as the discussion on mailing list didn't really evolve beyond the "I wish we had a full SQL support". There's some syntactic issues with OR notation beyond the simple cases. Same would apply to NOT-notation, although not as bad. I guess options like the following are possibility.

• !tagName=* (no metric with tag called tagName)
• tagName=!value (no metrics which have tagName with value - issue, does it still have to have that tagName with some other value?)

But !tagName=value would not make sense. tagName=this|!notthis wouldn't also make sense as we could just ignore the |!notthis part.

[burmanm]

And yes, MetricIndex is worthless now that HWKMETRICS-114 is merged. I can do a rebase and remove that.

[jsanda]

OR for tag names and negation are possible options, just like the regexp-filtering of values. Syntax might on the other hand be a bit more problematic. The reason I set with implicit AND in this case is that the OR can be substituted by making several queries and just combining the results.

True, OR can be done my making multiple calls in parallel. That is, calls to MetricsService.findMetricsWithFilters(), which would not help with the REST API. I agree that the syntax might present some more challenges.

I'd rather do these changes on another PR to get these things going as the discussion on mailing list didn't really evolve beyond the "I wish we had a full SQL support".

Agreed.

[jsanda]

In the code that queries by tag name and value, a separate query is executed for each tag value. Why not instead do a single query by tag name and then filter client side by the tag values?

[jsanda]

In TagIndexResultSetTransformer do we need,

&& MetricType.userTypes().contains(MetricType.fromCode(r.getInt(0)))

in the filter call?

[jsanda]

Given the increase in functionality for searching by tags, it might be worthwhile to have a separate test class for tags functionality in core-impl. That can be discussed separately though.

Other than the questions/comments I have raised, it looks good!

[burmanm]

The tagvalues was done to reduce the amount of data transferred (if certain tag would have a lot of values), however for complex filtering this won't be an option anyway (for example with regexp-filtering).

So it would be possible to just fetch everything and filter then on the client side, small network hit and also less queries sent to the server (and simpler logic).

[burmanm]

The TagIndexResultSetTransformer does not want to return tags that are associated with non-user-querable types such as COUNTER_RATE (there could be more in the future). So we drop them at that point.

[jsanda]

The tagvalues was done to reduce the amount of data transferred (if certain tag would have a lot of values), however for complex filtering this won't be an option anyway (for example with regexp-filtering).

The amount of data within a single partition is a growing concern of mine as well. I created https://issues.jboss.org/browse/HWKMETRICS-196 to further discuss and address it.

[jsanda]

The TagIndexResultSetTransformer does not want to return tags that are associated with non-user-querable types...

I didn't think about that. Makes sense. Thanks!

[FilipB]

Please ignore check named "default". It is already disabled and will be not visible in next pull requests

[jsanda]

Could you add some test coverage for the regex functionality that you added in the last two commits?

[jsanda]

Thanks for the additional tests!