SpongeAPI icon indicating copy to clipboard operation
SpongeAPI copied to clipboard

Rework of the Metrics API (Missing undecided config value)

Open BrainStone opened this issue 6 years ago • 11 comments

As per the discussion on the Discord, I'm writing this down as an issue/feature request.

Per older and newer discussion it was initially agreed upon that Sponge provided an API for plugins to check if they may send metrics. And that they are supposed to ask for consent. The API was added in a very basic sense. The issue being the plugins only knowing if metrics are enabled or not. Meaning that even if an admin purposely disabled them, they would get asked every time, if a plugin ever decided to do that notification.

That's the core issue. Whie discussing it several suggestions came up to streamline the process and provide a nice and consitent UI to the user/admin.

I propose the following:

  • Let's have the following global stats options:
    • Undecided (admins get asked to decide) (undecided)
    • Enabled by default (true or enabled)
    • Disabled by default (false or disabled)
    • Please ask individually (ask)
  • Every individual plugin gets these options:
    • Undecided. If global is set to enabled or disabled, this defaults to that value. If the global setting is on ask, the admins get individually asked for each plugin checking if it's allowed to send. (undecided)
    • Enabled (true or enabled)
    • Disabled (false or disabled)

This is to allow people to give general consent or dissent, for the ones that really don't want to be bothered by individual prompts and also to give those who want it, full controll out of the box.
This natually needs commands to allow changing all these settings directly, so they can be used in clickable links/texts.

Now this shouldn't be too hard to implement.

In addition I had a few other suggestions to improve usability:

  • Move all the metrics stuff into it's own config, so it can be easily found. I find that the metrics settings are super hidden inside the global.conf and spent quite a while looking for them. Additionally it would make it easier to migrate any existing settings, since they will definately need to change if they are supposed to be kept, as the old system defaults to false, which would mean that when the new system gets in place, everything is still disabled, which would be a considerable loss for plugin devs.
  • Actually implement the prompts in Sponge itself. This has quite a few advantages:
    • You make it a lot easier for plugin and stats devs to work with the restrictions. Like that basically the same stuff doesn't need to be implemented a billion times over
    • It would make sure the messages and prompts are standardized and consitent. Having every plugin have their own style and timing can get super annoying
    • If there ever will be a rivaling metrics system to bStats (or some dev makes their own custom) the user doesn't get two messages for the global setting
    • It might even be possible to prevent plugins executing the commands on behalf of users this way, as it would be all Sponge internal.

Now I would kindly ask that if that's the way it'll be solved, that the message is worded something like

Would you like to allow plugins to collect anonymized metrics? This helps the developers improve their plugins.  
Yes - No - Please ask me individually

Essentially asking politely to enable it while pointing out it helps the devs make the plugin better.

BrainStone avatar Feb 15 '19 19:02 BrainStone

Not sure if this is still the case but I noticed Sponge was adding values to the config for every plugin loaded. We don't need to save undecided for plugins that dont even have metrics.

Faithcaio avatar Feb 18 '19 13:02 Faithcaio

That's a good point. And naturally these prompts should only appear when plugins check if they may send metrics and individual prompts only for thos which check for it. Makes it also easier seeing which plugins do send metrics.

BrainStone avatar Feb 18 '19 22:02 BrainStone

https://github.com/SpongePowered/SpongeAPI/pull/2018

ImMorpheus avatar Jul 06 '19 15:07 ImMorpheus

The conditional requests sound good to me, however we should consider the timing when we will send them. If we send them "every time" a plugin checks its permission then we might annoy the admins. If we check it only during the first time no admin might be only at all. Maybe prompting this after joining the server might be an idea. Only if the first request is answered will the next one be shown. How should we handle cases where admins only "activate" their permissions with a special command?

ST-DDT avatar Jul 06 '19 18:07 ST-DDT

I think the best way to time it would be to check a permission when a player joins and then display the prompts that are there to be shown. One after the other. Ideally changing a plugins setting would send an event the plugin or stats library can listen to.

BrainStone avatar Jul 06 '19 19:07 BrainStone

For what it's worth, this was discussed a while ago and the guidelines we're going to strive for are https://drive.google.com/file/d/16kbuFVtuKasrKLTNb6gIOc-BOEyWBnFa/view (it does say we're striving for API 8, but if it hits 7.2 too, hey, so much the better).

Note that "conditional requests" are actually against the guidelines and should be handled by Sponge instead. It does call into question what the "undefined" state is for, but I'm not particularly against it being there either.

dualspiral avatar Jul 06 '19 23:07 dualspiral

Move all the metrics stuff into it's own config

👍 would love to see a metrics.conf in API 8.

jamierocks avatar Mar 11 '20 22:03 jamierocks

The associated PR was merged, but there seems to be more here than just what that PR proposed. I'm going to slap at 1.14 label on it now though, particularly with that last comment.

dualspiral avatar Jun 09 '20 09:06 dualspiral

metrics.conf was added by https://github.com/SpongePowered/Sponge/commit/ad94e11623b4d9e33769151e58be39c55acf323f

ImMorpheus avatar Mar 08 '21 22:03 ImMorpheus

This shouldn't have been closed - as well as the config not being an API element, there are other aspects of this that should be discussed.

This issue is not about the config file, it's about the wider API and prompt issues from a plugin standpoint, from above.

  • Actually implement the prompts in Sponge itself. This has quite a few advantages:

    • You make it a lot easier for plugin and stats devs to work with the restrictions. Like that basically the same stuff doesn't need to be implemented a billion times over
    • It would make sure the messages and prompts are standardized and consitent. Having every plugin have their own style and timing can get super annoying
    • If there ever will be a rivaling metrics system to bStats (or some dev makes their own custom) the user doesn't get two messages for the global setting
    • It might even be possible to prevent plugins executing the commands on behalf of users this way, as it would be all Sponge internal.

This may be better off on the Sponge repo than the API one, but this issue is not resolved yet.

dualspiral avatar Mar 09 '21 16:03 dualspiral

I missed the prompt part, good catch.

ImMorpheus avatar Mar 10 '21 17:03 ImMorpheus