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

Change alert levels to be independent #732

Merged
merged 1 commit into from
Jul 22, 2016
Merged

Change alert levels to be independent #732

merged 1 commit into from
Jul 22, 2016

Conversation

nathanielc
Copy link
Contributor

@nathanielc nathanielc commented Jul 20, 2016

Fixes #298

@beckettsean @rkuchan In reference to this blog post: https://influxdata.com/blog/tldr-influxdb-tech-tips-may-19-2016/ about Alert levels begin subsets. I think it should change, and this PR makes the change so that each alert level is independent of other levels.

Some of my reasons:

  • It's initially confusing, since its natural to think in the context of only a single alert level when writing the expression.
  • It's become really painful when your data does not follow a continuous domain (aka the data is not sort-able like a set of strings as labels).

Example of non continuous data:

// Data has status flag with values OK, WARN, or CRITICAL
// You have to write the expressions as
|alert()
    .warn(lambda: "status" == 'WARN' OR "status" == 'CRITICAL')
    .crit(lambda: "status" == 'CRITICAL')

This change allows the expressions to be simply

// Data has status flag with values OK, WARN, or CRITICAL
|alert()
    .warn(lambda: "status" == 'WARN')
    .crit(lambda: "status" == 'CRITICAL')

This represent a small breaking change so I want to get it in before the 1.0 official release.
The majority of the uses cases will be unaffected, only uses cases that leveraged the statefulness of expressions and relied on their execution order will break.

Wanted to make you both aware of this change since its been point of discussion before.

  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

@nathanielc nathanielc merged commit 0d92886 into master Jul 22, 2016
@nathanielc nathanielc deleted the nc-issue#298 branch July 22, 2016 15:25
@nathanielc nathanielc mentioned this pull request Jul 22, 2016
4 tasks
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.

1 participant