Skip to content

Commit

Permalink
addressed pr comments and refactored the code
Browse files Browse the repository at this point in the history 
WAN-2321 #time 30m
  • Loading branch information
@shriyanshk128T
shriyanshk128T committed Sep 7, 2023
1 parent 99fe1d3 commit a0629fe
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 69 deletions.
62 changes: 32 additions & 30 deletions plugins/inputs/t128_graphql/t128_graphql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net"
Expand Down Expand Up @@ -142,37 +141,39 @@ func (plugin *T128GraphQL) checkConfig() error {

// Gather takes in an accumulator and adds the metrics that the Input gathers
func (plugin *T128GraphQL) Gather(acc telegraf.Accumulator) error {

processedResponses := plugin.MakeRequest(acc)
if processedResponses != nil {
for _, processedResponse := range processedResponses {
acc.AddFields(
plugin.CollectorName,
processedResponse.Fields,
processedResponse.Tags,
)
}
if !plugin.RetryIfNotFound && plugin.endpointNotFound {
return nil
}
processedResponses, err := plugin.MakeRequest()
if err != nil && processedResponses == nil {
acc.AddError(err)
return nil
} else if err != nil && processedResponses != nil {
acc.AddError(err)
}
for _, processedResponse := range processedResponses {
acc.AddFields(
plugin.CollectorName,
processedResponse.Fields,
processedResponse.Tags,
)
}

return nil
}

// MakeRequest can be used by other plugins using GraphQL functionality
func (plugin *T128GraphQL) MakeRequest(acc telegraf.Accumulator) []*ProcessedResponse {
if !plugin.RetryIfNotFound && plugin.endpointNotFound {
return nil
}
func (plugin *T128GraphQL) MakeRequest() ([]*ProcessedResponse, error) {
var errstrings []string

request, err := plugin.createRequest()
if err != nil {
acc.AddError(fmt.Errorf("failed to create a request for query %s: %w", plugin.Query, err))
return nil
return nil, fmt.Errorf("failed to create a request for query %s: %w", plugin.Query, err)
}

response, err := plugin.client.Do(request)
if err != nil {
acc.AddError(fmt.Errorf("failed to make graphQL request for collector %s: %w", plugin.CollectorName, err))
return nil
return nil, fmt.Errorf("failed to make graphQL request for collector %s: %w", plugin.CollectorName, err)
}
defer response.Body.Close()

Expand All @@ -184,30 +185,29 @@ func (plugin *T128GraphQL) MakeRequest(acc telegraf.Accumulator) []*ProcessedRes
if response.StatusCode < 200 || response.StatusCode >= 300 {
template := fmt.Sprintf("status code %d not OK for collector ", response.StatusCode) + plugin.CollectorName + ": %s"
for _, err := range decodeAndReportJSONErrors(message, template) {
acc.AddError(err)
errstrings = append(errstrings, err.Error())
}
return nil
return nil, fmt.Errorf(strings.Join(errstrings, "\n"))
}

//decode json
jsonParsed, err := gabs.ParseJSON(message)
if err != nil {
acc.AddError(fmt.Errorf("invalid json response for collector %s: %w", plugin.CollectorName, err))
return nil
return nil, fmt.Errorf("invalid json response for collector %s: %w", plugin.CollectorName, err)
}

//look for other errors in response
exists := jsonParsed.Exists("errors")
if exists {
template := fmt.Sprintf("found errors in response for collector %s", plugin.CollectorName) + ": %s"
for _, err := range decodeAndReportJSONErrors(message, template) {
acc.AddError(err)
errstrings = append(errstrings, err.Error())

if strings.Contains(err.Error(), "returned a 404") {
plugin.endpointNotFound = true

if !plugin.RetryIfNotFound {
acc.AddError(errors.New("collector configured to not retry when endpoint not found (404), stopping queries"))
errstrings = append(errstrings, "collector configured to not retry when endpoint not found (404), stopping queries")
}
}
}
Expand All @@ -216,15 +216,17 @@ func (plugin *T128GraphQL) MakeRequest(acc telegraf.Accumulator) []*ProcessedRes
//look for empty response
dataExists := jsonParsed.Exists("data")
if !dataExists {
acc.AddError(fmt.Errorf("no data found in response for collector %s", plugin.CollectorName))
return nil
errorMessage := fmt.Errorf("no data found in response for collector %s", plugin.CollectorName)
errstrings = append(errstrings, errorMessage.Error())
return nil, fmt.Errorf(strings.Join(errstrings, ", "))
}
processedResponses, err := ProcessResponse(jsonParsed, plugin.CollectorName, plugin.Config.Fields, plugin.Config.Tags)
if err != nil {
acc.AddError(err)
return nil
return nil, err
} else if dataExists && exists {
return processedResponses, fmt.Errorf(strings.Join(errstrings, ", "))
} else {
return processedResponses
return processedResponses, nil
}
}

Expand Down
13 changes: 4 additions & 9 deletions plugins/inputs/t128_graphql/t128_graphql_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ var CollectorTestCases = []struct {
}`},
ExpectedMetrics: nil,
ExpectedErrors: []string{
"found errors in response for collector test-collector: Cannot query field \"invalid-field\" on type \"ArpEntryType\".",
"no data found in response for collector test-collector",
"found errors in response for collector test-collector: Cannot query field \"invalid-field\" on type \"ArpEntryType\"., no data found in response for collector test-collector",
},
ExpectedRequests: []int{1},
},
Expand All @@ -158,10 +157,8 @@ var CollectorTestCases = []struct {
}`},
ExpectedMetrics: nil,
ExpectedErrors: []string{
"found errors in response for collector test-collector: highwayManager@CHSSDWCond01CHI.CHSSDWCondMD returned a 404",
"no data found in response for collector test-collector",
"found errors in response for collector test-collector: highwayManager@CHSSDWCond01CHI.CHSSDWCondMD returned a 404",
"no data found in response for collector test-collector",
"found errors in response for collector test-collector: highwayManager@CHSSDWCond01CHI.CHSSDWCondMD returned a 404, no data found in response for collector test-collector",
"found errors in response for collector test-collector: highwayManager@CHSSDWCond01CHI.CHSSDWCondMD returned a 404, no data found in response for collector test-collector",
},
RetryIfNotFound: true,
ExpectedRequests: []int{1, 2},
Expand All @@ -181,9 +178,7 @@ var CollectorTestCases = []struct {
}`},
ExpectedMetrics: nil,
ExpectedErrors: []string{
"found errors in response for collector test-collector: highwayManager@CHSSDWCond01CHI.CHSSDWCondMD returned a 404",
"no data found in response for collector test-collector",
"collector configured to not retry when endpoint not found (404), stopping queries",
"found errors in response for collector test-collector: highwayManager@CHSSDWCond01CHI.CHSSDWCondMD returned a 404, collector configured to not retry when endpoint not found (404), stopping queries, no data found in response for collector test-collector",
},
RetryIfNotFound: false,
ExpectedRequests: []int{1, 1},
Expand Down
60 changes: 40 additions & 20 deletions plugins/inputs/t128_peer_path/t128_peer_path.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ var peerPathFields = map[string]string{"status": "status", "enabled": "enabled"}
var peerPathTags = map[string]string{
"node": "node",
"adjacentAddress": "adjacentAddress",
"adjacentHostname": "adjacentHostname",
"deviceInterface": "deviceInterface",
"networkInterface": "networkInterface",
"vlan": "vlan",
"routerName": "allPeers/nodes/routerName",
}
var peerPathQuery = "query {\nallPeers{\nnodes{\npaths{\nadjacentAddress\nadjacentHostname\ndeviceInterface\nenabled\nnetworkInterface\nnode\nstatus\nvlan}\nrouterName}}}"

var peerPathHostnameQuery = "query {\nallPeers{\nnodes{\npaths{\nadjacentAddress\nadjacentHostname\ndeviceInterface\nenabled\nnetworkInterface\nnode\nstatus\nvlan}\nrouterName}}}"

var peerPathQuery = "query {\nallPeers{\nnodes{\npaths{\nadjacentAddress\ndeviceInterface\nenabled\nnetworkInterface\nnode\nstatus\nvlan}\nrouterName}}}"

var sampleConfig = `
[[inputs.t128_peer_path]]
Expand Down Expand Up @@ -74,6 +76,17 @@ func (*T128PeerPath) Description() string {
// Init sets up the input to be ready for action
func (plugin *T128PeerPath) Init() error {

var query string
if plugin.ExcludeHostname {
if _, found := peerPathTags["adjacentHostname"]; found {
delete(peerPathTags, "adjacentHostname")
}
} else {
if _, found := peerPathTags["adjacentHostname"]; !found {
peerPathTags["adjacentHostname"] = "adjacentHostname"
}
query = peerPathHostnameQuery
}
plugin.gqlCollector = &t128_graphql.T128GraphQL{
CollectorName: plugin.CollectorName,
BaseURL: plugin.BaseURL,
Expand All @@ -83,7 +96,7 @@ func (plugin *T128PeerPath) Init() error {
Tags: peerPathTags,
Timeout: plugin.Timeout,
RetryIfNotFound: plugin.RetryIfNotFound,
Query: peerPathQuery,
Query: query,
}
err := plugin.gqlCollector.Init()
if err != nil {
Expand All @@ -94,25 +107,32 @@ func (plugin *T128PeerPath) Init() error {

// Gather takes in an accumulator and adds the metrics that the Input gathers
func (plugin *T128PeerPath) Gather(acc telegraf.Accumulator) error {
processedResponses := plugin.gqlCollector.MakeRequest(acc)
if processedResponses != nil {
for _, processedResponse := range processedResponses {
for k, v := range processedResponse.Tags {
if k == "adjacentAddress" && v == "127.117.97.105" {
delete(processedResponse.Tags, "adjacentAddress")
continue
}
if (k == "adjacentHostname" && v == "") || (plugin.ExcludeHostname) {
delete(processedResponse.Tags, "adjacentHostname")
continue
}
if !plugin.RetryIfNotFound && plugin.endpointNotFound {
return nil
}
processedResponses, err := plugin.gqlCollector.MakeRequest()
if err != nil && processedResponses == nil {
acc.AddError(err)
return nil
} else if err != nil && processedResponses != nil {
acc.AddError(err)
}
for _, processedResponse := range processedResponses {
for k, v := range processedResponse.Tags {
if k == "adjacentAddress" && v == "127.117.97.105" {
delete(processedResponse.Tags, "adjacentAddress")
continue
}
if k == "adjacentHostname" && v == "" {
delete(processedResponse.Tags, "adjacentHostname")
continue
}
acc.AddFields(
plugin.CollectorName,
processedResponse.Fields,
processedResponse.Tags,
)
}
acc.AddFields(
plugin.CollectorName,
processedResponse.Fields,
processedResponse.Tags,
)
}

return nil
Expand Down
21 changes: 11 additions & 10 deletions plugins/inputs/t128_peer_path/t128_peer_path_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ type Endpoint struct {
}

const (
ValidPeerPathRequest = `{"query":"query {\nallPeers{\nnodes{\npaths{\nadjacentAddress\nadjacentHostname\ndeviceInterface\nenabled\nnetworkInterface\nnode\nstatus\nvlan}\nrouterName}}}"}`
ValidPeerPathRequestWithHostName = `{"query":"query {\nallPeers{\nnodes{\npaths{\nadjacentAddress\nadjacentHostname\ndeviceInterface\nenabled\nnetworkInterface\nnode\nstatus\nvlan}\nrouterName}}}"}`
ValidPeerPathRequestWithoutHostName = `{"query":"query {\nallPeers{\nnodes{\npaths{\nadjacentAddress\ndeviceInterface\nenabled\nnetworkInterface\nnode\nstatus\nvlan}\nrouterName}}}"}`
)

var CollectorTestCases = []struct {
Expand All @@ -34,9 +35,9 @@ var CollectorTestCases = []struct {
ExpectedRequests []int
}{
{
Name: "exclude hostname set to true",
ExcludeHostname: true,
Endpoint: Endpoint{"/api/v1/graphql/", 200, ValidPeerPathRequest, `{
Name: "empty adjacent hostname",
ExcludeHostname: false,
Endpoint: Endpoint{"/api/v1/graphql/", 200, ValidPeerPathRequestWithHostName, `{
"data": {
"allPeers": {
"nodes": [
Expand All @@ -46,7 +47,7 @@ var CollectorTestCases = []struct {
{
"node": "node1",
"adjacentAddress": "10.10.10.10",
"adjacentHostname": "fake-peer",
"adjacentHostname": null,
"status": "UP",
"enabled": true,
"deviceInterface": "wan5",
Expand Down Expand Up @@ -80,9 +81,9 @@ var CollectorTestCases = []struct {
ExpectedRequests: []int{1},
},
{
Name: "empty adjacent hostname",
ExcludeHostname: false,
Endpoint: Endpoint{"/api/v1/graphql/", 200, ValidPeerPathRequest, `{
Name: "exclude hostname set to true",
ExcludeHostname: true,
Endpoint: Endpoint{"/api/v1/graphql/", 200, ValidPeerPathRequestWithoutHostName, `{
"data": {
"allPeers": {
"nodes": [
Expand All @@ -92,7 +93,7 @@ var CollectorTestCases = []struct {
{
"node": "node1",
"adjacentAddress": "10.10.10.10",
"adjacentHostname": null,
"adjacentHostname": "fake-peer",
"status": "UP",
"enabled": true,
"deviceInterface": "wan5",
Expand Down Expand Up @@ -128,7 +129,7 @@ var CollectorTestCases = []struct {
{
Name: "multiple paths",
ExcludeHostname: false,
Endpoint: Endpoint{"/api/v1/graphql/", 200, ValidPeerPathRequest, `{
Endpoint: Endpoint{"/api/v1/graphql/", 200, ValidPeerPathRequestWithHostName, `{
"data": {
"allPeers": {
"nodes": [
Expand Down

0 comments on commit a0629fe

Please sign in to comment.