Skip to content

Implement #1359: collect telemetry#2283

Merged
tobiasdiez merged 9 commits into
JabRef:masterfrom
tobiasdiez:azureInsights
Apr 6, 2017
Merged

Implement #1359: collect telemetry#2283
tobiasdiez merged 9 commits into
JabRef:masterfrom
tobiasdiez:azureInsights

Conversation

@tobiasdiez

@tobiasdiez tobiasdiez commented Nov 18, 2016

Copy link
Copy Markdown
Member

In this PR, Microsoft Azure Application Insights is used to record the following information about how JabRef is used:

  • Number of users and sessions
  • Dialogs opened (so far only the About dialog is tracked, as proof of concept)
  • New database is opened, along with how many entries it contains
  • All exceptions are automatically recorded
  • A few user information (Country, OS, screen size, JabRef version, Java version)

If you give your ok, I will continue and implement the following:

  • Track all dialogs
  • Add "opt-out" preference

In #1359, we decided to use google analytics for this, but as Microsoft already provided a nice interface for java, I went for this route.

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@tobiasdiez tobiasdiez added the status: waiting-for-feedback The submitter or other users need to provide more information about the issue label Nov 18, 2016
@lenhard

lenhard commented Nov 18, 2016

Copy link
Copy Markdown
Member

Well, since it was decided in the devcall to implement this, it is ok for me if you go ahead. Regarding the choice of the tracking technology, I trust your skills. The code that is needed to track a dialog seems to be very simple in any case.

I am just not sure if an opt-out is the way to go, or if it should rather be an opt-in. Maybe we could display a dialog asking for consent on first startup. That would be more privacy-friendly in my point of view and we do not want JabRef to appear that it tries to grab user data. I am sure there would be enough people who are willing to contribute their data even in case of opt-in.

@matthiasgeiger

Copy link
Copy Markdown
Member

👍 for opt-in

<ApplicationInsights xmlns="http://schemas.microsoft.com/ApplicationInsights/2013/Settings" schemaVersion="2014-05-30">

<!-- The key from the Azure portal: -->
<InstrumentationKey>c2435fd3-cb59-423c-846b-04898b101cc1</InstrumentationKey>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably inject this later via an ENV variable?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is possible yes, but what is the advantage?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Security by obscurity. In that way, the key is not in the source, but in TravisCI environment variables, where only JabRef developers have access to by using "echo $key" in the travis.yml in a pull request. If we can somehow encrypt the JAR file, we can further prevent others from stealing the key.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done!

@stefan-kolb

stefan-kolb commented Nov 18, 2016

Copy link
Copy Markdown
Member

Nice that you give it a try 😄 👍 GA had no appropriate library?

@grimes2

grimes2 commented Nov 18, 2016

Copy link
Copy Markdown
Contributor

Why do you want to spy almost everything I do?

@lenhard

lenhard commented Nov 18, 2016

Copy link
Copy Markdown
Member

@grimes2 My guess (I haven't participated in the discussion on this) is that this would enable us to see which features are actually used and which are not. If we had had data on content selector usage, we would not have removed them. At the same time, we would be able to see if there is a point at all in optimizing JabRef usage for really large databases or if this is a waste of time.

@tobiasdiez tobiasdiez added after-migration-to-javafx and removed status: waiting-for-feedback The submitter or other users need to provide more information about the issue labels Dec 16, 2016
@tobiasdiez tobiasdiez added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed after-migration-to-javafx labels Mar 19, 2017
@tobiasdiez

tobiasdiez commented Mar 19, 2017

Copy link
Copy Markdown
Member Author

I added tracking of other dialogs and changed the default to "do not share telemetry data". Also, after the first minute of using JabRef, the user is asked whether he wants to share is data. I actually preferred to have a more non-invasive notification similar to
ie
JavaFX provides a NotificationPanel exactly for this scenario, however, it doesn't work if the rest of the application is in swing.

Anyway, this PR is now finally ready for your feedback!


For later reference (I will add this links also to the wiki):

@tobiasdiez tobiasdiez changed the title [WIP] Implement #1359: collect telemetry Implement #1359: collect telemetry Mar 19, 2017
@lenhard

lenhard commented Mar 31, 2017

Copy link
Copy Markdown
Member

I tried it out locally, the dialog appears as desired, and the option can be disabled via the preferences. Code-wise, I have no objections. However, JabRef does not terminate when I close it and I get the a lot of messages in the console, e.g.:

AI: TRACE 31-03-2017 13:39, 28: Telemetry Configuration: illegal instrumentation key: null
AI: TRACE 31-03-2017 13:39, 28: InProcessTelemetryChannel sending telemetry
AI: ERROR 31-03-2017 13:39, 44: Failed to send, Bad request  : {"itemsReceived":1,"itemsAccepted":0,"errors":[{"index":0,"statusCode":400,"message":"Invalid instrumentation key"}]}
AI: TRACE 31-03-2017 13:39, 28: Telemetry Configuration: illegal instrumentation key: null
AI: TRACE 31-03-2017 13:39, 28: InProcessTelemetryChannel sending telemetry
AI: ERROR 31-03-2017 13:39, 46: Failed to send, Bad request  : {"itemsReceived":1,"itemsAccepted":0,"errors":[{"index":0,"statusCode":400,"message":"Invalid instrumentation key"}]}

Since this is a very sensitive topic, I think we should make the dialog a little more explanatory. Currently it says

To improve the user experience, we would like to collect data on the features you use. No personal data will be collected.

While everything is in that statement, I think we should be more explicit and repeat ourselves. Suggestion:

To improve the user experience, we would like to collect anonymous statistics on the features you use. We will only record what features you access and how often you do it. We will neither collect any personal data nor the content of bibliographic items. The collected data will be stored on XXX and we will only use it to prioritize our development efforts. If you choose to allow data collection, you can later disable it via Options -> Preferences -> General

I know this is quite long, but since this is a delicate topic, I think it is necessary. Btw.: where is the data actually stored?

@JabRef/developers I think this is so important that everyone should consider it briefly.

And a last comment: We will get flack from the Linux freaks when they find out that we dare to include a microsoft library. So be prepared for it ;-)

@tobiasdiez

Copy link
Copy Markdown
Member Author

I like your suggestion for the dialog text and updated it accordingly. The data is store on Microsoft Azure server...no idea where exactly 😄 .

The reason for the error message is that a correct instrumentation key is only inserted by travis and thus you get these messages when you run JabRef from the IDE.

@lenhard

lenhard commented Mar 31, 2017

Copy link
Copy Markdown
Member

By the way, this is what I just got after installing soapUI:
soapui

Of course, they have a much more explicit privacy policy page for that.

@tobiasdiez tobiasdiez merged commit 36b5381 into JabRef:master Apr 6, 2017
@tobiasdiez tobiasdiez deleted the azureInsights branch April 6, 2017 14:51
Siedlerchr added a commit that referenced this pull request Apr 6, 2017
* upstream/master:
  Implement #1359: collect telemetry (#2283)
  Write groups under tag `group` instead of `groupstree` to enhance compatibility with previous versions (#2704)
  Avoid conversion of single underscores (#2711)
  Fix #628: implement hierarchical keywords (#2703)
@lenhard lenhard mentioned this pull request Apr 7, 2017
Siedlerchr added a commit that referenced this pull request Apr 13, 2017
* upstream/master: (39 commits)
  Fix fetcher test
  Allow failures for fetcher test (#2730)
  Use JabRefExecutor service
  Move DOI fetching to separate thread #2682
  Remove gui dependency in logic (#2726)
  Fixed freeze on Mac OS X when creating/editing groups (#2727)
  Only ask once if telemetry data should be collected
  Update wiremock from 2.5.1 to 2.6.0
  Update mockito-core from 2.7.21 to 2.7.22
  Update log4j to latest version
  Azure test (#2724)
  Fix build
  Move expand filename to FileUtil
  Unicode conversion bibtexkey (#2720)
  Add sorting of all groups and subgroups, recursively (#2666)
  Only check capitalization of note and howpublished fields if they start with a word character
  Remove overhauled @author tag
  Implement #1359: collect telemetry (#2283)
  Add licenses of new dependencies
  Fix cssStyleHelper warnings
  ...
@koppor koppor mentioned this pull request Oct 13, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants