-
Notifications
You must be signed in to change notification settings - Fork 485
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.
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
feat(cwl): Add clear screen and stop session code lens #5958
Changes from 2 commits
4a7a75f
1f8d9dd
104fb2c
432ecf1
9b2c3a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -13,7 +13,7 @@ import { | |||||||||
LiveTailSessionUpdate, | ||||||||||
StartLiveTailResponseStream, | ||||||||||
} from '@aws-sdk/client-cloudwatch-logs' | ||||||||||
import { globals, ToolkitError } from '../../../shared' | ||||||||||
import { getLogger, ToolkitError } from '../../../shared' | ||||||||||
|
||||||||||
export async function tailLogGroup( | ||||||||||
registry: LiveTailSessionRegistry, | ||||||||||
|
@@ -32,32 +32,29 @@ export async function tailLogGroup( | |||||||||
region: wizardResponse.regionLogGroupSubmenuResponse.region, | ||||||||||
} | ||||||||||
const session = new LiveTailSession(liveTailSessionConfig) | ||||||||||
if (registry.has(session.uri)) { | ||||||||||
if (registry.has(session.uri.toString())) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
aws-toolkit-vscode/packages/core/src/awsService/cloudWatchLogs/cloudWatchLogsUtils.ts Line 33 in d4d1ae0
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aws-toolkit-vscode/packages/core/src/awsService/cloudWatchLogs/registry/logDataRegistry.ts Lines 51 to 53 in d4d1ae0
This makes me wonder why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the aim of these two registries are different from one another. I think this is a separate responsiblity from the LogDataRegistry that is actually persisting LogData, and making requests to FilterLogEvents. |
||||||||||
await prepareDocument(session) | ||||||||||
return | ||||||||||
} | ||||||||||
registry.set(session.uri, session) | ||||||||||
registry.set(session.uri.toString(), session) | ||||||||||
|
||||||||||
const document = await prepareDocument(session) | ||||||||||
const timer = globals.clock.setInterval(() => { | ||||||||||
session.updateStatusBarItemText() | ||||||||||
}, 500) | ||||||||||
|
||||||||||
hideShowStatusBarItemsOnActiveEditor(session, document) | ||||||||||
registerTabChangeCallback(session, registry, document, timer) | ||||||||||
registerTabChangeCallback(session, registry, document) | ||||||||||
|
||||||||||
const stream = await session.startLiveTailSession() | ||||||||||
|
||||||||||
await handleSessionStream(stream, document, session, timer) | ||||||||||
await handleSessionStream(stream, document, session) | ||||||||||
} | ||||||||||
|
||||||||||
export function closeSession(sessionUri: vscode.Uri, registry: LiveTailSessionRegistry, timer: NodeJS.Timer) { | ||||||||||
globals.clock.clearInterval(timer) | ||||||||||
const session = registry.get(sessionUri) | ||||||||||
export function closeSession(sessionUri: vscode.Uri, registry: LiveTailSessionRegistry) { | ||||||||||
const session = registry.get(sessionUri.toString()) | ||||||||||
if (session === undefined) { | ||||||||||
throw new ToolkitError(`No LiveTail session found for URI: ${sessionUri.toString()}`) | ||||||||||
} | ||||||||||
session.stopLiveTailSession() | ||||||||||
registry.delete(sessionUri) | ||||||||||
registry.delete(sessionUri.toString()) | ||||||||||
} | ||||||||||
|
||||||||||
export async function clearDocument(textDocument: vscode.TextDocument) { | ||||||||||
|
@@ -80,8 +77,7 @@ async function prepareDocument(session: LiveTailSession): Promise<vscode.TextDoc | |||||||||
async function handleSessionStream( | ||||||||||
stream: AsyncIterable<StartLiveTailResponseStream>, | ||||||||||
document: vscode.TextDocument, | ||||||||||
session: LiveTailSession, | ||||||||||
timer: NodeJS.Timer | ||||||||||
session: LiveTailSession | ||||||||||
) { | ||||||||||
try { | ||||||||||
for await (const event of stream) { | ||||||||||
|
@@ -100,8 +96,21 @@ async function handleSessionStream( | |||||||||
session.isSampled = isSampled(event.sessionUpdate) | ||||||||||
} | ||||||||||
} | ||||||||||
} finally { | ||||||||||
globals.clock.clearInterval(timer) | ||||||||||
} catch (e) { | ||||||||||
if (session.isAborted) { | ||||||||||
//Expected case. User action cancelled stream (CodeLens, Close Editor, etc.). | ||||||||||
//AbortSignal interrupts the LiveTail stream, causing error to be thrown here. | ||||||||||
//Can assume that stopLiveTailSession() has already been called - AbortSignal is only | ||||||||||
//exposed through that method. | ||||||||||
getLogger().info(`Session ${session.uri.toString()} stopped.`) | ||||||||||
keeganirby marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
} else { | ||||||||||
//Unexpected exception. | ||||||||||
session.stopLiveTailSession() | ||||||||||
throw ToolkitError.chain( | ||||||||||
e, | ||||||||||
`Unexpected on-stream execption while tailing session: ${session.uri.toString()}` | ||||||||||
keeganirby marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
) | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
|
@@ -196,13 +205,12 @@ function hideShowStatusBarItemsOnActiveEditor(session: LiveTailSession, document | |||||||||
function registerTabChangeCallback( | ||||||||||
session: LiveTailSession, | ||||||||||
registry: LiveTailSessionRegistry, | ||||||||||
document: vscode.TextDocument, | ||||||||||
timer: NodeJS.Timer | ||||||||||
document: vscode.TextDocument | ||||||||||
) { | ||||||||||
vscode.window.tabGroups.onDidChangeTabs((tabEvent) => { | ||||||||||
const isOpen = isLiveTailSessionOpenInAnyTab(session) | ||||||||||
if (!isOpen) { | ||||||||||
closeSession(session.uri, registry, timer) | ||||||||||
closeSession(session.uri, registry) | ||||||||||
void clearDocument(document) | ||||||||||
} | ||||||||||
}) | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
/*! | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import * as vscode from 'vscode' | ||
import { cloudwatchLogsLiveTailScheme } from '../../../shared/constants' | ||
|
||
export class LiveTailCodeLensProvider implements vscode.CodeLensProvider { | ||
onDidChangeCodeLenses?: vscode.Event<void> | undefined | ||
|
||
provideCodeLenses( | ||
document: vscode.TextDocument, | ||
token: vscode.CancellationToken | ||
): vscode.ProviderResult<vscode.CodeLens[]> { | ||
const uri = document.uri | ||
if (uri.scheme !== cloudwatchLogsLiveTailScheme) { | ||
return [] | ||
} | ||
const codeLenses: vscode.CodeLens[] = [] | ||
codeLenses.push(this.buildClearDocumentCodeLens(document)) | ||
codeLenses.push(this.buildStopTailingCodeLens(document)) | ||
return codeLenses | ||
} | ||
|
||
private buildClearDocumentCodeLens(document: vscode.TextDocument): vscode.CodeLens { | ||
const range = this.getBottomOfDocumentRange(document) | ||
const command: vscode.Command = { | ||
title: 'Clear document', | ||
command: 'aws.cwl.clearDocument', | ||
arguments: [document], | ||
} | ||
return new vscode.CodeLens(range, command) | ||
} | ||
|
||
private buildStopTailingCodeLens(document: vscode.TextDocument): vscode.CodeLens { | ||
const range = this.getBottomOfDocumentRange(document) | ||
const command: vscode.Command = { | ||
title: 'Stop tailing', | ||
command: 'aws.cwl.stopTailingLogGroup', | ||
arguments: [document], | ||
} | ||
return new vscode.CodeLens(range, command) | ||
} | ||
|
||
private getBottomOfDocumentRange(document: vscode.TextDocument): vscode.Range { | ||
return new vscode.Range( | ||
new vscode.Position(document.lineCount - 1, 0), | ||
new vscode.Position(document.lineCount - 1, 0) | ||
) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have a different scheme, and a different codelens, for "live tail"? The codelenses for the non-livetail log group search would be useful in live tail. Why are they separate?
Whether a log group is being "tailed" or not is just a flag. It's not an entirely different concept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CodeLenses in the existing CWL experiences are not very applicable to our LiveTail use case.
"Load newer" & "Load older" don't apply when data is being streamed in constantly. For
View full log stream
, customers can apply LogStream filter parameters to their liveTail session already so having this as a codeLens in a Livetail session doesn't seem necessary.On the other hand, a
SearchLogGroup
command won't need aStop tailing
code lens.These UX are quite separate.
We already separate the CodeLens providers for
logData
andlogStream
. Given the UX and functionality of LiveTail is different from our other experiences I think it fits to have a separate CodeLens provider.This is the same reasoning behind having a separate scheme as we discussed here. The way we retrieve/display logs from a LiveTail session stream differs enough from the existing LogDataRegistry that I felt that it was best to separate these logic's, as opposed to finding a way to generalize.
I feel that LiveTail and the existing experiences are separate enough from each other that they should be treated as such.