Skip to content

Commit

Permalink
Metrics now include compute region if available (#170)
Browse files Browse the repository at this point in the history
  • Loading branch information
bryceitoc9 authored Nov 21, 2020
1 parent 7e89eb3 commit effb58a
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 22 deletions.
12 changes: 0 additions & 12 deletions src/shared/extensionUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
38 changes: 37 additions & 1 deletion src/shared/telemetry/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<string | undefined> {
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
}
7 changes: 3 additions & 4 deletions src/shared/telemetry/defaultTelemetryService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/fakeExtensionContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/test/globalSetup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
Expand Down
44 changes: 44 additions & 0 deletions src/test/shared/telemetry/activation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -17,6 +18,7 @@ import {
hasUserSeenTelemetryNotice,
setHasUserSeenTelemetryNotice,
sanitizeTelemetrySetting,
getComputeRegion,
} from '../../../shared/telemetry/activation'
import { DefaultSettingsConfiguration } from '../../../shared/settingsConfiguration'
import { extensionSettingsPrefix } from '../../../shared/constants'
Expand Down Expand Up @@ -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)
})
})
6 changes: 3 additions & 3 deletions src/test/shared/telemetry/defaultTelemetryService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
})

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit effb58a

Please sign in to comment.