Skip to content

Respect "telemetry.telemetryLevel" as well as "telemetry.enableTelemetry" #2824

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.

Sign up for GitHub

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

Merged
robertbrignull merged 17 commits into from
Oct 9, 2023
Merged

Respect "telemetry.telemetryLevel" as well as "telemetry.enableTelemetry" #2824

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
90093fb
Rearrange settings so the telemetry settings are clearer
robertbrignull Sep 15, 2023
9c76ba3
Check both telemetry.telemetryLevel and telemetry.enableTelemetry
robertbrignull Sep 15, 2023
c7bd343
Use sendTelemetryErrorEvent when sending errors
robertbrignull Sep 15, 2023
c7a9337
Use .qualifiedName instead of writing out manually
robertbrignull Sep 15, 2023
f3e72a0
Delete unused relevantSettings field
robertbrignull Sep 15, 2023
a506276
Update custom telemetry setting description
robertbrignull Sep 15, 2023
af5547f
Use correct values of telemtetry level
robertbrignull Sep 15, 2023
5fc9a73
Add telemtry tag to settings
robertbrignull Sep 18, 2023
9ec6590
Use env.isTelemetryEnabled instead of checking configuration values o…
robertbrignull Sep 18, 2023
d403361
Use onDidChangeTelemetryEnabled instead of listening for config value…
robertbrignull Sep 18, 2023
177c770
Delete config values that we on longer read directly
robertbrignull Sep 18, 2023
48399a9
Merge branch 'main' into robertbrignull/telemetry
robertbrignull Sep 19, 2023
aad1fee
Stop mocking sendTelemetryException because we don't use that anywher…
robertbrignull Sep 19, 2023
ca96cdf
Mock sendTelemetryErrorEvent and check appropriately
robertbrignull Sep 19, 2023
4227ff6
Mock env.itTelemetryEnable
robertbrignull Sep 19, 2023
d499ff3
Merge branch 'main' into robertbrignull/telemetry
robertbrignull Oct 5, 2023
2b0bd84
Place setup before initialising listener
robertbrignull Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions extensions/ql-vscode/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,13 +446,20 @@
"type": "boolean",
"default": false,
"scope": "application",
"markdownDescription": "Specifies whether to send CodeQL usage telemetry. This setting AND the global `#telemetry.enableTelemetry#` setting must be checked for telemetry to be sent to GitHub. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code)"
"markdownDescription": "Specifies whether to send CodeQL usage telemetry. This setting AND the one of the global telemetry settings (`#telemetry.enableTelemetry#` or `#telemetry.telemetryLevel#`) must be enabled for telemetry to be sent to GitHub. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code)",
"tags": [
"telemetry",
"usesOnlineServices"
]
},
"codeQL.telemetry.logTelemetry": {
"type": "boolean",
"default": false,
"scope": "application",
"description": "Specifies whether or not to write telemetry events to the extension log."
"description": "Specifies whether or not to write telemetry events to the extension log.",
"tags": [
"telemetry"
]
}
}
}
Expand Down
19 changes: 9 additions & 10 deletions extensions/ql-vscode/src/common/vscode/telemetry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import {
Extension,
ExtensionContext,
ConfigurationChangeEvent,
env,
} from "vscode";
import TelemetryReporter from "vscode-extension-telemetry";
import {
ConfigListener,
CANARY_FEATURES,
ENABLE_TELEMETRY,
GLOBAL_ENABLE_TELEMETRY,
LOG_TELEMETRY,
isIntegrationTestMode,
isCanary,
Expand Down Expand Up @@ -59,8 +59,6 @@ export class ExtensionTelemetryListener
extends ConfigListener
implements AppTelemetry
{
static relevantSettings = [ENABLE_TELEMETRY, CANARY_FEATURES];

private reporter?: TelemetryReporter;

private cliVersionStr = NOT_SET_CLI_VERSION;
Expand All @@ -72,6 +70,10 @@ export class ExtensionTelemetryListener
private readonly ctx: ExtensionContext,
) {
super();

env.onDidChangeTelemetryEnabled(async () => {
await this.initialize();
});
}

/**
Expand All @@ -91,18 +93,15 @@ export class ExtensionTelemetryListener
async handleDidChangeConfiguration(
e: ConfigurationChangeEvent,
): Promise<void> {
if (
e.affectsConfiguration("codeQL.telemetry.enableTelemetry") ||
e.affectsConfiguration("telemetry.enableTelemetry")
) {
if (e.affectsConfiguration(ENABLE_TELEMETRY.qualifiedName)) {
await this.initialize();
}

// Re-request telemetry so that users can see the dialog again.
// Re-request if codeQL.canary is being set to `true` and telemetry
// is not currently enabled.
if (
e.affectsConfiguration("codeQL.canary") &&
e.affectsConfiguration(CANARY_FEATURES.qualifiedName) &&
CANARY_FEATURES.getValue() &&
!ENABLE_TELEMETRY.getValue()
) {
Expand Down Expand Up @@ -212,7 +211,7 @@ export class ExtensionTelemetryListener
properties.stack = error.stack;
}

this.reporter.sendTelemetryEvent("error", properties, {});
this.reporter.sendTelemetryErrorEvent("error", properties, {});
}

/**
Expand All @@ -224,7 +223,7 @@ export class ExtensionTelemetryListener
// if global telemetry is disabled, avoid showing the dialog or making any changes
let result = undefined;
if (
GLOBAL_ENABLE_TELEMETRY.getValue() &&
env.isTelemetryEnabled &&
// Avoid showing the dialog if we are in integration test mode.
!isIntegrationTestMode()
) {
Expand Down
20 changes: 7 additions & 13 deletions extensions/ql-vscode/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,15 @@ export const VSCODE_SAVE_BEFORE_START_SETTING = new Setting(

const ROOT_SETTING = new Setting("codeQL");

// Global configuration
// Telemetry configuration
const TELEMETRY_SETTING = new Setting("telemetry", ROOT_SETTING);
const AST_VIEWER_SETTING = new Setting("astViewer", ROOT_SETTING);
const CONTEXTUAL_QUERIES_SETTINGS = new Setting(
"contextualQueries",
ROOT_SETTING,
);
const GLOBAL_TELEMETRY_SETTING = new Setting("telemetry");
const LOG_INSIGHTS_SETTING = new Setting("logInsights", ROOT_SETTING);

export const LOG_TELEMETRY = new Setting("logTelemetry", TELEMETRY_SETTING);
export const ENABLE_TELEMETRY = new Setting(
"enableTelemetry",
TELEMETRY_SETTING,
);

export const GLOBAL_ENABLE_TELEMETRY = new Setting(
"enableTelemetry",
GLOBAL_TELEMETRY_SETTING,
);

// Distribution configuration
const DISTRIBUTION_SETTING = new Setting("cli", ROOT_SETTING);
export const CUSTOM_CODEQL_PATH_SETTING = new Setting(
Expand Down Expand Up @@ -475,6 +463,7 @@ export function allowCanaryQueryServer() {
return value === undefined ? true : !!value;
}

const LOG_INSIGHTS_SETTING = new Setting("logInsights", ROOT_SETTING);
export const JOIN_ORDER_WARNING_THRESHOLD = new Setting(
"joinOrderWarningThreshold",
LOG_INSIGHTS_SETTING,
Expand All @@ -484,6 +473,7 @@ export function joinOrderWarningThreshold(): number {
return JOIN_ORDER_WARNING_THRESHOLD.getValue<number>();
}

const AST_VIEWER_SETTING = new Setting("astViewer", ROOT_SETTING);
/**
* Hidden setting: Avoids caching in the AST viewer if the user is also a canary user.
*/
Expand All @@ -492,6 +482,10 @@ export const NO_CACHE_AST_VIEWER = new Setting(
AST_VIEWER_SETTING,
);

const CONTEXTUAL_QUERIES_SETTINGS = new Setting(
"contextualQueries",
ROOT_SETTING,
);
/**
* Hidden setting: Avoids caching in jump to def and find refs contextual queries if the user is also a canary user.
*/
Expand Down