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

Convert interval to be an array of intervals #5

Open
kaspernissen opened this issue Feb 4, 2019 · 3 comments
Open

Convert interval to be an array of intervals #5

kaspernissen opened this issue Feb 4, 2019 · 3 comments
Labels
enhancement New feature or request

Comments

@kaspernissen
Copy link
Member

kaspernissen commented Feb 4, 2019

If you want to perform the same query but aggregate at different intervals, it would be handy to provide a string array of intervals instead of just a string right now.

Example now:

queries:
- query: res.statusCode>=200 | count(field=context.userId, distinct=true)
  repo: services
  interval: 24h
  metric_name: active_app_users
- query: res.statusCode>=200 | count(field=context.userId, distinct=true)
  repo: services
  interval: 1h
  metric_name: active_app_users
- query: res.statusCode>=200 | count(field=context.userId, distinct=true)
  repo: services
  interval: 5m
  metric_name: active_app_users
- query:  res.statusCode>=200 | count(field=context.userId, distinct=true)
  repo: services
  interval: 1m
  metric_name: active_app_users

Would produce the same metrics output:

queries:
- query: res.statusCode>=200 | count(field=context.userId, distinct=true)
  repo: services
  interval: [1m, 5m, 1h, 24h]
  metric_name: active_app_users
@kaspernissen kaspernissen added the enhancement New feature or request label Feb 4, 2019
@Crevil
Copy link
Member

Crevil commented Feb 4, 2019

Makes good sense. Maybe we should just make it the default? So removing the interval: 1m option all together?

@kaspernissen
Copy link
Member Author

Make it the default that we always output 4 different intervals? We don't always want that many intervals. I think it makes good sense to be able to specify the interval as an array, and interval is required unless we want to fall back to a default interval.

@Crevil
Copy link
Member

Crevil commented Feb 4, 2019

No, I mean make it the default for interval to be an array. :)

So if you only want one value, you specify it like:

queries:
- query: res.statusCode>=200 | count(field=context.userId, distinct=true)
  repo: services
  interval: [1m]
  metric_name: active_app_users

The reasoning to do so, is to avoid explaining and maintaining functionality that it can either be a string or an array of strings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants