-
Notifications
You must be signed in to change notification settings - Fork 36
Get detector #207
Get detector #207
Changes from all commits
ed28a5f
dddc3bb
0746fec
9a43f92
8086177
6964ea0
1c790a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
/* | ||
* Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* Licensed under the Apache License, Version 2.0 (the "License"). | ||
* You may not use this file except in compliance with the License. | ||
* A copy of the License is located at | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* or in the "license" file accompanying this file. This file is distributed | ||
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
* express or implied. See the License for the specific language governing | ||
* permissions and limitations under the License. | ||
*/ | ||
|
||
package cmd | ||
|
||
import ( | ||
"encoding/json" | ||
entity "esad/internal/entity/ad" | ||
"esad/internal/handler/ad" | ||
"fmt" | ||
"github.com/spf13/cobra" | ||
) | ||
|
||
const ( | ||
commandCat = "cat" | ||
) | ||
|
||
//catCmd prints detectors configuration based on id and name pattern. | ||
var catCmd = &cobra.Command{ | ||
Use: commandCat + " [flags] [list of detectors]", | ||
Short: "Concatenate and print detectors based on id or name pattern", | ||
Long: `concatenate and print detectors based on pattern, use "" to make sure the name is not matched with pwd lists'`, | ||
Run: func(cmd *cobra.Command, args []string) { | ||
//If no args, display usage | ||
if len(args) < 1 { | ||
if err := cmd.Usage(); err != nil { | ||
fmt.Println(err) | ||
} | ||
return | ||
} | ||
idStatus, _ := cmd.Flags().GetBool("id") | ||
commandHandler, err := getCommandHandler() | ||
if err != nil { | ||
fmt.Println(err) | ||
} | ||
// default is name | ||
action := ad.GetAnomalyDetectorsByNamePattern | ||
if idStatus { | ||
action = getDetectorsByID | ||
} | ||
results, err := getDetectors(commandHandler, args, action) | ||
if err != nil { | ||
fmt.Println(err) | ||
return | ||
} | ||
printDetectors(results) | ||
}, | ||
} | ||
|
||
func getDetectors( | ||
commandHandler *ad.Handler, args []string, get func(*ad.Handler, string) ( | ||
[]*entity.DetectorOutput, error)) ([]*entity.DetectorOutput, error) { | ||
var results []*entity.DetectorOutput | ||
for _, detector := range args { | ||
output, err := get(commandHandler, detector) | ||
if err != nil { | ||
return nil, err | ||
} | ||
results = append(results, output...) | ||
} | ||
return results, nil | ||
} | ||
|
||
//getDetectorsByID gets detector output based on ID as argument | ||
func getDetectorsByID(commandHandler *ad.Handler, ID string) ([]*entity.DetectorOutput, error) { | ||
|
||
output, err := ad.GetAnomalyDetectorByID(commandHandler, ID) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return []*entity.DetectorOutput{output}, nil | ||
} | ||
|
||
//printDetectors displays the list of output. Since this is json format, use indent function to | ||
// pretty print before printing on console | ||
func printDetectors(results []*entity.DetectorOutput) { | ||
if results == nil { | ||
return | ||
} | ||
for _, d := range results { | ||
formattedOutput, err := json.MarshalIndent(d, "", " ") | ||
if err != nil { | ||
fmt.Println(err) | ||
return | ||
} | ||
fmt.Println(string(formattedOutput)) | ||
} | ||
} | ||
|
||
func init() { | ||
esadCmd.AddCommand(catCmd) | ||
catCmd.Flags().BoolP("name", "", true, "Input is name or pattern") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The message "Input is name or pattern" is visible to user? Have you confirmed the wording with tech writer? I feel we should say There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed offline, i will fix this accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, will approve this PR. Please fix this in another PR. |
||
catCmd.Flags().BoolP("id", "", false, "Input is id") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,12 +35,14 @@ type Controller interface { | |
StartDetector(context.Context, string) error | ||
StopDetector(context.Context, string) error | ||
DeleteDetector(context.Context, string, bool, bool) error | ||
GetDetector(context.Context, string) (*entity.DetectorOutput, error) | ||
CreateAnomalyDetector(context.Context, entity.CreateDetectorRequest) (*string, error) | ||
CreateMultiEntityAnomalyDetector(ctx context.Context, request entity.CreateDetectorRequest, interactive bool, display bool) ([]string, error) | ||
SearchDetectorByName(context.Context, string) ([]entity.Detector, error) | ||
StartDetectorByName(context.Context, string, bool) error | ||
StopDetectorByName(context.Context, string, bool) error | ||
DeleteDetectorByName(context.Context, string, bool, bool) error | ||
GetDetectorsByName(context.Context, string, bool) ([]*entity.DetectorOutput, error) | ||
} | ||
|
||
type controller struct { | ||
|
@@ -160,6 +162,24 @@ func (c controller) DeleteDetector(ctx context.Context, id string, interactive b | |
} | ||
return nil | ||
} | ||
|
||
//GetDetector fetch detector based on DetectorID | ||
func (c controller) GetDetector(ctx context.Context, ID string) (*entity.DetectorOutput, error) { | ||
if len(ID) < 1 { | ||
return nil, fmt.Errorf("detector Id: %s cannot be empty", ID) | ||
} | ||
response, err := c.gateway.GetDetector(ctx, ID) | ||
if err != nil { | ||
return nil, err | ||
} | ||
var data entity.DetectorResponse | ||
err = json.Unmarshal(response, &data) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return mapper.MapToDetectorOutput(data) | ||
} | ||
|
||
func processEntityError(err error) error { | ||
var c entity.CreateError | ||
data := fmt.Sprintf("%v", err) | ||
|
@@ -467,5 +487,35 @@ func (c controller) DeleteDetectorByName(ctx context.Context, name string, force | |
} | ||
} | ||
return nil | ||
} | ||
|
||
//GetDetectorsByName get detector based on name pattern. It first calls SearchDetectorByName and then | ||
// gets lists of detectorId and call GetDetector to get individual detector configuration | ||
func (c controller) GetDetectorsByName(ctx context.Context, pattern string, display bool) ([]*entity.DetectorOutput, error) { | ||
matchedDetectors, err := c.getDetectorsToProcess(ctx, "fetch", pattern) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What use cases have you tested for the name pattern?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I followed this from documentation: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you do some test for different regex use cases? Like "test.*", "test[0-9]" etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will create an issue to update search detecter using regex and will submit new PR with test cases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
if err != nil { | ||
return nil, err | ||
} | ||
if matchedDetectors == nil { | ||
return nil, nil | ||
} | ||
var bar *pb.ProgressBar | ||
if display { | ||
bar = createProgressBar(len(matchedDetectors)) | ||
} | ||
var output []*entity.DetectorOutput | ||
for _, detector := range matchedDetectors { | ||
data, err := c.GetDetector(ctx, detector.ID) | ||
if err != nil { | ||
return nil, err | ||
} | ||
output = append(output, data) | ||
if bar != nil { | ||
bar.Increment() | ||
} | ||
} | ||
if bar != nil { | ||
bar.Finish() | ||
} | ||
return output, nil | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
{ | ||
"_id" : "detectorID", | ||
"_version" : 1, | ||
"_primary_term" : 1, | ||
"_seq_no" : 3, | ||
"anomaly_detector" : { | ||
"name" : "detector", | ||
"description" : "Test detector", | ||
"time_field" : "timestamp", | ||
"indices" : [ | ||
"order*" | ||
], | ||
"filter_query" : {"bool" : {"filter" : [{"exists" : {"field" : "value","boost" : 1.0}}],"adjust_pure_negative" : true,"boost" : 1.0}}, | ||
"detection_interval" : { | ||
"period" : { | ||
"interval" : 5, | ||
"unit" : "Minutes" | ||
} | ||
}, | ||
"window_delay" : { | ||
"period" : { | ||
"interval" : 1, | ||
"unit" : "Minutes" | ||
} | ||
}, | ||
"schema_version" : 0, | ||
"feature_attributes" : [ | ||
{ | ||
"feature_id" : "mYccEnIBTXsGi3mvMd8_", | ||
"feature_name" : "total_order", | ||
"feature_enabled" : true, | ||
"aggregation_query" : {"total_order":{"sum":{"field":"value"}}} | ||
} | ||
], | ||
"last_update_time" : 1589441737319 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it best practice to use single-letter variable name in Go? How about replace
d
withdetector
? That will be easier to read the code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/golang/go/wiki/CodeReviewComments#variable-names
The convention is you only need more descriptive if it is used far away from declaration. Especially for iterators, single variable name is preferred.