Skip to content

refactor(ec2): pass log level to connect script #6161

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Dec 6, 2024
Merged

refactor(ec2): pass log level to connect script #6161

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d033ec7
avoid logging token
Hweinstock Dec 4, 2024
42c14ac
use require no log instead
Hweinstock Dec 5, 2024
9762ff9
pass log level down to connect script
Hweinstock Dec 5, 2024
1381dc4
make file location in debug mode only
Hweinstock Dec 5, 2024
db445fb
Merge branch 'feature/postreinvent' into ec2/loglevel
Hweinstock Dec 5, 2024
b7b36ad
use flag to determine if script should debug
Hweinstock Dec 5, 2024
4e80178
adjust var name
Hweinstock Dec 5, 2024
232d88e
Merge branch 'ec2/loglevel' of github.com:Hweinstock/aws-toolkit-vsco…
Hweinstock Dec 5, 2024
ca6046a
make import absolute
Hweinstock Dec 5, 2024
2f8be88
add a check around log level set
Hweinstock Dec 5, 2024
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
18 changes: 13 additions & 5 deletions packages/core/resources/ec2_connect
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# Usage:
# When connecting to a dev environment
# AWS_REGION=… AWS_SSM_CLI=… STREAM_URL=… TOKEN=… LOG_FILE_LOCATION==… ./ec2_connect
# AWS_REGION=… AWS_SSM_CLI=… STREAM_URL=… TOKEN=… LOG_FILE_LOCATION==… LOG_LEVEL=… ./ec2_connect

set -e
set -u
Expand Down Expand Up @@ -44,13 +44,21 @@ _ec2() {

_main() {
_log "=============================================================================="

_require AWS_SSM_CLI "${AWS_SSM_CLI:-}"
_require LOG_LEVEL "${LOG_LEVEL:-}"
_require AWS_REGION "${AWS_REGION:-}"

_require SESSION_ID "${SESSION_ID:-}"
_require_nolog STREAM_URL "${STREAM_URL:-}"
_require_nolog TOKEN "${TOKEN:-}"
_require SESSION_ID "${SESSION_ID:-}"
_require LOG_FILE_LOCATION "${LOG_FILE_LOCATION:-}"

# Only log file paths when debug level is set in toolkit.
if [ "${LOG_LEVEL:-}" -le 2 ]; then
_require AWS_SSM_CLI "${AWS_SSM_CLI:-}"
_require LOG_FILE_LOCATION "${LOG_FILE_LOCATION:-}"
else
_require_nolog AWS_SSM_CLI "${AWS_SSM_CLI:-}"
_require_nolog LOG_FILE_LOCATION "${LOG_FILE_LOCATION:-}"
fi

_ec2 "$AWS_SSM_CLI" "$AWS_REGION" "$STREAM_URL" "$TOKEN" "$SESSION_ID"
}
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/awsService/ec2/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ export class Ec2Connecter implements vscode.Disposable {
const ssmSession = await this.startSSMSession(selection.instanceId)

const vars = getEc2SsmEnv(selection, ssm, ssmSession)
getLogger().info(`ec2: connect script logs at ${vars.LOG_FILE_LOCATION}`)
getLogger().debug(`ec2: connect script logs at ${vars.LOG_FILE_LOCATION}`)

const envProvider = async () => {
return { [sshAgentSocketVariable]: await startSshAgent(), ...vars }
}
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/awsService/ec2/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { copyToClipboard } from '../../shared/utilities/messages'
import { Ec2Selection } from './prompter'
import { sshLogFileLocation } from '../../shared/sshConfig'
import { SSM } from 'aws-sdk'
import { globals } from '../../shared'

export function getIconCode(instance: SafeEc2Instance) {
if (instance.LastSeenStatus === 'running') {
Expand Down Expand Up @@ -42,6 +43,7 @@ export function getEc2SsmEnv(
STREAM_URL: session.StreamUrl,
SESSION_ID: session.SessionId,
TOKEN: session.TokenValue,
LOG_LEVEL: globals.logOutputChannel.logLevel,
Copy link
Contributor

@justinmk3 justinmk3 Dec 5, 2024

Choose a reason for hiding this comment

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

looks like getLogger doesn't have e.g. getLogLevel. otoh, we could make the flag a simple boolean like LOG_DEBUG:

Suggested change
LOG_LEVEL: globals.logOutputChannel.logLevel,
LOG_DEBUG: getLogger().logLevelEnabled('debug') ? '1': '0',

that also would avoid the use of -le in the bash script

Copy link
Contributor Author

@Hweinstock Hweinstock Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think logLevelEnabled works as intended. I was able to produce this:

2024-12-05 17:11:27.838 [info] vscode log level: info
2024-12-05 17:11:27.838 [info] is debug enabled?: true

and I believe its because the internal log level is debug until the user changes it based on the code here:

aws-toolkit-vscode/packages/core/src/shared/logger/activation.ts

Lines 43 to 49 in aa332da

logChannel.onDidChangeLogLevel?.((logLevel) => {
const newLogLevel = fromVscodeLogLevel(logLevel)
mainLogger.setLogLevel(newLogLevel) // Also logs a message.
})
mainLogger.setLogLevel('debug') // HACK: set to "debug" when debugging the extension.
setLogger(mainLogger)

I see the comment so I am wondering if there something special about the fact that I am running locally? Otherwise this seems like a bug.

Once I manually set the log level (i.e. trigger that event) this doesn't happen.

Copy link
Contributor

@justinmk3 justinmk3 Dec 5, 2024

Choose a reason for hiding this comment

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

I think when I added that "HACK" line, I assume that something in vscode would always overwrite the log-level, except when running in debug-mode. That line definitely needs some attention.

Perhaps the easiest fix is to wrap that line in if (!isRelease() && ...) or whatever

},
process.env
)
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/shared/extensionGlobals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { ExtensionContext, OutputChannel, Uri } from 'vscode'
import { ExtensionContext, LogOutputChannel, OutputChannel, Uri } from 'vscode'
import { LoginManager } from '../auth/deprecated/loginManager'
import { AwsResourceManager } from '../dynamicResources/awsResourceManager'
import { AWSClientBuilder } from './awsClientBuilder'
Expand Down Expand Up @@ -191,7 +191,7 @@ export interface ToolkitGlobals {
/**
* Log messages. Use `outputChannel` for application messages.
*/
logOutputChannel: OutputChannel
logOutputChannel: LogOutputChannel
loginManager: LoginManager
awsContextCommands: AwsContextCommands
awsContext: AwsContext
Expand Down
Loading