From effb58ab3b06f789e44a62da8c09243e388227c5 Mon Sep 17 00:00:00 2001 From: Bryce Ito Date: Fri, 20 Nov 2020 18:40:23 -0800 Subject: [PATCH] Metrics now include compute region if available (#170) --- src/shared/extensionUtilities.ts | 12 ----- src/shared/telemetry/activation.ts | 38 +++++++++++++++- .../telemetry/defaultTelemetryService.ts | 7 ++- src/test/fakeExtensionContext.ts | 2 +- src/test/globalSetup.test.ts | 2 +- src/test/shared/telemetry/activation.test.ts | 44 +++++++++++++++++++ .../telemetry/defaultTelemetryService.test.ts | 6 +-- 7 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/shared/extensionUtilities.ts b/src/shared/extensionUtilities.ts index 36d648bc958..dbe59cf81c6 100644 --- a/src/shared/extensionUtilities.ts +++ b/src/shared/extensionUtilities.ts @@ -79,18 +79,6 @@ export function isCloud9(): boolean { return getIdeType() === IDE.cloud9 } -/** - * Returns the compute region (e.g. Cloud9 region) or 'not-regional' if used in a non-regional setting. - * TODO: Implement this function!!! - */ -export function getComputeRegion(): string | undefined { - if (isCloud9()) { - return 'not-implemented' - } - - return undefined -} - export class ExtensionUtilities { public static getLibrariesForHtml(names: string[], webview: vscode.Webview): vscode.Uri[] { const basePath = path.join(ext.context.extensionPath, 'media', 'libs') diff --git a/src/shared/telemetry/activation.ts b/src/shared/telemetry/activation.ts index a7f9e17da89..5a5561539cd 100644 --- a/src/shared/telemetry/activation.ts +++ b/src/shared/telemetry/activation.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { MetadataService } from 'aws-sdk' import * as vscode from 'vscode' import { AwsContext } from '../awsContext' import { SettingsConfiguration } from '../settingsConfiguration' @@ -12,6 +13,7 @@ import { getLogger } from '../logger' import { TelemetryService } from './telemetryService' import * as nls from 'vscode-nls' +import { isCloud9 } from '../extensionUtilities' const localize = nls.loadMessageBundle() const LEGACY_SETTINGS_TELEMETRY_VALUE_DISABLE = 'Disable' @@ -43,7 +45,13 @@ export async function activate(activateArguments: { awsContext: AwsContext toolkitSettings: SettingsConfiguration }) { - ext.telemetry = new DefaultTelemetryService(activateArguments.extensionContext, activateArguments.awsContext) + const computeRegion = await getComputeRegion() + + ext.telemetry = new DefaultTelemetryService( + activateArguments.extensionContext, + activateArguments.awsContext, + computeRegion + ) // Convert setting to boolean if it is not already await sanitizeTelemetrySetting(activateArguments.toolkitSettings) @@ -176,3 +184,31 @@ export async function handleTelemetryNoticeResponse( getLogger().error('Error while handling response from telemetry notice: %O', err as Error) } } + +/** + * Returns the Cloud9 compute region or 'unknown' if we can't pull a region, or `undefined` if this is not Cloud9. + */ +export async function getComputeRegion( + metadataService: MetadataService = new MetadataService(), + isC9: boolean = isCloud9() +): Promise { + if (isC9) { + try { + const identity: { [key: string]: string; region: string } = await new Promise((resolve, reject) => { + // MetadataService.request() doesn't support .promise()... + metadataService.request('/latest/dynamic/instance-identity/document', (err, data) => { + if (err) { + reject(err) + } + resolve(JSON.parse(data)) + }) + }) + + return identity.region || 'unknown' + } catch (e) { + return 'unknown' + } + } + // not cloud9, region has no meaning + return undefined +} diff --git a/src/shared/telemetry/defaultTelemetryService.ts b/src/shared/telemetry/defaultTelemetryService.ts index 8bfeeb292c7..bb4632e2e8d 100644 --- a/src/shared/telemetry/defaultTelemetryService.ts +++ b/src/shared/telemetry/defaultTelemetryService.ts @@ -9,7 +9,6 @@ import * as path from 'path' import { v4 as uuidv4 } from 'uuid' import { ExtensionContext } from 'vscode' import { AwsContext } from '../awsContext' -import { getComputeRegion } from '../extensionUtilities' import { getLogger } from '../logger' import { MetricDatum } from './clienttelemetry' import { DefaultTelemetryClient } from './defaultTelemetryClient' @@ -38,6 +37,7 @@ export class DefaultTelemetryService implements TelemetryService { public constructor( private readonly context: ExtensionContext, private readonly awsContext: AwsContext, + private readonly computeRegion?: string, publisher?: TelemetryPublisher ) { const persistPath = context.globalStoragePath @@ -231,9 +231,8 @@ export class DefaultTelemetryService implements TelemetryService { } const commonMetadata = [{ Key: ACCOUNT_METADATA_KEY, Value: accountValue }] - const computeRegion = getComputeRegion() - if (computeRegion) { - commonMetadata.push({ Key: COMPUTE_REGION_KEY, Value: computeRegion }) + if (this.computeRegion) { + commonMetadata.push({ Key: COMPUTE_REGION_KEY, Value: this.computeRegion }) } if (event.Metadata) { diff --git a/src/test/fakeExtensionContext.ts b/src/test/fakeExtensionContext.ts index 8207ad724ab..450975b9f80 100644 --- a/src/test/fakeExtensionContext.ts +++ b/src/test/fakeExtensionContext.ts @@ -86,7 +86,7 @@ export class FakeExtensionContext implements vscode.ExtensionContext { const outputChannel = new MockOutputChannel() const channelLogger = new FakeChannelLogger() const fakeTelemetryPublisher = new FakeTelemetryPublisher() - const telemetryService = new DefaultTelemetryService(ctx, awsContext, fakeTelemetryPublisher) + const telemetryService = new DefaultTelemetryService(ctx, awsContext, undefined, fakeTelemetryPublisher) return { extensionContext: ctx, awsContext: awsContext, diff --git a/src/test/globalSetup.test.ts b/src/test/globalSetup.test.ts index 2cc494cfe9c..f4214a18ebb 100644 --- a/src/test/globalSetup.test.ts +++ b/src/test/globalSetup.test.ts @@ -41,7 +41,7 @@ before(async () => { enqueue(...events: any[]) {}, async flush() {}, } - const service = new DefaultTelemetryService(mockContext, mockAws, mockPublisher) + const service = new DefaultTelemetryService(mockContext, mockAws, undefined, mockPublisher) ext.telemetry = service ext.templateRegistry = new CloudFormationTemplateRegistry() }) diff --git a/src/test/shared/telemetry/activation.test.ts b/src/test/shared/telemetry/activation.test.ts index 4d5b529072d..521d6ad3090 100644 --- a/src/test/shared/telemetry/activation.test.ts +++ b/src/test/shared/telemetry/activation.test.ts @@ -4,6 +4,7 @@ */ import * as assert from 'assert' +import { AWSError, MetadataService } from 'aws-sdk' import * as sinon from 'sinon' import * as vscode from 'vscode' @@ -17,6 +18,7 @@ import { hasUserSeenTelemetryNotice, setHasUserSeenTelemetryNotice, sanitizeTelemetrySetting, + getComputeRegion, } from '../../../shared/telemetry/activation' import { DefaultSettingsConfiguration } from '../../../shared/settingsConfiguration' import { extensionSettingsPrefix } from '../../../shared/constants' @@ -195,3 +197,45 @@ describe('hasUserSeenTelemetryNotice', async () => { }) }) }) + +describe('getComputeRegion', async () => { + let metadataService = new MetadataService() + + let sandbox: sinon.SinonSandbox + + before(() => { + sandbox = sinon.createSandbox() + }) + + afterEach(() => { + sandbox.restore() + }) + + it('returns a compute region', async () => { + sandbox.stub(metadataService, 'request').callsArgWith(1, undefined, '{"region": "us-weast-1"}') + + const val = await getComputeRegion(metadataService, true) + assert.strictEqual(val, 'us-weast-1') + }) + + it('returns "unknown" if cloud9 and the MetadataService request fails', async () => { + sandbox.stub(metadataService, 'request').callsArgWith(1, {} as AWSError, 'lol') + + const val = await getComputeRegion(metadataService, true) + assert.strictEqual(val, 'unknown') + }) + + it('returns "unknown" if cloud9 and can not find a region', async () => { + sandbox.stub(metadataService, 'request').callsArgWith(1, undefined, '{"legion": "d\'honneur"}') + + const val = await getComputeRegion(metadataService, true) + assert.strictEqual(val, 'unknown') + }) + + it('returns undefined if not cloud9', async () => { + sandbox.stub(metadataService, 'request').callsArgWith(1, undefined, 'lol') + + const val = await getComputeRegion(metadataService, false) + assert.strictEqual(val, undefined) + }) +}) diff --git a/src/test/shared/telemetry/defaultTelemetryService.test.ts b/src/test/shared/telemetry/defaultTelemetryService.test.ts index a5c84844a5f..6f0e3978dde 100644 --- a/src/test/shared/telemetry/defaultTelemetryService.test.ts +++ b/src/test/shared/telemetry/defaultTelemetryService.test.ts @@ -33,7 +33,7 @@ beforeEach(() => { mockContext = new FakeExtensionContext() mockAws = new FakeAwsContext() mockPublisher = new FakeTelemetryPublisher() - service = new DefaultTelemetryService(mockContext, mockAws, mockPublisher) + service = new DefaultTelemetryService(mockContext, mockAws, undefined, mockPublisher) ext.telemetry = service }) @@ -85,7 +85,7 @@ describe('DefaultTelemetryService', () => { it('events automatically inject the active account id into the metadata', async () => { const mockAwsWithIds = makeFakeAwsContextWithPlaceholderIds(({} as any) as AWS.Credentials) - service = new DefaultTelemetryService(mockContext, mockAwsWithIds, mockPublisher) + service = new DefaultTelemetryService(mockContext, mockAwsWithIds, undefined, mockPublisher) ext.telemetry = service service.clearRecords() service.telemetryEnabled = true @@ -124,7 +124,7 @@ describe('DefaultTelemetryService', () => { const mockAwsBad = ({ getCredentialAccountId: () => 'this is bad!', } as any) as AwsContext - service = new DefaultTelemetryService(mockContext, mockAwsBad, mockPublisher) + service = new DefaultTelemetryService(mockContext, mockAwsBad, undefined, mockPublisher) ext.telemetry = service service.clearRecords() service.telemetryEnabled = true