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

Automate Stack Trace Transmission to Devs #108

Closed
rkathey opened this issue Jun 27, 2016 · 19 comments
Closed

Automate Stack Trace Transmission to Devs #108

rkathey opened this issue Jun 27, 2016 · 19 comments
Labels
feature Adding functionality that adds value low Low priority bug/enhancement

Comments

@rkathey
Copy link
Contributor

rkathey commented Jun 27, 2016

Currently on a stack trace, MapTool reports it in a window with instructions to report it to the MapTool developers. The requested functionality is to automate sending of the stack trace to a location for review by the Dev team, including OS and Java Version, machine state (CPU, mem, disk space, etc.) and MapTool log files.

The functionality would need to include the ability to prevent spamming (see Az).

@rkathey rkathey added the feature Adding functionality that adds value label Jun 27, 2016
@rkathey rkathey added this to the 1.4.3.x Dev Build milestone Jun 27, 2016
@Azhrei
Copy link
Member

Azhrei commented Jun 27, 2016

Probably a two-phase upload. MT contacts the server and says, "Hi. I'm MT 1.4.1. I'd like to upload a crash log that's X bytes in size." The server can check that the message hash is correct and respond, "Ok. POST it to this URL." This will be a unpublished URL with a random number in the query string. Only valid, preauthorized random numbers will be allowed (meaning the first exchange has to work for the second to be valid).

I'l do this in PHP on the server side. Shouldn't be very tough. We can check for duplicates at this time as well. Might even be able to check for duplicates before uploading by having MT calculate a hash of the stack trace and sending that to the server; if the hash is already registered, we can tell MT that it's already been filed. Probably should hash it using the MT version number, since a bug that appears in two separate versions is something that should prod another look. :)

We should grab everything in the Help>Debug panel, but with any personal information scrubbed (usernames, home directory?).

How do we want the information going to developers? Do we dump the log in a directory on the server and post a message to Slack or Github or something? Or maybe there should be a WordPress interface so that the UI is a little nicer than a directory listing? Do we want to make it visible to users in a way that they could file an issue in GitHub and then link to the stack trace? Do you think our users would go through that effort?

@JamzTheMan
Copy link
Member

I like this approach. My opinion would be to open a GitHub issue for it, with a link to the zip of logs/info. If personal info is scrubbed a public link shouldn't be an issue. If the same stackTrace is already submitted, add a comment to the issue for the log link. Add tags to the issue for version # and OS.
You can then report back the user with a URL to the github issue. They don't need a login to see them, only to comment.

@JamzTheMan
Copy link
Member

FYI there is a php lib for the github api

@Azhrei
Copy link
Member

Azhrei commented Jun 27, 2016

Okay.

I'm thinking the uploaded zip file should contain a text file with the stack trace and a separate file with the contents of the debug panel. I think the checksum for the file inside the zip should be enough to uniquely identify the content and I don't need to calculate a separate checksum/hash on the content.

Lastly, the MT version number may not be enough when there are betas floating around; for example, if I use a custom build for a home game. Is there a (simple) way to put the git commit hash somewhere that MT can get to it? Then MT can report it's version number but also it's git commit entry to be really specific... It could be the same properties file as the MT version and other particulars...?

@JamzTheMan
Copy link
Member

Gradle could insert the commit into a file but of course, we could never guarantee you were not building from eclipse without committing...

We should only enable reporting on production versions? Custom builds should do their own debugging?

@JamzTheMan
Copy link
Member

We also talked about splitting dev builds into a separate build/install so those would be unstable production builds and should be reported and we can tag them as such.

@rkathey
Copy link
Contributor Author

rkathey commented Jun 27, 2016

In my opinion, changing build methodologies between dev, test and prod is bad juju

@JamzTheMan
Copy link
Member

Eh? I'm just talking about allowing dev build to install next to prod version with separate settings/.maptool folder

@rkathey
Copy link
Contributor Author

rkathey commented Jun 27, 2016

I really hope I can edit that comment when I get home.

So same build for dev/test release to non-devs. That's good.

@JamzTheMan
Copy link
Member

Using octodroid? U can edit...
And ya. I was just stating that a custom/interim build should not report.

@rkathey
Copy link
Contributor Author

rkathey commented Jun 27, 2016

Agree completely. Also, it would be good if the various forks out there didn't report when someone forks for their own use/product. For that matter, it would be nice if those didn't report in server stats either.

@JamzTheMan
Copy link
Member

Craig will have to sign the official releases.

@Azhrei
Copy link
Member

Azhrei commented Jun 28, 2016

They don't need to be signed, we just need to use a hash digest that uses a seed that only we know. If any other version of MT were to connect, our server side tool would reject it as invalid.

@JamzTheMan JamzTheMan removed this from the 1.4.3.x Dev Build milestone Feb 26, 2019
@JamzTheMan JamzTheMan added 3 low Low priority bug/enhancement labels Feb 26, 2019
@JamzTheMan
Copy link
Member

@Azhrei & @cwisniew
Going through some older issues and this reminded me...

Currently I use https://sentry.io for Nerps. Granted there is some noise as it does get a lot of stack traces I haven't had time to look into but anyhoo...

What do you guys think about using it for RPTools repo? Currently 1.5 IS setup to be able to use this. Currently there is a .properties file that holds a DSN for a sentry.io account and this is blank for now (so it will NOT send any info)

To enable it, we would need to create a new account for RPTools. And I would think, probably at least a popup or something to ask the user? Would a "checkbox" on the existing stacktrace window, default to off, be good enough and save this checkbox to AppPreferences?

@Azhrei
Copy link
Member

Azhrei commented Feb 26, 2019

Would it be logging only exceptions? If so, the we wouldn't need a pop up, IMO. Just the checkbox that says, "send to the development team from now on" or something. When they close the panel, send it if the checkbox is turned on. Is that what you were thinking of?

@JamzTheMan
Copy link
Member

Yeah. Right now it logs the stack traces but also sends the server settings so I think we get maybe the PC name? We also get OS and the client name as name@rptools.net as the email.

And ya, was thinking just that, a send to Dev team check box.

FYI sentry.io can create GitHub issues from the stack traces. It's pretty slick.

You get like 5000 a month IIRC and has spike protection.

@Azhrei
Copy link
Member

Azhrei commented Feb 26, 2019

I think that'd be great. Maybe add a textarea so that the user can describe what they were doing when the crash occurred? I would think the stack trace would be enough in most cases, but sometimes we get those traces that are results of an expose event in the GUI and then the trace is pretty much useless; user input would be necessary.

Is there any value in feeding the DSN to MapTool from our web site? So when someone runs MT, it connects to RPTools.net and we give them a DSN. I can't imagine we'll hit 5k/month (at least, I certainly hope we don't!), but if it happens, it might be nice to switch other events to another DSN. If we had two or three, we could round-robin the one we hand out to MT at startup, meaning we'd have 15k at that point.

And maybe we should start keeping a history of the last few error messages and including them in the report. Whenever MapTool.showError() is invoked, just add to a list. The macro engine should maybe do the same thing...?

Anyway, short term we turn it on and use it. Long term, we consider the other stuff. :)

@JamzTheMan
Copy link
Member

I like that idea, a text box would be a good addition.

IIRC it will also send and stack traces that gets logged (it's controlled by the log4j2 config)

And I have it so Dev mode doesn't log to sentry, only production.

I think if we get over 5k it's probably the same stack over and over so it should inspire is to fix it? :)

@JamzTheMan JamzTheMan removed the 3 label Sep 6, 2019
@thelsing
Copy link
Collaborator

We have sentry enabled. So we can close this I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding functionality that adds value low Low priority bug/enhancement
Projects
None yet
Development

No branches or pull requests

4 participants