Skip to content

Commit 6bb16d6

Browse files
vogellavogella
committed
Leave bundle lifecycle events out of what is recorded
Bundle activation was more than four fifths of everything recorded and says little about how anyone works. Filter it out by default, with a checkbox in the upload preview to bring it back. Filters now run before an event is buffered, so what they drop is never written. A filter that throws lets the event through. Guard the bundle id: the log monitor records events without one, so the "org.eclipse only" filter dereferenced null on every logged status.
1 parent 430fa83 commit 6bb16d6

9 files changed

Lines changed: 201 additions & 5 deletions

File tree

‎org.eclipse.epp.usagedata.recording.tests/src/org/eclipse/epp/usagedata/internal/recording/RecorderFilteringTests.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2008 The Eclipse Foundation.
3+
* All rights reserved. This program and the accompanying materials
4+
* are made available under the terms of the Eclipse Public License v1.0
5+
* which accompanies this distribution, and is available at
6+
* http://www.eclipse.org/legal/epl-v10.html
7+
*
8+
* Contributors:
9+
* The Eclipse Foundation - initial API and implementation
10+
*******************************************************************************/
11+
package org.eclipse.epp.usagedata.internal.recording;
12+
13+
import static org.junit.jupiter.api.Assertions.assertFalse;
14+
import static org.junit.jupiter.api.Assertions.assertTrue;
15+
16+
import org.eclipse.epp.usagedata.internal.gathering.events.UsageDataEvent;
17+
import org.eclipse.epp.usagedata.internal.recording.filtering.UsageDataEventFilter;
18+
import org.eclipse.epp.usagedata.internal.recording.settings.UsageDataRecordingSettings;
19+
import org.junit.jupiter.api.Test;
20+
21+
/**
22+
* These tests confirm that the recorder asks the filter before an event is
23+
* buffered, so that what the filter leaves out never reaches the disk.
24+
*/
25+
public class RecorderFilteringTests {
26+
27+
@Test
28+
public void testEventTheFilterRejectsIsNotRecorded() {
29+
UsageDataRecorder recorder = recorderFiltering(event -> false);
30+
31+
assertFalse(recorder.isIncluded(event()));
32+
}
33+
34+
@Test
35+
public void testEventTheFilterAcceptsIsRecorded() {
36+
UsageDataRecorder recorder = recorderFiltering(event -> true);
37+
38+
assertTrue(recorder.isIncluded(event()));
39+
}
40+
41+
@Test
42+
public void testEventIsRecordedWhenTheFilterThrows() {
43+
UsageDataRecorder recorder = recorderFiltering(event -> {
44+
throw new IllegalStateException("the filter is broken"); //$NON-NLS-1$
45+
});
46+
47+
assertTrue(recorder.isIncluded(event()));
48+
}
49+
50+
@Test
51+
public void testEventIsRecordedWhenThereAreNoSettings() {
52+
UsageDataRecorder recorder = new UsageDataRecorder() {
53+
@Override
54+
protected UsageDataRecordingSettings getSettings() {
55+
return null;
56+
}
57+
};
58+
59+
assertTrue(recorder.isIncluded(event()));
60+
}
61+
62+
private UsageDataEvent event() {
63+
return new UsageDataEvent("started", "bundle", "org.eclipse.core", "org.eclipse.core", "1.0.0", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
64+
System.currentTimeMillis());
65+
}
66+
67+
private UsageDataRecorder recorderFiltering(Decision decision) {
68+
final UsageDataRecordingSettings settings = new UsageDataRecordingSettings() {
69+
@Override
70+
public UsageDataEventFilter getFilter() {
71+
return new StubFilter(decision);
72+
}
73+
};
74+
return new UsageDataRecorder() {
75+
@Override
76+
protected UsageDataRecordingSettings getSettings() {
77+
return settings;
78+
}
79+
};
80+
}
81+
82+
private interface Decision {
83+
boolean includes(UsageDataEvent event);
84+
}
85+
86+
private static class StubFilter implements UsageDataEventFilter {
87+
private final Decision decision;
88+
89+
StubFilter(Decision decision) {
90+
this.decision = decision;
91+
}
92+
93+
public boolean includes(UsageDataEvent event) {
94+
return decision.includes(event);
95+
}
96+
97+
public void addFilterChangeListener(org.eclipse.epp.usagedata.internal.recording.filtering.FilterChangeListener listener) {
98+
}
99+
100+
public void removeFilterChangeListener(org.eclipse.epp.usagedata.internal.recording.filtering.FilterChangeListener listener) {
101+
}
102+
}
103+
}

‎org.eclipse.epp.usagedata.recording.tests/src/org/eclipse/epp/usagedata/internal/recording/filtering/PreferencesBasedFilterTests.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,37 @@ public void setup() {
4141
filter = new PreferencesBasedFilter();
4242
getPreferencesStore().setToDefault(UsageDataRecordingSettings.FILTER_ECLIPSE_BUNDLES_ONLY_KEY);
4343
getPreferencesStore().setToDefault(UsageDataRecordingSettings.FILTER_PATTERNS_KEY);
44+
getPreferencesStore().setToDefault(UsageDataRecordingSettings.FILTER_BUNDLE_EVENTS_KEY);
45+
}
46+
47+
@Test
48+
public void testBundleEventsAreFilteredByDefault() {
49+
assertTrue(filter.isBundleEventsFiltered());
50+
assertFalse(filter.includes(createBundleEvent("org.eclipse.core")));
51+
}
52+
53+
@Test
54+
public void testFilteringBundleEventsKeepsEverythingElse() {
55+
assertTrue(filter.includes(eclipseEvent1));
56+
assertTrue(filter.includes(nonEclipseEvent));
57+
}
58+
59+
@Test
60+
public void testBundleEventsAreIncludedWhenNotFiltered() {
61+
filter.setBundleEventsFiltered(false);
62+
63+
assertTrue(filter.includes(createBundleEvent("org.eclipse.core")));
64+
}
65+
66+
@Test
67+
public void testBundleEventFilterBeatsTheEclipseOnlyFilter() {
68+
filter.setEclipseOnly(true);
69+
70+
assertFalse(filter.includes(createBundleEvent("org.eclipse.core")));
71+
}
72+
73+
private UsageDataEvent createBundleEvent(String bundleId) {
74+
return new UsageDataEvent("started", "bundle", bundleId, bundleId, "version", System.currentTimeMillis());
4475
}
4576

4677
@AfterEach

‎org.eclipse.epp.usagedata.recording/src/org/eclipse/epp/usagedata/internal/recording/UsageDataRecorder.java‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.eclipse.core.runtime.IStatus;
2121
import org.eclipse.epp.usagedata.internal.gathering.events.UsageDataEvent;
2222
import org.eclipse.epp.usagedata.internal.gathering.events.UsageDataEventListener;
23+
import org.eclipse.epp.usagedata.internal.recording.filtering.UsageDataEventFilter;
2324
import org.eclipse.epp.usagedata.internal.recording.settings.UsageDataRecordingSettings;
2425
import org.eclipse.epp.usagedata.internal.recording.uploading.BasicUploader;
2526
import org.eclipse.epp.usagedata.internal.recording.uploading.UploadManager;
@@ -82,14 +83,32 @@ public synchronized void stop() {
8283
public synchronized void accept(UsageDataEvent event) {
8384
if (event == null) return;
8485
if (!canAcceptEvents()) return;
85-
86+
8687
if (!running) return;
88+
if (!isIncluded(event)) return;
8789
events.add(event);
88-
90+
8991
if (events.size() >= EVENT_COUNT_THRESHOLD) dumpEvents();
90-
92+
9193
uploadDataIfNecessary();
9294
}
95+
96+
/**
97+
* This method applies the filter before the event is buffered, so that what
98+
* the filter leaves out is never written to disk in the first place. A
99+
* filter that throws is treated as letting the event through: dropping data
100+
* because a filter misbehaved would be the worse of the two failures.
101+
*/
102+
boolean isIncluded(UsageDataEvent event) {
103+
if (getSettings() == null) return true;
104+
UsageDataEventFilter filter = getSettings().getFilter();
105+
if (filter == null) return true;
106+
try {
107+
return filter.includes(event);
108+
} catch (RuntimeException e) {
109+
return true;
110+
}
111+
}
93112

94113
protected void uploadDataIfNecessary() {
95114
if (getSettings() == null) return;

‎org.eclipse.epp.usagedata.recording/src/org/eclipse/epp/usagedata/internal/recording/filtering/PreferencesBasedFilter.java‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@
3333
*/
3434
public class PreferencesBasedFilter extends AbstractUsageDataEventFilter {
3535

36+
/** The kind recorded for every OSGi bundle lifecycle change. */
37+
private static final String BUNDLE_KIND = "bundle"; //$NON-NLS-1$
38+
3639
public PreferencesBasedFilter() {
3740
hookListeners();
3841
}
@@ -62,15 +65,19 @@ public void dispose() {
6265
boolean isFilterProperty(String property) {
6366
if (UsageDataRecordingSettings.FILTER_ECLIPSE_BUNDLES_ONLY_KEY.equals(property)) return true;
6467
if (UsageDataRecordingSettings.FILTER_PATTERNS_KEY.equals(property)) return true;
68+
if (UsageDataRecordingSettings.FILTER_BUNDLE_EVENTS_KEY.equals(property)) return true;
6569
return false;
6670
}
6771

6872
public boolean includes(UsageDataEvent event) {
73+
if (isBundleEventsFiltered() && BUNDLE_KIND.equals(event.kind)) return false;
74+
// Not every event comes from a bundle; the log monitor reports none.
75+
String bundleId = event.bundleId == null ? "" : event.bundleId; //$NON-NLS-1$
6976
if (includeOnlyEclipseDotOrgBundles()) {
70-
return event.bundleId.startsWith("org.eclipse."); //$NON-NLS-1$
77+
return bundleId.startsWith("org.eclipse."); //$NON-NLS-1$
7178
}
7279
for (String filter : getFilterPatterns()) {
73-
if (matches(filter, event.bundleId)) return false;
80+
if (matches(filter, bundleId)) return false;
7481
}
7582
return true;
7683
}
@@ -141,5 +148,14 @@ public boolean isEclipseOnly() {
141148
return getPreferenceStore().getBoolean(UsageDataRecordingSettings.FILTER_ECLIPSE_BUNDLES_ONLY_KEY);
142149
}
143150

151+
public void setBundleEventsFiltered(boolean value) {
152+
getPreferenceStore().setValue(UsageDataRecordingSettings.FILTER_BUNDLE_EVENTS_KEY, value);
153+
UsageDataRecordingActivator.getDefault().savePluginPreferences();
154+
}
155+
156+
public boolean isBundleEventsFiltered() {
157+
return getPreferenceStore().getBoolean(UsageDataRecordingSettings.FILTER_BUNDLE_EVENTS_KEY);
158+
}
159+
144160

145161
}

‎org.eclipse.epp.usagedata.recording/src/org/eclipse/epp/usagedata/internal/recording/settings/UsageDataRecordingPreferenceInitializer.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public void initializeDefaultPreferences() {
2323
preferenceStore.setDefault(UsageDataRecordingSettings.ASK_TO_UPLOAD_KEY, UsageDataRecordingSettings.ASK_TO_UPLOAD_DEFAULT);
2424
preferenceStore.setDefault(UsageDataRecordingSettings.UPLOAD_MODE_KEY, UsageDataRecordingSettings.UPLOAD_MODE_DEFAULT);
2525
preferenceStore.setDefault(UsageDataRecordingSettings.FILTER_ECLIPSE_BUNDLES_ONLY_KEY, false);
26+
preferenceStore.setDefault(UsageDataRecordingSettings.FILTER_BUNDLE_EVENTS_KEY, UsageDataRecordingSettings.FILTER_BUNDLE_EVENTS_DEFAULT);
2627
}
2728

2829
}

‎org.eclipse.epp.usagedata.recording/src/org/eclipse/epp/usagedata/internal/recording/settings/UsageDataRecordingSettings.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public class UsageDataRecordingSettings implements UploadSettings {
5454
public static final String LOG_SERVER_ACTIVITY_KEY = UsageDataRecordingActivator.PLUGIN_ID + ".log-server"; //$NON-NLS-1$
5555
public static final String FILTER_ECLIPSE_BUNDLES_ONLY_KEY = UsageDataRecordingActivator.PLUGIN_ID + ".filter-eclipse-only"; //$NON-NLS-1$
5656
public static final String FILTER_PATTERNS_KEY = UsageDataRecordingActivator.PLUGIN_ID + ".filter-patterns"; //$NON-NLS-1$
57+
public static final String FILTER_BUNDLE_EVENTS_KEY = UsageDataRecordingActivator.PLUGIN_ID + ".filter-bundle-events"; //$NON-NLS-1$
5758

5859
public static final String UPLOAD_URL_KEY = UsageDataRecordingActivator.PLUGIN_ID + ".upload-url"; //$NON-NLS-1$
5960

@@ -92,6 +93,14 @@ public class UsageDataRecordingSettings implements UploadSettings {
9293
*/
9394
public static final long UPLOAD_DIRECTORY_MAX_BYTES = 10L * 1024L * 1024L;
9495

96+
/**
97+
* Bundle lifecycle events are filtered out of uploads by default. Starting
98+
* an IDE reports every bundle it activates, which is the great majority of
99+
* what gets recorded and says far less about how anyone works than the
100+
* commands, editors and views it buries.
101+
*/
102+
public static final boolean FILTER_BUNDLE_EVENTS_DEFAULT = true;
103+
95104
private int consecutiveFailedAttempts = 0;
96105

97106
private PreferencesBasedFilter filter = new PreferencesBasedFilter();

‎org.eclipse.epp.usagedata.ui/src/org/eclipse/epp/usagedata/internal/ui/preview/Messages.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ private Messages() {
3838
public static String UploadPreview_7;
3939
public static String UploadPreview_8;
4040
public static String UploadPreview_9;
41+
public static String UploadPreview_12;
4142

4243
static {
4344
NLS.initializeMessages(BUNDLE_NAME, Messages.class);

‎org.eclipse.epp.usagedata.ui/src/org/eclipse/epp/usagedata/internal/ui/preview/UploadPreview.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ public class UploadPreview {
8888

8989
private Button eclipseOnlyButton;
9090

91+
private Button bundleEventsButton;
92+
9193
private Button addFilterButton;
9294

9395
public UploadPreview(UploadParameters parameters) {
@@ -102,6 +104,7 @@ public Control createControl(final Composite parent) {
102104

103105
createDescriptionText(composite);
104106
createEventsTable(composite);
107+
createBundleEventsButton(composite);
105108
createEclipseOnlyButton(composite);
106109
createButtons(composite);
107110

@@ -318,6 +321,17 @@ public void widgetDisposed(DisposeEvent e) {
318321
updateButtons();
319322
}
320323

324+
private void createBundleEventsButton(Composite buttons) {
325+
bundleEventsButton = new Button(buttons, SWT.CHECK);
326+
bundleEventsButton.setText(Messages.UploadPreview_12);
327+
bundleEventsButton.addSelectionListener(new SelectionAdapter() {
328+
@Override
329+
public void widgetSelected(SelectionEvent e) {
330+
((PreferencesBasedFilter)parameters.getFilter()).setBundleEventsFiltered(bundleEventsButton.getSelection());
331+
}
332+
});
333+
}
334+
321335
private void createEclipseOnlyButton(Composite buttons) {
322336
eclipseOnlyButton = new Button(buttons, SWT.CHECK);
323337
eclipseOnlyButton.setText(Messages.UploadPreview_9);
@@ -332,6 +346,7 @@ public void widgetSelected(SelectionEvent e) {
332346
private void updateButtons() {
333347
if (parameters.getFilter() instanceof PreferencesBasedFilter) {
334348
PreferencesBasedFilter filter = (PreferencesBasedFilter)parameters.getFilter();
349+
bundleEventsButton.setSelection(filter.isBundleEventsFiltered());
335350
if (filter.isEclipseOnly()) {
336351
eclipseOnlyButton.setSelection(true);
337352
addFilterButton.setEnabled(false);

‎org.eclipse.epp.usagedata.ui/src/org/eclipse/epp/usagedata/internal/ui/preview/messages.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,4 @@ UploadPreview_6=Bundle Id
1414
UploadPreview_7=Version
1515
UploadPreview_8=When
1616
UploadPreview_9=Only upload events from "org.eclipse" bundles
17+
UploadPreview_12=Leave out bundle start and stop events

0 commit comments

Comments
 (0)