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

Adding conjoined-field-names for graphite templates #5562

Merged
merged 10 commits into from
Feb 18, 2016
Merged

Adding conjoined-field-names for graphite templates #5562

merged 10 commits into from
Feb 18, 2016

Conversation

chrusty
Copy link
Contributor

@chrusty chrusty commented Feb 6, 2016

I've been working on moving several of my clients away from using Graphite recently, switching to InfluxDB and Telegraf.

One of these clients has 200+ microservices all sending metrics through StatsD to Graphite, so I'm looking for a way to drop Telegraf in (running the statsd-input) as a replacement, using templates to massage the metrics into InfluxDB with tags (rather than re-build and re-deploy all of these things to support tagged metrics).

The feature added by this PR allows a compound "field" to be derived from any number of dotted strings at the end of the template (rather than just from one). This will result in a manageable number of measurements in the InfluxDB database (one for each microservice), with fields for each metric.

Consider this example:
Input:: com.company.service.helloworld.handler.hellorequest.error
Template: com.company.service.* measurement.measurement.measurement.measurement.field*

The result would be:
Measurement: com_company_service_helloworld
Field: handler_hellorequest_error

I realise there is also some work required for Telegraf to support this in conjunction with "timing" metrics.

Please let me know if you think this is crazy, or worth pursuing. Also, thanks for all the work on InfluxDB and Telegraf - these are really great tools!

@jwilder
Copy link
Contributor

jwilder commented Feb 8, 2016

@chrusty This was discussed awhile back. One issue w/ field* is if it's used in combination with measurement*. See: #4176 (comment)

Before this can be merged, we need to figure out how to handle these cases:

  • field* combined with measurement*. e.g. service.measurement*.field* would be non-deterministic on a measurement like sensu.metric.net.host.interface.eth0.tx_bytes. Probably need to return an error for this template.

Could you add some test cases for this combination?

@@ -237,6 +237,9 @@ func (t *template) Apply(line string) (string, map[string]string, string, error)
return "", nil, "", fmt.Errorf("'field' can only be used once in each template: %q", line)
}
field = fields[i]
} else if tag == "field*" {
field = strings.Join(fields[i:], "_")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use t.separator instead of hard-coded _.

@chrusty
Copy link
Contributor Author

chrusty commented Feb 10, 2016

@jwilder OK, fair-enough. I've made the changes you suggested (after hitting a conflict with some concurrent changes in master). Let me know what you think, and I'll squash it down to one commit.

wildcard_field_specified = true
}
}
if wildcard_field_specified && wildcard_measurement_specified {
Copy link
Contributor

Choose a reason for hiding this comment

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

Go style is to use mixedCaps instead of underscores: https://golang.org/doc/effective_go.html#mixed-caps

Can you rename to something like hasFieldWildcard and hasMeasurementWildcard?

@jwilder
Copy link
Contributor

jwilder commented Feb 12, 2016

@chrusty Small styling issue and can you update the changelog and credit yourself for this PR? After that, looks good.

@jwilder
Copy link
Contributor

jwilder commented Feb 12, 2016

@chrusty Also, can you sign the CLA? https://influxdata.com/community/cla/

@chrusty
Copy link
Contributor Author

chrusty commented Feb 14, 2016

@jwilder OK, that's all done. Thanks for your patience!

@jwilder
Copy link
Contributor

jwilder commented Feb 17, 2016

@chrusty Changes look good. Needs a rebase again though.

@chrusty
Copy link
Contributor Author

chrusty commented Feb 18, 2016

@jwilder OK, conflicts are gone

@jwilder
Copy link
Contributor

jwilder commented Feb 18, 2016

👍 Thanks @chrusty!

jwilder added a commit that referenced this pull request Feb 18, 2016
…ite-templates

Adding conjoined-field-names for graphite templates
@jwilder jwilder merged commit 0cb47cc into influxdata:master Feb 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants