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

Make threshold for slow event configurable #3427

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Outfluencer
Copy link
Contributor

@Outfluencer Outfluencer commented Jan 21, 2023

configuration in millis, can be used for debuging or event monitoring, by setting value to less than 1 every event will be monitored

@Janmm14
Copy link
Contributor

Janmm14 commented Jan 21, 2023

I just really hope we don't see plugin devs recommending to increase that value if plugin users are complaining about it.

@Outfluencer
Copy link
Contributor Author

Should not be our problem, but i also hope that no plugins devs are abusing it

@MrIvanPlays
Copy link
Contributor

This is definitely gonna be abused. No from me.

@Outfluencer
Copy link
Contributor Author

I dont understand the problem, if the server owner wants to change the value he has the possibility. In wich way is this abuse

@Outfluencer
Copy link
Contributor Author

I could set a max cap to the current value so you can just make the threshold lower, so no plugin dev can recommend to increase the value.
I will wait for the opinion of @md-5

@MrIvanPlays
Copy link
Contributor

I dont understand the problem, if the server owner wants to change the value he has the possibility. In wich way is this abuse

A server owner says a developer his plugin is triggering the warning, therefore the developer "suggests" changing that value to something high, e.g 99999999999999, therefore abuse.

I could set a max cap to the current value so you can just make the threshold lower, so no plugin dev can recommend to increase the value.

That's probably better than just outright give everything to the server owner.

@Outfluencer
Copy link
Contributor Author

This would also be usefull to debug event executions by setting it to 0

@Outfluencer
Copy link
Contributor Author

@md-5 is there a reason this is not merged yet?

@Outfluencer
Copy link
Contributor Author

this feature would have helped me very much to spot a bug (i found out what was the problem but it tooked hours), that was caused by using an event wrong, with this pr we could debug all event executions (very helpfull),
i see no reason for not adding this

Copy link
Contributor

@Janmm14 Janmm14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to move the multiplication to the constant. And to use TimeUnit#convert or a comment to make input time unit clear.

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