Add parentheses to sanitize list to prevent invalid metric name generation

[nixargh]

Using PR(:1) type of statistics and others, that have brackets, leads to an error. Clodwatch types reference

Config example:

discovery:
  jobs:
    - type: AWS/ELB
      regions:
        - eu-central-1
      period: 60
      length: 60
      dimensionNameRequirements:
        - AvailabilityZone
        - LoadBalancerName
      metrics:
        - name: Latency
          statistics: ["PR(:10)"]

Error:

 error(s) occurred:
* "aws_elb_latency_pr(_10)" is not a valid metric name

After fix it works like this:

aws_elb_latency_pr__10_{account_id="***...

Looks ugly from my POV but works and requires minimal changes 😃

[SuperQ]

Interesting, PR metrics seem to follow Prometheus histogram bucket format. We could probably add real support for this. Do you have a link to docs of what values are available?

The ELB metrics doc doesn't seem to include it. :(

[nixargh]

Interesting, PR metrics seem to follow Prometheus histogram bucket format. We could probably add real support for this. Do you have a link to docs of what values are available?

The ELB metrics doc doesn't seem to include it. :(

Actually my initial post contains the link to 'statistics' description: https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Statistics-definitions.html

And I agree that some of them looks like a good source to build a prometheus histogram:

Trimmed count (TC) is the number of data points in the chosen range for a trimmed mean statistic. 

probably together with:

SampleCount is the number of data points during the period.

So my plan is to collect a number of TCs, then relabel them and use for latency SLO with sloth framework the same way as I use other metrics. Something like configs below (but I haven't add SampleCount).
YACE config:

---
apiVersion: v1alpha1
sts-region: eu-central-1
discovery:
  jobs:
    - type: AWS/ELB
      regions:
        - eu-central-1
      period: 600
      length: 600
      dimensionNameRequirements:
        - AvailabilityZone
        - LoadBalancerName
      metrics:
        - name: Latency
          statistics:
            - p50
            - p90
            - p99
            - "TC(:0.2)"
            - "TC(:0.4)"
            - "TC(:0.8)"
            - "TC(:1)"
            - "TC(:2.5)"
            - "TC(:5)"
            - "TC(:10)"
            - "TC(:20)"
            - "TC(:40)"
            - "TC(:80)"
            - "TC(:160)"
            - "TC(:320)"

Relabel config:

---
scrape_configs:
  - job_name: yace
    static_configs:
      - targets:
        - localhost:5000
    metric_relabel_configs:
      - source_labels: [__name__]
        regex: 'aws_elb_latency_tc__([0-9]+)_([0-9]+)_'
        replacement: "$1.$2"
        target_label: le

      - source_labels: [__name__]
        regex: 'aws_elb_latency_tc__([0-9]+)_'
        replacement: "$1"
        target_label: le

      - source_labels: [__name__]
        regex: 'aws_elb_latency_tc__[0-9_]+_'
        replacement: aws_elb_request_time_seconds_bucket
        target_label: __name__

Result:

[SuperQ]

I read the docs, but it wasn't clear how buckets are managed on the cloudwatch side. It says it "creates" buckets but with no information about how this really works.

[cristiangreco]

@nixargh thanks for this contribution! If you have time and interest, would be cool to add a test case for this change! ❤️