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

Thresholds wrongly triggering a notification #119

Closed
alexdelprete opened this issue Nov 3, 2022 · 11 comments · Fixed by #138
Closed

Thresholds wrongly triggering a notification #119

alexdelprete opened this issue Nov 3, 2022 · 11 comments · Fixed by #138
Assignees
Labels
🐛 bug Something isn't working

Comments

@alexdelprete
Copy link
Contributor

Describe the bug
When setting thresholds, from what I interpret, if the values of the test are lower than thresholds for speed, and higher for ping, a notification should be created. Instead, I'm receiving notification for threshold every time, but test values do not hit those thresholds.

To Reproduce
Set threshold and do a test.

Expected behavior
I expect notifications to be fired only if test speed values are lower than thresholds or ping is higher.
Would also be good in the notification to describe which thresholds triggered it.

Screenshots
image
image

@stepanov1975
Copy link

Same issue here

@alexjustesen alexjustesen self-assigned this Nov 4, 2022
@alexjustesen alexjustesen added the 🐛 bug Something isn't working label Nov 4, 2022
@alexjustesen
Copy link
Owner

I'm planning on overhauling the event system this weekend (maybe, priority is going to be breweries). Currently is a little hacked together and won't scale well.

I like the idea of saying which threshold triggered as well so I'll be introducing that too.

@alexdelprete
Copy link
Contributor Author

Drink a good beer for me too. Cheers. :)

@alexjustesen
Copy link
Owner

@alexdelprete needs a little more work but the PR below fixes the absolute notifications and the preview is up so you can see what per threshold notifications looks like #138.

thanks for the suggestion!

@alexdelprete
Copy link
Contributor Author

So the notification will be for each specific threshold? great!

@alexjustesen
Copy link
Owner

So the notification will be for each specific threshold? great!

bingo

@alexdelprete
Copy link
Contributor Author

Now that I think about it...I'll wait for the latest influxdb patches (I read your comments in the PR) to be merged so I can test both issues.

@stepanov1975
Copy link

Pulled latest version and the problem is still here

@alexdelprete
Copy link
Contributor Author

alexdelprete commented Nov 10, 2022

Pulled latest version and the problem is still here

0.2.1 works for me. Did you recreate the container?

Anyway, more fixes to influx are coming.

@alexjustesen
Copy link
Owner

Pulled latest version and the problem is still here

Haven't done a release yet for that code. Currently sitting in PR #138 and will be probably a v0.3.0 tag

@alexdelprete
Copy link
Contributor Author

alexdelprete commented Nov 10, 2022

Haven't done a release yet for that code. Currently sitting in PR #138 and will be probably a v0.3.0 tag

0.2.1 contained the first fix by @adamus1red, and it's working here:

fix sending data to influxdb by @adamus1red in #130

but 0.3.0 will be much better I bet. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants