Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add appdynamicscloud metric provider #1360

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
Specify more accurate error message
Signed-off-by: charleslin-appd <[email protected]>
  • Loading branch information
charleslin-appd committed Feb 12, 2023
commit c77d8ebefc9a7e9eaeb12d2c1abe39f7dabf930f
6 changes: 3 additions & 3 deletions pkg/metrics/providers/appdynamicscloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,18 @@ func (p *AppDynamicsCloudProvider) RunQuery(query string) (float64, error) {
return 0, fmt.Errorf("failed to un-marshaling result: %s.\n error: %w", string(body), err)
}
if len(anyValue) < 1 {
return 0, fmt.Errorf("invalid response: %s: %w", string(body), ErrNoValuesFound)
return 0, fmt.Errorf("invalid response for query: %s: %w", query, ErrNoValuesFound)
}
// actual result (non-meta data) is in the data element of the last item
// of the json array
data := anyValue[len(anyValue)-1]["data"].([]any)
Copy link
Member

Choose a reason for hiding this comment

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

according to the OpenAPI docs:

The ModelResultChunk will be always the first item in the response array. The rest of the items might be a mix of DataResultChunk, ErrorResultChunk and HeartbeatResultChunk

how are we safely assuming that the data we are interested in will always be the last item of the array?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @aryan9600 ,

Thank you for reviewing and the questions. AppDynamics Cloud Query response is a big model that covers metrics, logs, traces and spans. I am hoping to reduce the code complexity since we only retrieves a single floating value from the upstream. As long as the query submitted to the upstream is for a single metrics value, the result is either empty DataResultChunk or an array of {timestamp, numbers} pair with the last item being the value that we wanted. Rather than including the entire package of the API data model and un-marshaling all the meta data, what I try to do is to put enough error checking around each type cast so that we can safely return the numeric value at the end. I just realized that I need to error check the last type cast to float64 as well. Will it be good enough once that is added?

Thanks,

Charles

Copy link
Author

Choose a reason for hiding this comment

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

Hi @aryan9600,

I added error checking for type cast of the last item of the array to float. Love to hear your thoughts.

Thanks,

Charles

if len(data) < 1 {
return 0, fmt.Errorf("invalid response: %s: %w", string(body), ErrNoValuesFound)
return 0, fmt.Errorf("metrics not available for query: %s", query)
}
// nested in the data element of the first item
data_data := data[0].([]any)
if len(data_data) < 2 {
return 0, fmt.Errorf("invalid response: %s: %w", string(body), ErrNoValuesFound)
return 0, fmt.Errorf("invalid data item for query: %s", query)
}
// metrics data is the second element, the first element is the
// source, e.g. "sys:derived"
Expand Down