crccheck / cloudwatch-to-graphite

Helper for pushing AWS CloudWatch metrics to Graphite
Apache License 2.0
55 stars 28 forks source link

Allow for per-metric definition of formatter #30

Closed kali-hernandez closed 8 years ago

kali-hernandez commented 9 years ago

Allows declaration of a yaml-wide formatter but still permit each metric to use its own formatter.

Useful if we want to customize the metric names depending on the type of metric requested.

e.g.:

- Namespace: "AWS/DynamoDB"
  MetricName: "SuccessfulRequestLatency"
  Statistics: "Average"
  Dimensions:
    TableName: "DeviceEvent"
    Operation: "Query"
  Unit: "Milliseconds"
- Namespace: "AWS/DynamoDB"
  MetricName: "ProvisionedWriteCapacityUnits"
  Statistics: "SampleCount"
  Dimensions:
    TableName: "PlaybackStream"
  Unit: "Count"
  Formatter: 'cloudwatch.%(Namespace)s.table-%(dimension)s.%(MetricName)s.%(Unit)s'
Options:
  Formatter: 'cloudwatch.%(Namespace)s.%(dimension)s.%(MetricName)s.%(statistic)s.%(Unit)s'
  Period: 5
  Count: 1

The metric ProvisionedWriteCapacityUnits will be output using the extra .table- in the string

crccheck commented 9 years ago

I'm pretty sure it's possible to do this now. Here's how the config would look:

- Namespace: "AWS/DynamoDB"
  MetricName: "SuccessfulRequestLatency"
  Statistics: "Average"
  Dimensions:
    TableName: "DeviceEvent"
    Operation: "Query"
  Unit: "Milliseconds"
- Namespace: "AWS/DynamoDB"
  MetricName: "ProvisionedWriteCapacityUnits"
  Statistics: "SampleCount"
  Dimensions:
    TableName: "PlaybackStream"
  Unit: "Count"
  Options:
    Formatter: 'cloudwatch.%(Namespace)s.table-%(dimension)s.%(MetricName)s.%(Unit)s'
Options:
  Formatter: 'cloudwatch.%(Namespace)s.%(dimension)s.%(MetricName)s.%(statistic)s.%(Unit)s'
  Period: 5
  Count: 1

I just nested the Formatter in another Options

The pro of your syntax is it's slightly terser, but at the cost of a little ambiguity.

kali-hernandez commented 9 years ago

Right, I did not notice (or think I could do it that way). Then I think this PR makes no sense anymore.

crccheck commented 8 years ago

closing because of discussion above. TL;DR: feature already exists