Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions packages/core/src/awsService/cloudWatchLogs/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import * as vscode from 'vscode'
import { CLOUDWATCH_LOGS_SCHEME } from '../../shared/constants'
import { CLOUDWATCH_LOGS_LIVETAIL_SCHEME, CLOUDWATCH_LOGS_SCHEME } from '../../shared/constants'
Copy link
Contributor

@justinmk3 justinmk3 Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename CLOUDWATCH_LOGS_LIVETAIL_SCHEME to conform to the lint rule. CLOUDWATCH_LOGS_SCHEME was marked as "ignore lint rule" as a temporary workaround, not something to continue when new symbols are introduced.

Why is a new scheme needed? Why not use a querystring parameter with the existing aws-cwl:// scheme?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rename, didn't fully realize the linter suppression, was just following precedent. I can update the variable naming.

The reason I added a new scheme is because I am not leveraging the existing document and code lens providers. The logic for fetching logs, and the supplied codeLenses for LiveTail are different than those in the existing providers. I didn't know query param would be an option, but since the providers are tied to the scheme itself, I'm not sure if a query param does what I was intending for by having separate providers. Let me know if that can be done!

I could use a query param and a shared scheme, but then I assume that each time one of the providers run, we'd need to inspect the URI and have a branching logic to handle a LiveTail case or not. In that case I'd prefer to just handle these cases in separate classes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not leveraging the existing document and code lens providers

I believe the existing abstraction is generalized to allow different "providers" of CWL events. So it could be re-used. But it may be a judgement call about whether that saves or increases complexity.

import { Settings } from '../../shared/settings'
import { addLogEvents } from './commands/addLogEvents'
import { copyLogResource } from './commands/copyLogResource'
Expand All @@ -20,11 +20,15 @@ import { changeLogSearchParams } from './changeLogSearch'
import { CloudWatchLogsNode } from './explorer/cloudWatchLogsNode'
import { loadAndOpenInitialLogStreamFile, LogStreamCodeLensProvider } from './document/logStreamsCodeLensProvider'
import { tailLogGroup } from './commands/tailLogGroup'
import { LiveTailDocumentProvider } from './document/liveTailDocumentProvider'
import { LiveTailSessionRegistry } from './registry/liveTailSessionRegistry'

export async function activate(context: vscode.ExtensionContext, configuration: Settings): Promise<void> {
const registry = LogDataRegistry.instance
const liveTailRegistry = LiveTailSessionRegistry.instance

const documentProvider = new LogDataDocumentProvider(registry)
const liveTailDocumentProvider = new LiveTailDocumentProvider()

context.subscriptions.push(
vscode.languages.registerCodeLensProvider(
Expand All @@ -40,6 +44,10 @@ export async function activate(context: vscode.ExtensionContext, configuration:
vscode.workspace.registerTextDocumentContentProvider(CLOUDWATCH_LOGS_SCHEME, documentProvider)
)

context.subscriptions.push(
vscode.workspace.registerTextDocumentContentProvider(CLOUDWATCH_LOGS_LIVETAIL_SCHEME, liveTailDocumentProvider)
)

context.subscriptions.push(
vscode.workspace.onDidCloseTextDocument((doc) => {
if (doc.isClosed && doc.uri.scheme === CLOUDWATCH_LOGS_SCHEME) {
Expand Down Expand Up @@ -97,7 +105,7 @@ export async function activate(context: vscode.ExtensionContext, configuration:
node instanceof LogGroupNode
? { regionName: node.regionCode, groupName: node.logGroup.logGroupName! }
: undefined
await tailLogGroup(logGroupInfo)
await tailLogGroup(liveTailRegistry, logGroupInfo)
})
)
}
145 changes: 141 additions & 4 deletions packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,154 @@
* SPDX-License-Identifier: Apache-2.0
*/

import * as vscode from 'vscode'
import { TailLogGroupWizard } from '../wizard/tailLogGroupWizard'
import { getLogger } from '../../../shared'
import { CancellationError } from '../../../shared/utilities/timeoutUtils'
import { LiveTailSession, LiveTailSessionConfiguration } from '../registry/liveTailSession'
import { LiveTailSessionRegistry } from '../registry/liveTailSessionRegistry'
import { LiveTailSessionLogEvent, StartLiveTailResponseStream } from '@aws-sdk/client-cloudwatch-logs'
import { ToolkitError } from '../../../shared'

export async function tailLogGroup(logData?: { regionName: string; groupName: string }): Promise<void> {
export async function tailLogGroup(
registry: LiveTailSessionRegistry,
logData?: { regionName: string; groupName: string }
): Promise<void> {
const wizard = new TailLogGroupWizard(logData)
const wizardResponse = await wizard.run()
if (!wizardResponse) {
throw new CancellationError('user')
}

//TODO: Remove Log. For testing while we aren't yet consuming the wizardResponse.
getLogger().info(JSON.stringify(wizardResponse))
const liveTailSessionConfig: LiveTailSessionConfiguration = {
logGroupName: wizardResponse.regionLogGroupSubmenuResponse.data,
logStreamFilter: wizardResponse.logStreamFilter,
logEventFilterPattern: wizardResponse.filterPattern,
region: wizardResponse.regionLogGroupSubmenuResponse.region,
}
const session = new LiveTailSession(liveTailSessionConfig)
if (registry.has(session.uri)) {
await prepareDocument(session)
return
}
registry.set(session.uri, session)

const document = await prepareDocument(session)
registerTabChangeCallback(session, registry, document)
const stream = await session.startLiveTailSession()

await handleSessionStream(stream, document, session)
}

export function closeSession(sessionUri: vscode.Uri, registry: LiveTailSessionRegistry) {
const session = registry.get(sessionUri)
if (session === undefined) {
throw new ToolkitError(`No LiveTail session found for URI: ${sessionUri.toString()}`)
}
session.stopLiveTailSession()
registry.delete(sessionUri)
}

export async function clearDocument(textDocument: vscode.TextDocument) {
const edit = new vscode.WorkspaceEdit()
const startPosition = new vscode.Position(0, 0)
const endPosition = new vscode.Position(textDocument.lineCount, 0)
edit.delete(textDocument.uri, new vscode.Range(startPosition, endPosition))
await vscode.workspace.applyEdit(edit)
}

async function prepareDocument(session: LiveTailSession): Promise<vscode.TextDocument> {
const textDocument = await vscode.workspace.openTextDocument(session.uri)
await clearDocument(textDocument)
await vscode.window.showTextDocument(textDocument, { preview: false })
await vscode.languages.setTextDocumentLanguage(textDocument, 'log')
return textDocument
}

async function handleSessionStream(
stream: AsyncIterable<StartLiveTailResponseStream>,
document: vscode.TextDocument,
session: LiveTailSession
) {
try {
for await (const event of stream) {
if (event.sessionUpdate !== undefined && event.sessionUpdate.sessionResults !== undefined) {
const formattedLogEvents = event.sessionUpdate.sessionResults.map<string>((logEvent) =>
formatLogEvent(logEvent)
)
if (formattedLogEvents.length !== 0) {
await updateTextDocumentWithNewLogEvents(formattedLogEvents, document, session.maxLines)
}
}
}
} catch (err) {
throw new ToolkitError('Caught on-stream exception')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the point of this? the exception that was caught likely has more useful information.

please review all of the code guidelines: https://github.com/aws/aws-toolkit-vscode/blob/master/docs/CODE_GUIDELINES.md#exceptions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack, will adress.

}
}

function formatLogEvent(logEvent: LiveTailSessionLogEvent): string {
if (!logEvent.timestamp || !logEvent.message) {
return ''
}
const timestamp = new Date(logEvent.timestamp).toLocaleTimeString('en', {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use one of the existing shared datetime format utils. that ensures that we use consistent formatting everywhere in the extension:

export function formatLocalized(d: Date = new Date(), cloud9 = isCloud9()): string {
const dateFormat = new Intl.DateTimeFormat('en-US', {
year: 'numeric',
month: 'short',
day: 'numeric',
})
const timeFormat = new Intl.DateTimeFormat('en-US', {
hour: 'numeric',
minute: 'numeric',
second: 'numeric',
timeZoneName: cloud9 ? 'short' : 'shortOffset',
})
return `${dateFormat.format(d)} ${timeFormat.format(d)}`
}
/**
* Matches Insights console timestamp, e.g.: 2019-03-04T11:40:08.781-08:00
* TODO: Do we want this this verbose? Log stream just shows HH:mm:ss
*/
export function formatDateTimestamp(forceUTC: boolean, d: Date = new Date()): string {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, will address.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circling back to this, when I demoed LiveTail to our CWL product manager, it was requested that we shorten the time we display to just HH:MM:SS. The reason being that these events are coming in live, so the additional context of Date and timezone weren't seen as important.

In that case, is this a required change to make?

timeStyle: 'medium',
hour12: false,
timeZone: 'UTC',
})
let line = timestamp.concat('\t', logEvent.message)
if (!line.endsWith('\n')) {
line = line.concat('\n')
}
return line
}

async function updateTextDocumentWithNewLogEvents(
formattedLogEvents: string[],
document: vscode.TextDocument,
maxLines: number
) {
const edit = new vscode.WorkspaceEdit()
formattedLogEvents.forEach((formattedLogEvent) =>
edit.insert(document.uri, new vscode.Position(document.lineCount, 0), formattedLogEvent)
)
await vscode.workspace.applyEdit(edit)
}

/**
* The LiveTail session should be automatically closed if the user does not have the session's
* document in any Tab in their editor.
*
* `onDidCloseTextDocument` doesn't work for our case because the tailLogGroup command will keep the stream
* writing to the doc even when all its tabs/editors are closed, seemingly keeping the doc 'open'.
* Also there is no guarantee that this event fires when an editor tab is closed
*
* `onDidChangeVisibleTextEditors` returns editors that the user can see its contents. An editor that is open, but hidden
* from view, will not be returned. Meaning a Tab that is created (shown in top bar), but not open, will not be returned. Even if
* the tab isn't visible, we want to continue writing to the doc, and keep the session alive.
*/
function registerTabChangeCallback(
session: LiveTailSession,
registry: LiveTailSessionRegistry,
document: vscode.TextDocument
) {
vscode.window.tabGroups.onDidChangeTabs((tabEvent) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i'm understanding this correctly I think you might want vscode.workspace.textDocuments instead. I've never actually seen vscode.window.tabGroups before 🤔. The process would be more or less the same expect instead of onDidChangeTabs and interating over every tab group you can just. Something like:

vscode.workspace.onDidChangeTextDocument((textDocument) => {
    if (textDocument.document.uri.toString() === liveTailSession.uri.toString()) {

    }
})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we are trying to achieve is that if a user doesn't have the LiveTail session open in any tab in their editor, that we close the session.

What I've learned is that the underlying TextDocument can remain open, even if its editor tabs are closed. It seems that the LiveTail command will keep the doucment open even if all its editors are closed because the command is still writing from the stream to the document. https://code.visualstudio.com/api/references/vscode-api#workspace.onDidCloseTextDocument. This suggests looking at onDidChangeVisibleTextEditors, but that event fires even when swapping tabs, not closing. https://code.visualstudio.com/api/references/vscode-api#window.onDidChangeVisibleTextEditors

This implementation is admittedly heavy, but has been the only thing I've found that fits what I'm trying to accomplish.

const isOpen = isLiveTailSessionOpenInAnyTab(session)
if (!isOpen) {
closeSession(session.uri, registry)
void clearDocument(document)
}
})
}

function isLiveTailSessionOpenInAnyTab(liveTailSession: LiveTailSession) {
let isOpen = false
vscode.window.tabGroups.all.forEach(async (tabGroup) => {
tabGroup.tabs.forEach((tab) => {
if (tab.input instanceof vscode.TabInputText) {
if (liveTailSession.uri.toString() === tab.input.uri.toString()) {
isOpen = true
}
}
})
})
return isOpen
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*!
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

import * as vscode from 'vscode'

export class LiveTailDocumentProvider implements vscode.TextDocumentContentProvider {
provideTextDocumentContent(uri: vscode.Uri, token: vscode.CancellationToken): vscode.ProviderResult<string> {
//Content will be written to the document via handling a LiveTail response stream in the TailLogGroup command.
return ''
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
import * as vscode from 'vscode'
import { CloudWatchLogsClient, StartLiveTailCommand, StartLiveTailCommandOutput } from '@aws-sdk/client-cloudwatch-logs'
import {
CloudWatchLogsClient,
StartLiveTailCommand,
StartLiveTailResponseStream,
} from '@aws-sdk/client-cloudwatch-logs'
import { LogStreamFilterResponse } from '../wizard/liveTailLogStreamSubmenu'
import { CloudWatchLogsSettings } from '../cloudWatchLogsUtils'
import { Settings, ToolkitError } from '../../../shared'
Expand Down Expand Up @@ -58,12 +62,16 @@ export class LiveTailSession {
return this._logGroupName
}

public startLiveTailSession(): Promise<StartLiveTailCommandOutput> {
public async startLiveTailSession(): Promise<AsyncIterable<StartLiveTailResponseStream>> {
const command = this.buildStartLiveTailCommand()
try {
return this.liveTailClient.cwlClient.send(command, {
const commandOutput = await this.liveTailClient.cwlClient.send(command, {
abortSignal: this.liveTailClient.abortController.signal,
})
if (!commandOutput.responseStream) {
throw new ToolkitError('LiveTail session response stream is undefined.')
}
return commandOutput.responseStream
} catch (e) {
throw new ToolkitError('Encountered error while trying to start LiveTail session.')
}
Expand Down
Loading
Loading