Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

Support tag expressions in storage-{aggregations,schemas}.conf #845

Closed
shanson7 opened this issue Feb 7, 2018 · 7 comments · Fixed by #872
Closed

Support tag expressions in storage-{aggregations,schemas}.conf #845

shanson7 opened this issue Feb 7, 2018 · 7 comments · Fixed by #872
Assignees

Comments

@shanson7
Copy link
Collaborator

shanson7 commented Feb 7, 2018

With tag support in metrictank, there may be different retention/aggregations based on tag values, not just names. For instance, longer retention for prod data.

@woodsaj
Copy link
Member

woodsaj commented Feb 7, 2018

I think this needs to be done in https://github.com/graphite-project/carbon first. @DanCech

@DanCech
Copy link
Contributor

DanCech commented Feb 7, 2018

You can do it today by using a pattern like ;tag=value, or if you want to match multiple tags ;tagA=value(;.+)*;tagB=value

@shanson7
Copy link
Collaborator Author

shanson7 commented Feb 7, 2018

Ok, then MT would just need to pass path instead of def.Name here on these 2 lines:

path := def.NameWithTags()
schemaId, _ := mdata.MatchSchema(def.Name, def.Interval)
aggId, _ := mdata.MatchAgg(def.Name)

@shanson7
Copy link
Collaborator Author

shanson7 commented Feb 7, 2018

With the caveat that this could definitely break existing patterns (for tagged time-series)

@DanCech
Copy link
Contributor

DanCech commented Feb 7, 2018

It'll also make it match the carbon behavior, so I think it's the right thing to do.

@Dieterbe
Copy link
Contributor

With the caveat that this could definitely break existing patterns (for tagged time-series) [in MT]

not a concern, since tagging in MT is still beta.
this change is probably something that should have gone with #806 (but in lieu of that, can be added now I think)
@replay any thoughts?

@replay
Copy link
Contributor

replay commented Mar 8, 2018

That makes sense to me as @shanson7 described it, mdata.MatchAgg() would just need to get path instead of def.Name

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants