Skip to content

Commit

Permalink
fix(cwl): LiveTail fails to start when clicking with Play next to a L…
Browse files Browse the repository at this point in the history 
…ogGroup #5986

## Problem
Starting a LiveTail session by clicking Play next to a specific LogGroup
in the explorer menu is failing. This is because the LogGroup name is
passed into the Wizard response, and not a fully qualified Arn.

StartLiveTail API request requires ARNs.

## Solution
Detect if the context when initializing the TailLogGroup wizard is a
LogGroup Name or Arn. If it is just a name, construct the Arn and set
that in the Wizard response.

Renames `LogGroupName` in `LiveTailSession` to `LogGroupArn` to make it
more clear what is expected.
  • Loading branch information
@keeganirby
keeganirby authored Nov 12, 2024
1 parent 47595e6 commit 5b80409
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 23 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export async function tailLogGroup(
}

const liveTailSessionConfig: LiveTailSessionConfiguration = {
logGroupName: wizardResponse.regionLogGroupSubmenuResponse.data,
logGroupArn: wizardResponse.regionLogGroupSubmenuResponse.data,
logStreamFilter: wizardResponse.logStreamFilter,
logEventFilterPattern: wizardResponse.filterPattern,
region: wizardResponse.regionLogGroupSubmenuResponse.region,
Expand Down
17 changes: 9 additions & 8 deletions packages/core/src/awsService/cloudWatchLogs/registry/liveTailSession.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import {
StartLiveTailResponseStream,
} from '@aws-sdk/client-cloudwatch-logs'
import { LogStreamFilterResponse } from '../wizard/liveTailLogStreamSubmenu'
import { CloudWatchLogsSettings } from '../cloudWatchLogsUtils'
import { convertToTimeString, globals, Settings, ToolkitError } from '../../../shared'
import { CloudWatchLogsSettings, uriToKey } from '../cloudWatchLogsUtils'
import { convertToTimeString, getLogger, globals, Settings, ToolkitError } from '../../../shared'
import { createLiveTailURIFromArgs } from './liveTailSessionRegistry'
import { getUserAgent } from '../../../shared/telemetry/util'

export type LiveTailSessionConfiguration = {
logGroupName: string
logGroupArn: string
logStreamFilter?: LogStreamFilterResponse
logEventFilterPattern?: string
region: string
Expand All @@ -28,7 +28,7 @@ export type LiveTailSessionClient = {

export class LiveTailSession {
private liveTailClient: LiveTailSessionClient
private _logGroupName: string
private _logGroupArn: string
private logStreamFilter?: LogStreamFilterResponse
private logEventFilterPattern?: string
private _maxLines: number
Expand All @@ -45,7 +45,7 @@ export class LiveTailSession {
static settings = new CloudWatchLogsSettings(Settings.instance)

public constructor(configuration: LiveTailSessionConfiguration) {
this._logGroupName = configuration.logGroupName
this._logGroupArn = configuration.logGroupArn
this.logStreamFilter = configuration.logStreamFilter
this.liveTailClient = {
cwlClient: new CloudWatchLogsClient({
Expand All @@ -69,8 +69,8 @@ export class LiveTailSession {
return this._uri
}

public get logGroupName() {
return this._logGroupName
public get logGroupArn() {
return this._logGroupArn
}

public set eventRate(rate: number) {
Expand All @@ -93,6 +93,7 @@ export class LiveTailSession {
this.statusBarUpdateTimer = globals.clock.setInterval(() => {
this.updateStatusBarItemText()
}, 500)
getLogger().info(`LiveTail session started: ${uriToKey(this.uri)}`)
return commandOutput.responseStream
}

Expand Down Expand Up @@ -130,7 +131,7 @@ export class LiveTailSession {
}

return new StartLiveTailCommand({
logGroupIdentifiers: [this.logGroupName],
logGroupIdentifiers: [this.logGroupArn],
logStreamNamePrefixes: logStreamNamePrefix ? [logStreamNamePrefix] : undefined,
logStreamNames: logStreamName ? [logStreamName] : undefined,
logEventFilterPattern: this.logEventFilterPattern ? this.logEventFilterPattern : undefined,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/awsService/cloudWatchLogs/registry/liveTailSessionRegistry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class LiveTailSessionRegistry extends Map<string, LiveTailSession> {
}

export function createLiveTailURIFromArgs(sessionData: LiveTailSessionConfiguration): vscode.Uri {
let uriStr = `${cloudwatchLogsLiveTailScheme}:${sessionData.region}:${sessionData.logGroupName}`
let uriStr = `${cloudwatchLogsLiveTailScheme}:${sessionData.region}:${sessionData.logGroupArn}`

if (sessionData.logStreamFilter) {
if (sessionData.logStreamFilter.type !== 'all') {
Expand Down
17 changes: 15 additions & 2 deletions packages/core/src/awsService/cloudWatchLogs/wizard/tailLogGroupWizard.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import * as nls from 'vscode-nls'
import { ToolkitError } from '../../../shared'
import { globals, ToolkitError } from '../../../shared'
import { DefaultCloudWatchLogsClient } from '../../../shared/clients/cloudWatchLogsClient'
import { cwlFilterPatternHelpUrl } from '../../../shared/constants'
import { createBackButton, createExitButton, createHelpButton } from '../../../shared/ui/buttons'
Expand All @@ -29,7 +29,7 @@ export class TailLogGroupWizard extends Wizard<TailLogGroupWizardResponse> {
initState: {
regionLogGroupSubmenuResponse: logGroupInfo
? {
data: logGroupInfo.groupName,
data: buildLogGroupArn(logGroupInfo.groupName, logGroupInfo.regionName),
region: logGroupInfo.regionName,
}
: undefined,
Expand Down Expand Up @@ -81,6 +81,19 @@ async function getLogGroupQuickPickOptions(regionCode: string): Promise<DataQuic
return logGroupsOptions
}

export function buildLogGroupArn(logGroupName: string, region: string): string {
if (logGroupName.startsWith('arn:')) {
return logGroupName
}
const awsAccountId = globals.awsContext.getCredentialAccountId()
if (awsAccountId === undefined) {
throw new ToolkitError(
`Failed to construct Arn for LogGroup because awsAccountId is undefined. LogGroup: ${logGroupName}`
)
}
return `arn:aws:logs:${region}:${awsAccountId}:log-group:${logGroupName}`
}

function formatLogGroupArn(logGroupArn: string): string {
return logGroupArn.endsWith(':*') ? logGroupArn.substring(0, logGroupArn.length - 2) : logGroupArn
}
Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/test/awsService/cloudWatchLogs/commands/tailLogGroup.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ import {
import { getTestWindow } from '../../../shared/vscode/window'
import { CloudWatchLogsSettings, uriToKey } from '../../../../awsService/cloudWatchLogs/cloudWatchLogsUtils'
import { installFakeClock } from '../../../testUtil'
import { DefaultAwsContext } from '../../../../shared'

describe('TailLogGroup', function () {
const testLogGroup = 'test-log-group'
const testRegion = 'test-region'
const testMessage = 'test-message'
const testAwsAccountId = '1234'

let sandbox: sinon.SinonSandbox
let registry: LiveTailSessionRegistry
Expand Down Expand Up @@ -54,6 +56,7 @@ describe('TailLogGroup', function () {
})

it('starts LiveTailSession and writes to document. Closes tab and asserts session gets closed.', async function () {
sandbox.stub(DefaultAwsContext.prototype, 'getCredentialAccountId').returns(testAwsAccountId)
wizardSpy = sandbox.stub(TailLogGroupWizard.prototype, 'run').callsFake(async function () {
return getTestWizardResponse()
})
Expand Down Expand Up @@ -127,7 +130,7 @@ describe('TailLogGroup', function () {
})

const session = new LiveTailSession({
logGroupName: testLogGroup,
logGroupArn: testLogGroup,
region: testRegion,
})
registry.set(uriToKey(session.uri), session)
Expand All @@ -140,7 +143,7 @@ describe('TailLogGroup', function () {

it('clearDocument clears all text from document', async function () {
const session = new LiveTailSession({
logGroupName: testLogGroup,
logGroupArn: testLogGroup,
region: testRegion,
})
const testData = 'blah blah blah'
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/test/awsService/cloudWatchLogs/registry/liveTailRegistry.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('LiveTailSession URI', async function () {

it('is correct with no logStream filter, no filter pattern', function () {
const config: LiveTailSessionConfiguration = {
logGroupName: testLogGroupName,
logGroupArn: testLogGroupName,
region: testRegion,
}
const expectedUri = vscode.Uri.parse(expectedUriBase)
Expand All @@ -25,7 +25,7 @@ describe('LiveTailSession URI', async function () {

it('is correct with no logStream filter, with filter pattern', function () {
const config: LiveTailSessionConfiguration = {
logGroupName: testLogGroupName,
logGroupArn: testLogGroupName,
region: testRegion,
logEventFilterPattern: 'test-filter',
}
Expand All @@ -36,7 +36,7 @@ describe('LiveTailSession URI', async function () {

it('is correct with ALL logStream filter', function () {
const config: LiveTailSessionConfiguration = {
logGroupName: testLogGroupName,
logGroupArn: testLogGroupName,
region: testRegion,
logStreamFilter: {
type: 'all',
Expand All @@ -49,7 +49,7 @@ describe('LiveTailSession URI', async function () {

it('is correct with prefix logStream filter', function () {
const config: LiveTailSessionConfiguration = {
logGroupName: testLogGroupName,
logGroupArn: testLogGroupName,
region: testRegion,
logStreamFilter: {
type: 'prefix',
Expand All @@ -63,7 +63,7 @@ describe('LiveTailSession URI', async function () {

it('is correct with specific logStream filter', function () {
const config: LiveTailSessionConfiguration = {
logGroupName: testLogGroupName,
logGroupArn: testLogGroupName,
region: testRegion,
logStreamFilter: {
type: 'specific',
Expand All @@ -77,7 +77,7 @@ describe('LiveTailSession URI', async function () {

it('is correct with specific logStream filter and filter pattern', function () {
const config: LiveTailSessionConfiguration = {
logGroupName: testLogGroupName,
logGroupArn: testLogGroupName,
region: testRegion,
logStreamFilter: {
type: 'specific',
Expand Down
30 changes: 27 additions & 3 deletions packages/core/src/test/awsService/cloudWatchLogs/wizard/tailLogGroupWizard.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,28 @@
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
import * as sinon from 'sinon'

import { TailLogGroupWizard } from '../../../../awsService/cloudWatchLogs/wizard/tailLogGroupWizard'
import assert from 'assert'
import { buildLogGroupArn, TailLogGroupWizard } from '../../../../awsService/cloudWatchLogs/wizard/tailLogGroupWizard'
import { createWizardTester } from '../../../shared/wizards/wizardTestUtils'
import { DefaultAwsContext } from '../../../../shared'

describe('TailLogGroupWizard', async function () {
let sandbox: sinon.SinonSandbox

const testLogGroupName = 'testLogGroup'
const testRegion = 'testRegion'
const testAwsAccountId = '1234'

beforeEach(function () {
sandbox = sinon.createSandbox()
})

afterEach(function () {
sandbox.restore()
})

it('prompts regionLogGroup submenu first if context not provided', async function () {
const wizard = new TailLogGroupWizard()
const tester = await createWizardTester(wizard)
Expand All @@ -16,13 +33,20 @@ describe('TailLogGroupWizard', async function () {
})

it('skips regionLogGroup submenu if context provided', async function () {
sandbox.stub(DefaultAwsContext.prototype, 'getCredentialAccountId').returns(testAwsAccountId)
const wizard = new TailLogGroupWizard({
groupName: 'test-groupName',
regionName: 'test-regionName',
groupName: testLogGroupName,
regionName: testRegion,
})
const tester = await createWizardTester(wizard)
tester.regionLogGroupSubmenuResponse.assertDoesNotShow()
tester.logStreamFilter.assertShowFirst()
tester.filterPattern.assertShowSecond()
})

it('builds LogGroup Arn properly', async function () {
sandbox.stub(DefaultAwsContext.prototype, 'getCredentialAccountId').returns(testAwsAccountId)
const arn = buildLogGroupArn(testLogGroupName, testRegion)
assert.strictEqual(arn, `arn:aws:logs:${testRegion}:${testAwsAccountId}:log-group:${testLogGroupName}`)
})
})

0 comments on commit 5b80409

Please sign in to comment.