Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Get detector #207

Merged
merged 7 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 103 additions & 0 deletions cli/cmd/cat.go
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 {
Copy link
Contributor

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 with detector ? That will be easier to read the code.

Copy link
Member Author

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.

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")
Copy link
Contributor

Choose a reason for hiding this comment

The 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 Input is detector name or name pattern, Input is detector id

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, i will fix this accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

The 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")
}
50 changes: 50 additions & 0 deletions cli/internal/controller/ad/ad.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What use cases have you tested for the name pattern? SearchDetectorByName method is using match query. Why not use regexp ?

	payload := entity.SearchRequest{
		Query: entity.SearchQuery{
			Match: entity.Match{
				Name: name,
			},
		},
	}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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
}
86 changes: 86 additions & 0 deletions cli/internal/controller/ad/ad_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -724,3 +724,89 @@ func TestController_DeleteDetectorByName(t *testing.T) {
assert.NoError(t, err)
})
}

func TestController_GetDetectorByName(t *testing.T) {
detectorOutput := &entity.DetectorOutput{
ID: "detectorID",
Name: "detector",
Description: "Test detector",
TimeField: "timestamp",
Index: []string{"order*"},
Features: []entity.Feature{
{
Name: "total_order",
Enabled: true,
AggregationQuery: []byte(`{"total_order":{"sum":{"field":"value"}}}`),
},
},
Filter: []byte(`{"bool" : {"filter" : [{"exists" : {"field" : "value","boost" : 1.0}}],"adjust_pure_negative" : true,"boost" : 1.0}}`),
Interval: "5m",
Delay: "1m",
LastUpdatedAt: 1589441737319,
SchemaVersion: 0,
}
t.Run("get empty detector", func(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mockADGateway := adgateway.NewMockGateway(mockCtrl)
mockESController := esmockctrl.NewMockController(mockCtrl)
ctx := context.Background()
ctrl := New(os.Stdin, mockESController, mockADGateway)
_, err := ctrl.GetDetectorsByName(ctx, "", false)
assert.Error(t, err)
})
t.Run("search detector gateway failed", func(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
ctx := context.Background()
mockADGateway := adgateway.NewMockGateway(mockCtrl)
mockADGateway.EXPECT().SearchDetector(ctx, getSearchPayload("detector")).Return(nil, errors.New("gateway failed"))
mockESController := esmockctrl.NewMockController(mockCtrl)
ctrl := New(os.Stdin, mockESController, mockADGateway)
_, err := ctrl.GetDetectorsByName(ctx, "detector", false)
assert.EqualError(t, err, "gateway failed")
})
t.Run("search detector gateway returned empty", func(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
ctx := context.Background()
mockADGateway := adgateway.NewMockGateway(mockCtrl)
mockADGateway.EXPECT().SearchDetector(ctx, getSearchPayload("detector")).Return([]byte(`{}`), nil)
mockESController := esmockctrl.NewMockController(mockCtrl)
ctrl := New(os.Stdin, mockESController, mockADGateway)
actual, err := ctrl.GetDetectorsByName(ctx, "detector", false)
assert.NoError(t, err)
assert.Nil(t, actual)
})
t.Run("get detector gateway failed", func(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
ctx := context.Background()
mockADGateway := adgateway.NewMockGateway(mockCtrl)
mockADGateway.EXPECT().SearchDetector(ctx, getSearchPayload("detector")).Return(
helperLoadBytes(t, "search_response.json"), nil)
mockADGateway.EXPECT().GetDetector(ctx, "detectorID").Return(nil, errors.New("gateway failed"))
var stdin bytes.Buffer
stdin.Write([]byte("yes\n"))
mockESController := esmockctrl.NewMockController(mockCtrl)
ctrl := New(&stdin, mockESController, mockADGateway)
_, err := ctrl.GetDetectorsByName(ctx, "detector", false)
assert.EqualError(t, err, "gateway failed")
})
t.Run("get detector", func(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
ctx := context.Background()
mockADGateway := adgateway.NewMockGateway(mockCtrl)
mockADGateway.EXPECT().SearchDetector(ctx, getSearchPayload("detector")).Return(
helperLoadBytes(t, "search_response.json"), nil)
mockADGateway.EXPECT().GetDetector(ctx, "detectorID").Return(helperLoadBytes(t, "get_response.json"), nil)
mockESController := esmockctrl.NewMockController(mockCtrl)
var stdin bytes.Buffer
stdin.Write([]byte("yes\n"))
ctrl := New(&stdin, mockESController, mockADGateway)
res, err := ctrl.GetDetectorsByName(ctx, "detector", false)
assert.NoError(t, err)
assert.EqualValues(t, *res[0], *detectorOutput)
})
}
30 changes: 30 additions & 0 deletions cli/internal/controller/ad/mocks/mock_ad.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions cli/internal/controller/ad/testdata/get_response.json
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
}
}
29 changes: 29 additions & 0 deletions cli/internal/entity/ad/ad.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,32 @@ type Container struct {
type SearchResponse struct {
Hits Container `json:"hits"`
}

type Metadata CreateDetector

type AnomalyDetector struct {
Metadata
SchemaVersion int32 `json:"schema_version"`
LastUpdateTime uint64 `json:"last_update_time"`
}

//DetectorResponse represents detector's setting
type DetectorResponse struct {
ID string `json:"_id"`
AnomalyDetector AnomalyDetector `json:"anomaly_detector"`
}

//DetectorOutput represents detector's setting displayed to user
type DetectorOutput struct {
ID string
kaituo marked this conversation as resolved.
Show resolved Hide resolved
Name string `json:"name"`
Description string `json:"description"`
TimeField string `json:"time_field"`
Index []string `json:"indices"`
Features []Feature `json:"features"`
Filter json.RawMessage `json:"filter_query"`
Interval string `json:"detection_interval"`
Delay string `json:"window_delay"`
LastUpdatedAt uint64 `json:"last_update_time"`
SchemaVersion int32 `json:"schema_version"`
}
Loading