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 support for matching tags multiple times in Graphite parser #5419

Merged
merged 1 commit into from
Feb 9, 2016

Conversation

m4ce
Copy link

@m4ce m4ce commented Jan 21, 2016

Original ticket #5416

@jwilder
Copy link
Contributor

jwilder commented Jan 21, 2016

Graphite doesn't have a concept of a host so hard coding a special host tag doesn't make sense to me. It seems like you want to be able to define a tag multiple times in a template and match/combine them?

@m4ce
Copy link
Author

m4ce commented Jan 21, 2016

@jwilder,

well, the current use case I have only involves host, which is easy since the separator should always be dot. But yes, the first thing that came to mind was to match tags multiple times, but then what separator to use?

Thanks,
Matteo

@jwilder
Copy link
Contributor

jwilder commented Jan 21, 2016

@m4ce I understand. The issue is that host is too specific for your use case for this plugin. We'd have to generalize the PR to work with other tags for this to be able to be merged.

For separator, there is already a separator config option that should be used. By default, it's ..

@m4ce m4ce changed the title Adding support for matching host multiple times in Graphite parser Adding support for matching tags multiple times in Graphite parser Jan 22, 2016
@m4ce
Copy link
Author

m4ce commented Jan 22, 2016

@jwilder,

I'm not a go developer, so I did what I could :) I hope the changes are good enough.

Thank you,
Matteo

@m4ce m4ce force-pushed the master branch 2 times, most recently from d25bdea to 9d90e7f Compare January 22, 2016 10:42
@m4ce
Copy link
Author

m4ce commented Feb 4, 2016

Any plan to merge this in the next release? Currently, I am deploying a custom binary around which is sub-optimal :)

@chrusty
Copy link
Contributor

chrusty commented Feb 7, 2016

I've got a PR which is kinda related to this, which allows you to wildcard the "field" name of each metric:

#5562

@jwilder
Copy link
Contributor

jwilder commented Feb 8, 2016

@m4ce This looks better. We need to have some test cases for this code though. You could add one fairly easily to this test case: https://github.com/influxdata/influxdb/blob/master/services/graphite/parser_test.go#L34

@m4ce m4ce force-pushed the master branch 2 times, most recently from e8bce4e to da11d5e Compare February 9, 2016 08:59
@m4ce
Copy link
Author

m4ce commented Feb 9, 2016

@jwilder, I've added a case which tests multiple tags. I hope that is sufficient! Thank you!

jwilder added a commit that referenced this pull request Feb 9, 2016
Adding support for matching tags multiple times in Graphite parser
@jwilder jwilder merged commit 83b96b3 into influxdata:master Feb 9, 2016
@jwilder
Copy link
Contributor

jwilder commented Feb 9, 2016

Thanks @m4ce

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.

None yet

3 participants