fix(analysis): Graphite query - remove whitespaces #2752

mdsjip · 2023-04-30T16:21:19Z

If metric query contains surrounding whitespace, then it's not trimmed and Graphite host returns 400 Bad Request

Simplest example of analysis template:

apiVersion: argoproj.io/v1alpha1
kind: ClusterAnalysisTemplate
metadata:
  name: error-rate
  namespace: argo-rollouts
spec:
  metrics:
    - name: error-rate
      failureLimit: 1
      # Note that the Argo Rollouts Graphite metrics provider returns result as an array of float64s with 6 decimal places.
      successCondition: 'len(result) > 0 and result[len(result)-1] < asFloat(10)'
      provider:
        graphite:
          address: https://some-graphite-host.com
          query: |
            from=-2&target=sumSeries(app.http.*.*.count)

When this is parsed from YAML, this query contains a newline in the end, so when it's "trimmed", it still contains a trailing space, so when put into the query, it becomes:

format=json&from=-2min&target=sumSeries%28app.http.%2A.%2A.count%29+

This leads to 400 Bad Request response from Graphite, example cURL command:

$ curl 'https://some-graphite-host.com/render?format=json&from=-2min&target=sumSeries%28app.http.%2A.%2A.count%29+'
Bad Request: Bad Request

Target              : sumSeries(app.http.*.*.count) 
Parsed so far       : sumSeries(app.http.*.*.count)
Could not parse     :

Proposed solution: replace all whitespace in the query with an empty string.

Checklist:

Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
I've signed my commits with DCO
I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
My builds are green. Try syncing with master if they are not.
My organization is added to USERS.md.

github-actions · 2023-04-30T16:34:27Z

Go Published Test Results

1 952 tests 1 952 ✔️ 2m 36s ⏱️
  118 suites       0 💤
      1 files       0 ❌

Results for commit 628842f.

♻️ This comment has been updated with latest results.

codecov · 2023-04-30T16:34:36Z

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02 ⚠️

Comparison is base (c74c464) 81.64% compared to head (580a38c) 81.62%.

❗ Current head 580a38c differs from pull request most recent head 628842f. Consider uploading reports for the commit 628842f to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2752      +/-   ##
==========================================
- Coverage   81.64%   81.62%   -0.02%     
==========================================
  Files         133      133              
  Lines       20172    20157      -15     
==========================================
- Hits        16469    16454      -15     
  Misses       2849     2849              
  Partials      854      854