suppport HC detector in profile api #274
Conversation
Codecov Report
@@ Coverage Diff @@
## master #274 +/- ##
============================================
- Coverage 73.22% 71.24% -1.98%
- Complexity 1749 1760 +11
============================================
Files 180 185 +5
Lines 8119 8430 +311
Branches 670 719 +49
============================================
+ Hits 5945 6006 +61
- Misses 1864 2088 +224
- Partials 310 336 +26
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -140,19 +158,67 @@ private void prepareProfile( | |||
|
|
|||
| if (profilesToCollect.contains(ProfileName.ERROR)) { | |||
| GetRequest getStateRequest = new GetRequest(DetectorInternalState.DETECTOR_STATE_INDEX, detectorId); | |||
| client.get(getStateRequest, onGetDetectorState(listener, detectorId, enabledTimeMs)); | |||
| client.get(getStateRequest, onGetDetectorState(listener, detectorId, enabledTimeMs));// +1 | |||
| AnomalyDetector detector = AnomalyDetector.parse(parser, detectorId); | ||
| List<String> categoryField = detector.getCategoryField(); | ||
| if (categoryField == null || categoryField.size() != 1) { | ||
| listener.onResponse(new EntityProfile(categoryField.get(0), entityValue, false)); |
There was a problem hiding this comment.
I guess categoryField.get(0) will throw NPE if categoryField is null
There was a problem hiding this comment.
Good catch. will fix.
| @@ -23,7 +23,7 @@ | |||
| public class ProfileAction extends ActionType<ProfileResponse> { | |||
|
|
|||
| public static final ProfileAction INSTANCE = new ProfileAction(); | |||
| public static final String NAME = "cluster:admin/ad/detector/profile"; | |||
| public static final String NAME = "cluster:admin/opendistro/ad/detectors/profile"; | |||
There was a problem hiding this comment.
do we need also update this name for the FGAC changes? I remember we need have these names in the permission
There was a problem hiding this comment.
Good question. Sarat will address all transport action names later. External and internal transport name should follow different pattern. This action should be internal action.
| @@ -111,4 +111,8 @@ public void failImmediately(String errMsg, Exception e) { | |||
| public void respondImmediately(T o) { | |||
| this.delegate.onResponse(o); | |||
| } | |||
|
|
|||
| public void compareAndSetMaxResponseCount(int oldValue, int newValue) { | |||
| this.maxResponseCount.compareAndSet(oldValue, newValue); | |||
There was a problem hiding this comment.
how about the false result? do we need handle it?
There was a problem hiding this comment.
The client of this method need to take care of this.
There was a problem hiding this comment.
I dont think this will cause any big issue. but logically, should the result be returned to the client if they need take care of it?
| aggBuilder.field(categoryField.get(0)); | ||
| searchSourceBuilder.aggregation(aggBuilder); | ||
|
|
||
| SearchRequest request = new SearchRequest(detector.getIndices().toArray(new String[0]), searchSourceBuilder); |
There was a problem hiding this comment.
how is this detector index managed? Are there one/multi indices for each detector? I have this question because i don't see detector id in the search request.
| String nodeId = node.get().getId(); | ||
| DiscoveryNode localNode = clusterService.localNode(); | ||
| if (localNode.getId().equals(nodeId)) { | ||
| listener.onResponse(new EntityProfileResponse((cacheProvider.get().isActive(adID, modelId)))); |
There was a problem hiding this comment.
how about cache missing case?
Issue #, if available:
Description of changes:
Support high cardinality detector in profile API.
1.Total entity count
2.Active entity count
3.Profile entity status: active or not
Add job and detection state index in AD stats API
Did test by starting cluster locally for both single and HC detector.
Exclude some classes which coverage lower than our limitation. Will add more test case in another PR>
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.