Issue #5 Disable Telemetry #107
Conversation
jonbartels
force-pushed
the
issue-5-disable-telemetry
branch
from
381e341 to
d56b770
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR disables telemetry and user registration by completely removing the related backend and UI code.
- Removed telemetry scheduling and sending functionality from the server startup.
- Removed calls and UI components for user registration and telemetry from the client.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/src/com/mirth/connect/server/Mirth.java | Removed telemetry Timer scheduling and the UsageSenderTask. |
| server/src/com/mirth/connect/client/core/ConnectServiceUtil.java | Removed sendStatistics method and unused telemetry-related constants. |
| client/src/com/mirth/connect/client/ui/LoginPanel.java | Removed the call to sendUsageStatistics from the login handler. |
| client/src/com/mirth/connect/client/ui/Frame.java | Removed methods for user registration and telemetry sending. |
| client/src/com/mirth/connect/client/ui/FirstLoginDialog.java | Removed UI checkboxes and consent handling related to telemetry and registration. |
| client/src/com/mirth/connect/client/ui/FirstLoginDialog.form | Removed checkbox components related to registration from the form layout. |
Comments suppressed due to low confidence (1)
client/src/com/mirth/connect/client/ui/Frame.java:1948
- Since user registration and telemetry sending functionality have been removed, double-check that no lingering references or documentation remain calling these methods.
public void registerUser(final User user) {
server/src/com/mirth/connect/client/core/ConnectServiceUtil.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Jon Bartels <jon.bartels@teladochealth.com>
jonbartels
force-pushed
the
issue-5-disable-telemetry
branch
from
d56b770 to
5c3327d
Compare
mgaffigan
left a comment
mgaffigan
left a comment
There was a problem hiding this comment.
Questions:
- Are Postal Address, Organization, Role, and Industry useful outside the context of data collection?
- Should webadmin/WebContent/WEB-INF/jsp/index.jsp:310 be changed? (or maybe that should be on the branding PR?)
Related issues:
- I've opened #108 to try to avoid future merge conflicts due to incremental .gitignore edits
- This depends upon #79 to avoid leaking usage via notification checks. In the same milestone, though, so that shouldn't be an issue.
LGTM generally.
| import org.apache.commons.collections4.CollectionUtils; | ||
| import org.apache.commons.io.FileUtils; | ||
| import org.apache.commons.io.FilenameUtils; | ||
| import org.apache.commons.lang3.NotImplementedException; |
There was a problem hiding this comment.
Nit: Unused imports:
- org.apache.commons.lang3.NotImplementedException
- com.mirth.connect.client.core.ConnectServiceUtil
- com.mirth.connect.model.UpdateSettings
| logger.error(e); | ||
| } | ||
|
|
||
| configurationController.setStatus(ConfigurationController.STATUS_OK); |
There was a problem hiding this comment.
nit: Unused imports
- java.util.Timer
- java.util.TimerTask
- com.mirth.connect.client.core.ConnectServiceUtil
- com.mirth.connect.model.UpdateSettings
| private class UsageSenderTask extends TimerTask { | ||
| @Override | ||
| public void run() { | ||
| boolean isSent = ConnectServiceUtil.sendStatistics(configurationController.getServerId(), configurationController.getServerVersion(), true, usageController.createUsageStats(null), configurationController.getHttpsClientProtocols(), configurationController.getHttpsCipherSuites()); |
There was a problem hiding this comment.
This is the only use of usageController. Presumably this should kill the instantiation at :92. Should UsageController and UpdateSettings in entirety be killed?
| .addComponent(userConsentCheckBox) | ||
| .addPreferredGap(javax.swing.LayoutStyle.ComponentPlacement.RELATED, javax.swing.GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE) | ||
| .addComponent(contentTextPane) | ||
| .addPreferredGap(javax.swing.LayoutStyle.ComponentPlacement.RELATED, javax.swing.GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE) |
There was a problem hiding this comment.
:248 - :265 seems to store user information. Unclear of purpose. Might be removed.
|
Also see #25. The general idea when I opened #5 was to disable, not remove, so that it could be enabled downstream in the future. As discussed in discord, I liked your idea of having the methods which make the remote calls throw an |
|
I converted my PR to a draft. #115 is a narrower solution. |
|
Closing with the merge of #115 |
3 participants
Removed the code that sends:
The code was entirely REMOVED not just disabled or re-pointed to somewhere else.
This involved stripping the backend functions, then the UI actions that called them and finally the UI checkboxes and consent forms.
You can see the checkboxes are gone from the firstlogin screen:
I tested this locally on MacOS, ARM with Java 17.