cloud-bulldozer / go-commons

Code repository with all go common packages and libraries
Apache License 2.0
4 stars 9 forks source link

Comparison library #9

Closed rsevilla87 closed 1 year ago

rsevilla87 commented 1 year ago

Description

First implementation of a comparison library as suggested at https://github.com/cloud-bulldozer/ingress-perf/issues/1. The main function is Compare

// Compare returns error if value does not meet the tolerance as
// compared with the field extracted from the given query
//
// Where field is the field we want to compare, query is the query string
// to use for the search, stat is the type of aggregation to compare with value
// and toleration is the percentaje difference tolerated, it can negative
// it returns an error when the field doesn't meet the tolerancy, and an
// informative message when it does
func (c *Comparator) Compare(field, query string, stat Stat, value float64, tolerancy int) (string, error) {

This function calls a function to perform a query_string on the passed ElasticSearch instance/index

// queryStringStats perform a query of type query_string,to fetch the stats of a specific field
// this type of query accepts a simple query format similar to the kibana queries, i.e:
//
//  {
//   "aggs": {
//     "stats": {
//       "stats": {
//         "field": "our_field"
//       }
//     }
//   },
//   "query": {
//     "query_string": {
//       "query": "uuid.keyword: our_uuid AND param1.keyword: value1"
//     }
//   },
//   "size": 0
//  }
func (c *Comparator) queryStringStats(field, query string) (stats, error) {
rsevilla87 commented 1 year ago

Demo taken from the ingress-perf benchmark

$ ./bin/ingress-perf run --cfg config/example.yml --es-server=https://search-perfscale-dev-chmf5l4sh66lvxbnadi4bznl3a.us-west-2.es.amazonaws.com --es-index=ingress-performance --baseline-uuid=rosa-4.12-9w-2r-c5.4xlarge --baseline-index=ingress-performance-baseline  --tolerancy=50
time="2023-05-23 12:17:54" level=info msg="Running ingress performance adbddaf1-2a9e-4c53-a410-f98749fd901e" file="ingress-perf.go:41"
time="2023-05-23 12:17:54" level=info msg="Creating elastic indexer" file="ingress-perf.go:49"
time="2023-05-23 12:17:56" level=info msg="Starting ingress-perf" file="runner.go:42"
time="2023-05-23 12:18:00" level=info msg="Deploying benchmark assets" file="runner.go:148"
time="2023-05-23 12:18:02" level=info msg="Running test 1/1: http" file="runner.go:73"
time="2023-05-23 12:18:06" level=info msg="Running sample 1/3: 5s" file="exec.go:65"
time="2023-05-23 12:18:17" level=info msg="Summary: Rps=36212.07 req/s avgLatency=108545.60 μs P99Latency=273390.30 μs" file="exec.go:72"
time="2023-05-23 12:18:17" level=info msg="Running sample 2/3: 5s" file="exec.go:65"
time="2023-05-23 12:18:26" level=info msg="Summary: Rps=39225.47 req/s avgLatency=107981.03 μs P99Latency=436042.60 μs" file="exec.go:72"
time="2023-05-23 12:18:26" level=info msg="Running sample 3/3: 5s" file="exec.go:65"
time="2023-05-23 12:18:35" level=info msg="Summary: Rps=37162.24 req/s avgLatency=107182.25 μs P99Latency=233172.00 μs" file="exec.go:72"
time="2023-05-23 12:18:37" level=info msg="Indexing finished in 1.987s: created=3" file="runner.go:95"
time="2023-05-23 12:18:37" level=info msg="Comparing total_avg_rps with baseline: rosa-4.12-9w-2r-c5.4xlarge in index ingress-performance-baseline" file="runner.go:97"
time="2023-05-23 12:18:38" level=error msg="with a tolerancy of 50%: 37533.26 rps is 76.78% lower than baseline: 161627.30 rps" file="runner.go:108"
time="2023-05-23 12:18:38" level=fatal msg="some comparisons failed" file="ingress-perf.go:85"
$ echo $?
1
rsevilla87 commented 1 year ago

Not sure about the utility of the message from the Compare function, I'm open to ideas.

vishnuchalla commented 1 year ago

As we are already querying on a specific uuid, can we add support to multiple fields? For example

GET ingress-performance/_search
{
  "aggs": {
    "avg_lat_us_stats": {
      "stats": {
        "field": "avg_lat_us"
      }
    },
    "p99_lat_us_stats": {
      "stats": {
        "field": "p99_lat_us"
      }
    },
    "p95_lat_us_stats": {
      "stats": {
        "field": "p95_lat_us"
      }
    }
  },
  "query": {
    "query_string": {
      "query": "uuid.keyword:adbddaf1-2a9e-4c53-a410-f98749fd901e"
    }
  },
  "size": 0
}
rsevilla87 commented 1 year ago

As we are already querying on a specific uuid, can we add support to multiple fields? For example

GET ingress-performance/_search
{
  "aggs": {
    "avg_lat_us_stats": {
      "stats": {
        "field": "avg_lat_us"
      }
    },
    "p99_lat_us_stats": {
      "stats": {
        "field": "p99_lat_us"
      }
    },
    "p95_lat_us_stats": {
      "stats": {
        "field": "p95_lat_us"
      }
    }
  },
  "query": {
    "query_string": {
      "query": "uuid.keyword:adbddaf1-2a9e-4c53-a410-f98749fd901e"
    }
  },
  "size": 0
}

I personally prefer to keep this logic on of the Compare function, which is the most simple one since querying for multiple fields would also require to provide more values to compare, we can add a CompareMultipleFields function in future PRs. What do you think?

Sounds good to me.