KSP-CKAN / CKAN

The Comprehensive Kerbal Archive Network
https://forum.kerbalspaceprogram.com/index.php?/topic/197082-*
Other
1.98k stars 347 forks source link

Debug Logging Integration with KSP through a Plugin #1802

Open ferram4 opened 8 years ago

ferram4 commented 8 years ago

TL;DR: Create a KSP plugin to print a bunch of debug info to the log at startup to establish the state of the CKAN install for modder convenience. Have this automatically installed in every CKAN install. This should (I think) be much quicker to create, implement and maintain than #1797, but won't replace that.

Coding this should be pretty simple on the KSP side. I've already offered to do most of the work on that myself elsewhere, and @linuxgurugamer has already offered to help as well. However, since this code will be basically baby's first Part-less KSP plugin, any CKAN contributors / devs with any interest in getting into KSP modding should jump in.

Rationale

  1. Provides more info to modders when debugging CKAN installs, likely can help detect metadata errors right there, and works within the current support setup that already exists.
  2. Putting the info in KSP's debug log means that users don't have to do anything more than provide the logs modders already ask for.
  3. Allows some robustness in detecting CKAN issues for users that go around established procedures, whether they come from #1797 or elsewhere.

    Specs

On the CKAN side, all the information should end up placed in a text file somewhere within the CKAN directory for easy reading. I don't know the details of this, so someone more experienced should weigh in; this may require no changes or some minor ones at all. There will also need to be some method for getting it installed.

On the KSP side, the idea is pretty simple a single dll put in the GameData directory like any other plugin. Functionally, start instantly in the loading menu (similar to ModuleManager), read information from config file, print to the KSP log the first frame, destroy the class that did this, do nothing for the rest of the game. Simple, easy, clean.

Suggested things to print, I'm sure others have ideas:

  1. CKAN version
  2. Mods installed and their versions
  3. Whether mods were installed via CKAN or autodetected
  4. What repo the mod's metadata came from and if it is the official source (CKAN-meta or an author's repo)
  5. If KSP's version matches the KSP version that CKAN detected (find BuildID.txt and Readme.txt shenanigans) and what those are.
  6. Manually installed mods and GameData directory structure
  7. KSP-AVC info (maybe? depending on how difficult it is)

    Plugin Maintenance

With a KSP plugin comes the risk of it breaking every KSP update. Fortunately, the risk that this happens drops tremendously the less the plugin interacts with parts of KSP; as envisioned above, this plugin only works with KSP-specific code in 1 place (the entry point) and then works solely with the Unity game engine, which is far more stable codewise. I expect that no changes will be necessary for compatibility between KSP versions and that at worst only recompiles will be necessary.

Further Questions

  1. What do we call it? This will exist within the GameData directory like any other mod and needs to be distinct.
  2. How should it be installed and what options should users get here? Using it as a universal dependency seems disingenuous. How should users be informed that it's going to be there and do nothing but add a bit of stuff to the log?
  3. License for this thing? It is a separate plugin, so perhaps a different license than CKAN itself is ideal.
dewiniaid commented 8 years ago

So the data would essentially be the "General Statistics" part of #1797, appended to the log file at startup?

I can see it working something like:

  1. On a new CKAN install/adding a new install to CKAN/first CKAN release with this feature, do a one-time prompt to install the "CKANStats" plugin, which it does so by selecting it from the default repository.
  2. If the CKANStats plugin is detected, CKAN writes data that the plugin can't easily get to KSPDIR/ckan.stats -- like CKAN version, what KSP version CKAN thinks is present, names of autodetected mods, and the path to installed.ckan (which contains all of the installed mod versions)
  3. CKANstats reads ckan.stats and installed.ckan on startup and spits out the details to the output log. It can additionally include additional data directly from the running game environment like actual KSP version and all of the fun C# assembly version bits. If ckan.stats is not found, this should be mentioned specifically.
politas commented 8 years ago

I think I've commented on the forum that I think this is a great idea, and would be happy to do the coding work on it, but I would need some serious hand-holding follow-the-bouncing-ball assistance to get started.

anxcon commented 8 years ago

as this is stepping into logging i'll put it here since distribution of said log is a factor

simply having the plugin add to the log is not enough IMO, as not that many people understand where a log is / have to ask or be told first / etc. a button on ckan to open the log, is also not the best idea due to most probably open files in windows notepad, which long logs can crash it, and not all have NP++. perhaps a button to auto pastebin and supply link would help this.

ferram4 commented 8 years ago

@dewiniaid it sounds like it, though I'd shy away from calling it CKANStats or treating those as statistics. It could remind some people of the Modstats situation and cause them to freak out and now you've got pointless drama to deal with and we'll be back to where we started.

@politas Once we figure out exact details, just pester me on IRC. If I respond I'm awake and we can get some work done.

@anxcon Eh... I don't think any of those would be helpful.

Distribution of the logs isn't really difficult; finding the log only requires a short response to the user about its location, and then it tends to be found and uploaded very quickly.

I can also see ways where setting up something like that could be a massive pain for modders if it creates the wrong impression. If users think that it is more automatic than it is (that it auto-sends us the link and the log) then the experience of modders will turn into "CKAN users keep telling me that they're sending me logs but I don't get anything. Dammit, providing support to these people is pointless." At best, it'll either require another step of opening CKAN.exe to upload the log or will require in-game stuff (which has performance implications to have something ready to send a log at all times).

Ideally, I want to treat my CKAN users identically to my manual users: "Provide the log and repro steps, log is in X location, upload it somewhere and post the link." Differentiating between the two means having to tell if they're using CKAN or not beforehand and providing different steps. This is just supposed to be additional information about the CKAN install that fits neatly into the support system we have right now, not trying to add new features to the support system that users will need to be trained to use.

politas commented 8 years ago

I would go with "CKAN Logger" as the title, and include the default log locations in the mod's abstract. As for getting users to install it, all we need do is get mods whose authors will make use of the data include it as a dependency.

Cphoenix commented 8 years ago

all we need do is get mods whose authors will make use of the data include it as a dependency.

Technically speaking, wouldn't that basically be everyone? Who knows when and which authors might end-up using it once they see it in the logs along side a bug report. It might be worth making that plugin "special" in some way.

It's probably best to keep the logger and the stats plugin ideas at arm's length from each other. The logger is just going to dump some extra data to the KSP log. No harm there. In future it may be useful to have the stats plugin send telemetry back to the CKAN team. In such a case it'd have to be strictly opt-in for users for sure (since completely anonymizing data is nigh impossible).

Totally agree with @ferram4 though; the whole point of putting it in the KSP log is that it won't require any extra work for anyone once it's done. Modders already ask for logs reflexively. A quick search for "CKAN" and you ideally then know whether or not they are using CKAN to manage your mod (putting aside the possibility of users uninstalling that plugin).

politas commented 8 years ago

Agree on keeping the statistics logging separate. What @ferram4 has laid out in the initial post involves no invasion of privacy, it's just storing information CKAN already creates on the user's computer in another location. Sending the log files is a voluntary extra step.

Making it a dependency as I suggested means users have to opt in, unless they choose to install the relevant mods manually, so mod authors get what they want either way, and authors who are just making simple part mods can avoid any controversy that might come up.

linuxgurugamer commented 8 years ago

Why not do both? It's going to be a mod, so, for the modern who want to, have them require it. If not required, then it can be just another mod for people to install if they want it. I suppose you could make it a requirement of using ckan in general. Also, people are already used to log files. I don't see a need to implement a UI for this, unless what it logs goes into a different file On Jun 28, 2016 1:27 AM, "Myk" notifications@github.com wrote:

Agree on keeping the statistics logging separate. What @ferram4 https://github.com/ferram4 has laid out in the initial post involves no invasion of privacy, it's just storing information CKAN already creates on the user's computer in another location. Sending the log files is a voluntary extra step.

Making it a dependency as I suggested means users have to opt in, unless they choose to install the relevant mods manually, so mod authors get what they want either way, and authors who are just making simple part mods can avoid any controversy that might come up.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/KSP-CKAN/CKAN/issues/1802#issuecomment-228951836, or mute the thread https://github.com/notifications/unsubscribe/ABjf1o31nmqcx_2toF5BUwHow6554S8oks5qQLDQgaJpZM4I_Uf1 .

dewiniaid commented 8 years ago

@Ferram4 I totally forgot about that drama, though really I was just picking an arbitraryb placeholder name.

Re: statistics logging -- let's not confuse this overly with #1797. That issue concerns opt-in data collection. This would be the exact same data, just written to a log that is only shared when a player opts to do so as part of uploading a log. In fact, this might very well be part of the foundation for the other issue.

Re: other mods installing this as a dependency -- the logging functionally for mod versions/etc essentially requires CKAN itself anyways. Rather than add this as a dependency to a bunch of mods (which shouldn't truly have it as a dependency), I think having CKAN itself suggest the installation is the better option here.

A truly automated bug reporting system that neatly packages everything would be an amazing tool, but let's focus on the beginnings first (I.e. as per the original suggestion)

EDIT: wrote"installing a mod" not "uploading a log" by mistake.

ferram4 commented 8 years ago

I'd agree with @dewiniaid that this shouldn't be added as a dependency to mod metadata. It strikes me as a misuse of the dependency label for something that the mods themselves don't actually need to function and will cause all sorts of confusion when there are CKAN users that have it and are told that so many mods require it, but manual users don't get that. If the debug logger plugin isn't going to be installed through some special use thing in CKAN then it should at most be counted as a recommends, and I'd say that in that case it would probably be safe to add it to any plugin-having mod's metadata. I doubt there's anyone (modder or user) that will be annoyed with that situation.

politas commented 8 years ago

Ok, I'd be fine with that, though I don't see a problem with a mod author saying "if you want to install my mod via CKAN, you must have this installed for bug reporting purposes." That seems like a reasonable demand to me.

Cphoenix commented 8 years ago

Completely reasonable.

Re the stats stuff: I wasn't suggesting anything nefarious was going on, just wanted point-out that making sure users know the difference between the two will save future headaches on CKAN's end. :smile:

Blu3wolf commented 8 years ago

Ok, I'd be fine with that, though I don't see a problem with a mod author saying "if you want to install my mod via CKAN, you must have this installed for bug reporting purposes." That seems like a reasonable demand to me.

So long as the demand is a condition of seeking support, rather than a condition of using the mod, that would seem reasonable.

ferram4 commented 8 years ago

Ok, I'd be fine with that, though I don't see a problem with a mod author saying "if you want to install my mod via CKAN, you must have this installed for bug reporting purposes." That seems like a reasonable demand to me.

It is reasonable if it is set up as a dependency for simply installing mods through CKAN. Mods themselves arguably don't depend on this and should really only recommend it, but CKAN itself can be argued to depend on this for easing support purposes. Hence the question of whether it should have its own separate install implementation beyond the base mod installation.

Blu3wolf commented 8 years ago

As an optional component, sure. EDIT: having reviewed the idea, there is a convincing argument for this to be part of the default CKAN first install.

Cphoenix commented 8 years ago

Hence the question of whether it should have its own separate install implementation beyond the base mod installation.

I would say it definitely should. We're talking about what, one plugin? Maybe two once the stats plugin is serviceable? That's not in nightmare territory in terms of hard-coding parts of the install into the application itself. I'd say, create a .netkan for it and install it right next to all the other mods (so it is where a user would expect in the GUI and can easily and intuitively uninstall it on the off chance it does cause problems), but have CKAN prompt to install it once per KSP install it's managing by default, letting the user know "Hey, we installed this extra plugin. All it does is add more information to your KSP logs for the CKAN team or mod authors to use should you need support with CKAN-installed mods." (Putting "CKAN team" first before "mod authors" is very intentional, to subtly suggest that hey, bring problems to CKAN too/first!)

I think most people will just leave it alone after it's explained what it does.

Edited for clarity.

linuxgurugamer commented 8 years ago

Personally, I don't see the problem in saying "You're using CKAN, then this mod will be installed automatically". However, I'm sure some people will get their feathers ruffled by this, so why not do the following:

  1. New KSP install
  2. CKAN is told about the new install
  3. CKAN installs ckan mod, and lists it under "Installed Mods"
  4. User can uninstall ckan mod if desired
  5. In CKAN options, have an option to NOT automatically install ckan mod
Blu3wolf commented 8 years ago

Having the program take an action the user isnt expecting is a great way to alienate your users.

Would be best as a default, but one the user can opt out of, without it going ahead and making changes to the users install without it being told to.

linuxgurugamer commented 8 years ago

One way around that would be to have the option as I described, and also, when it is setting up a new KSP install, to open up a dialog saying that it would like to install a small mod designed to assist in the CKAN support.

dewiniaid commented 8 years ago

My thought process regarding the plugin install:

This allows appropriate prompting on both new installs and all existing installs after the first CKAN release supporting this feature.

For the actual prompt text:

"CKAN includes an optional Kerbal Space Program plugin that adds details about your CKAN version, installed addons, and your system configuration to KSP's output log. This information assists mod authors in troubleshooting any issues that you may encounter.

This data is NOT shared and does not leave your computer unless you upload the file manually.

Would you like to install it? (If you change your mind later, you can add or remove the "CKAN Diagnostics" addon)

[ ] Also choose this action for future KSP installs"

ferram4 commented 8 years ago

Of course, this all leads to the problem of how modders handle CKAN installs that don't have this installed.

Simple solution to this: implement this similarly to the way that CompatibilityChecker is in the mods that have it. Each version runs and if it is not the most recent version, it shuts down to let that version handle things, eventually electing one to execute the code. For this situation we can have CKAN's official version get priority while the embedded versions only run if that doesn't exist.

It may even be better to create this entirely as something for modders to embed rather than a centralized bit of code distributed by CKAN.

Modders who absolutely want this get more support info. CKAN's dependency field isn't misused for things that it shouldn't be used for. Users don't freak out over a new plugin that they don't strictly need that they don't know what it does. And there is precedent with this in terms of CompatibilityChecker, particularly the early versions that just created warnings and did nothing else.

Cphoenix commented 8 years ago

@ferram4 That's a pretty good solution. I'd still opt to have CKAN install it officially by default anyway, but if authors can bundle it with their mod too, that'd be swell, because it would give you guys a hard-and-fast way of saying "No, you want to use my mod with CKAN, you're bloody well going to have extra logging to save me headaches."

Then the only case you'd have to worry about is people taking that plugin out of your package after it's installed. In which case they're being obstinate, paranoid, and dumb, and you just can't design for that. :stuck_out_tongue: (Honestly though, who does that? If it ever happens to KSP-AVC, I doubt it's more than a small handful of people, if that.)

Edit: Let's not over-think this. All this proposed plugin will do is add extra lines to a log file. Save the questioning prompts that users get for stuff that's more intrusive, like a stats plugin would be if it sends data back to the CKAN team automatically.

ferram4 commented 8 years ago

No, I don't think you understand. Distributing it as source code for modders to implement in a way that the code becomes an integral part of the main mod dll. There would be no way to remove it short of recompiling from the source, exactly like CompatibilityChecker right now. No bundling of new dlls, no dependency fields, nada. Making CKAN install it as a separate thing is simply to handle part-only mods and mods where the authors don't want to include that because they're more lax with not getting that info.

Cphoenix commented 8 years ago

Ohhhhhhhh. I get it. That's even better. :)

BobPalmer commented 8 years ago

Sorry for hopping in late. Question: Will this show manually installed mods? Does it (Can it?) include KSP-AVC version info if present? A directory tree would also be nice. Just thinking of ways to support hybrid installs better... or cases where the user installed via CKAN then proceeded to do horrible things.

ferram4 commented 8 years ago

@BobPalmer Yeah, those all sound like good ideas. Directory structure is probably very easily doable as is manually installed mods (technically those should be covered by auto-detected, but the directory structure will be more clear). AVC depends on how that's implemented, I'm not sure about the details of that.

Added to the OP specs list.

BobPalmer commented 8 years ago

Thanks - KSP-AVC is basically a JSON file with .version info. I expect Cybutek has some code lying around as he does a nice summary on the start menu if you have the full KSP-AVC installed. It's a nice troubleshooting bit.

dewiniaid commented 8 years ago

@bobpalmer I'd vote letting it report the mods that CKAN autodetects. I have some issues reservations with it reporting the entire Gamedata/ tree though as it might lead to accidental information leakage (plus things like source control folders/etc), basically as mentioned in #1797.

Basically, imagine if you had started working on the assets for a hypothetical "UKS Plus" mod, forgot that your logs included the gamedata folder, and posted a bug report on another mod. Your probably wouldn't want to accidentally leak that project to the world before your ready.

BobPalmer commented 8 years ago

Leave it as an option then. tbh borked up folders (or not telling me you installed something you did) account for a lot of support issues, including those from CKAN users.

ferram4 commented 8 years ago

It is, in fact, the only common type of install error that happens to manual installs, so it's pretty necessary to have. The other less common install errors are leaving out the dependencies, but that usually requires a user being unnaturally "clever" first.

BobPalmer commented 8 years ago

Never underestimate the cleverness of our users ;)

anxcon commented 8 years ago

cleverness?...ok...sure....

i'd definately side with full folder tree, stuff in gamedata is ment for / and loaded by ksp, additional stuff (backups etc) should be outside gamedata. files however (names, hashchecks, etc) you might be able to scratch a point on, however folder tree i feel is a must / not intrusive

Blu3wolf commented 8 years ago

Its worth pointing out that in the situation presented by RD : "users install with CKAN, then proceed to do horrible things" - CKAN wont have autodetected those mods, unless it was run again afterwards. You wont be able to rely on AD mods from CKAN for that situation.

That caveat aside, Ferrams idea rocks.

politas commented 8 years ago

Ok, this is getting rather beyond "Reporting what CKAN has done", which I thought was the original idea. Could we make it so the version included in other plugins scans GameData specifically for the expected files for that mod, while the main CKAN plugin restricts itself to the information CKAN already has? That would let you see when users have messed with files after CKAN has done its thing. With some code to combine the results? How feasible does that sound?

Cphoenix commented 8 years ago

Basically, imagine if you had started working on the assets for a hypothetical "UKS Plus" mod, forgot that your logs included the gamedata folder, and posted a bug report on another mod. Your probably wouldn't want to accidentally leak that project to the world before your ready.

I was going to suggest hashing the directory names and comparing them to a limited rainbow table but that's probably over-the-top and can't really provide a solution to the possibility of data leakage since you can't use random salts. (I mean bcrypt and scrypt are good, especially if you stretch them a lot, but given that directory names are really, really, really simple, brute-force even at 100 or so hashes a second suddenly looks really fast. :neutral_face: )

It may be a much simpler option to just disable a directory tree dump by default, stick that setting in a config file, and ask users to set it to true whenever you suspect that the user has done something hinky, and then have them run KSP so it dumps the log and after it does that have it modify the config back to false. So in the case of mod authors reporting bugs to other authors, there would be far less worry about accidentally uncovering works-in-progress, or any other accidental leakage.

ferram4 commented 8 years ago

No, adding config files that need to be changed will make things unnecessarily complicated. I have a better idea:

A full GameData printout isn't done; instead, set up a system so that each mod that implements the distributable version can provide some extra bit of things to be printed out to the log, in the same way that CompatibilityChecker out-sources the actual details of whether a mod is compatible to each mod's implementation of CompatibilityChecker (you can literally make it return compatible == todayIsTuesday if you want). It doesn't guarantee the full privacy if someone really wants to dump the entirety of the GameData folder, but frankly I think concerns about the privacy of GameData folders, especially with regards to in-development mod work is utterly pointless at best.

If it really turns out that people are installing a lot of things manually and then lying about it to modders (on a scale that impacts support) we can revisit that later.

Anyway, @politas should this thing's repo be part of KSP-CKAN itself or elsewhere?

politas commented 8 years ago

@ferram4, My initial thought is elsewhere. I'd like it to be clear that this is a companion to CKAN, not a part of CKAN. The details of how to encourage users to install it can be left for after we've built it, I think.

Ok, I've created a Repo - https://github.com/politas/CKAN-Debug-Helper. Not much there, yet.

What you said about the distributable part providing extra things to be printed out in the log is where I was going with my last comment. Things like a list of files that should exist in GameData, and if we can implement some wildcarding, authors could check for, say, a specific version of ModuleManager, and/or any version of ModuleManager, as well as checking for the files installed by the mod.

HebaruSan commented 5 years ago

I guess the consensus here was that debating auto-install policy is much more fun than writing a plugin. ;)

We need something to install before any of that becomes relevant. I see no harm in starting with a basic, completely optional version as a proof of concept, then adding more output over time, and possibly adding prompting/auto-install functionality once it's complete and proven robust.

I'm lukewarm on incorporating output from other mods. I understand the value proposition, but those mods can already add their own checks and output in their normal code if they consider it important enough, without the wrinkle of depending on this CKAN-specific plugin. And it adds another failure point in what should be a simple, reliable CKAN logging plugin. It looks like a lose-lose collaboration to me.

Finally the question of sourcing the CKAN data. All of the CKAN-related info is located in KSP/CKAN/registry.json, except for the CKAN version, which could be added. Unfortunately that file is 15-20 MB and takes several seconds to parse, so it's a non-starter to do that at KSP load time. Generating a special file just for this project is one option, but then we would be storing the data twice and introduce the risk of getting out of sync; I think it would be better to split that file up instead so the plugin could read the official source of truth directly. Currently it combines lots and lots of "static" data (all of the available modules, most of the file) and a little "dynamic" data (what's currently installed, auto-detected, repos, etc.). Storing the static data in a separate file would make it feasible to read the rest at KSP startup (and could also speed up various other things such as ckan list).

HebaruSan commented 5 years ago

Oof, parsing JSON in a KSP plugin hurts. Newtonsoft keeps complaining about missing references instead of working, and Unity's JsonUtility can only handle a pathetically limited subset of JSON. KSP-AVC uses its own custom character-by-character logic. :disappointed:

Aman-Anas commented 4 years ago

Is this still a planned addition, or is it more of on the backburner right now?

HebaruSan commented 4 years ago

@Aman-Anas, I personally haven't worked on it since my previous comment, and I'm not aware of anyone else having done so either. I guess it's on a back burner. If you're interested in having a try, go for it!