Skip to content
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

deps(sdkv3): implement logging and endpoint override for client builder. #6441

Open
wants to merge 31 commits into
base: feature/sdkv3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
f464309
move in main file
Hweinstock Nov 22, 2024
afc0a26
add test file
Hweinstock Nov 22, 2024
75822d7
implement middlestack
Hweinstock Nov 22, 2024
be3c42a
add more detail to telemetry emitted
Hweinstock Nov 27, 2024
89ef745
add a default retry strategy
Hweinstock Nov 27, 2024
4890f73
use class type instead of interface
Hweinstock Dec 2, 2024
ccfd462
give up type info so that it compiles
Hweinstock Dec 2, 2024
0675afe
refactor types to have any middleware
Hweinstock Dec 2, 2024
3e8caf6
remove interface
Hweinstock Dec 2, 2024
67784e1
remove unused types
Hweinstock Dec 2, 2024
1d29b18
simplify naming
Hweinstock Dec 2, 2024
97375a3
merge in upstream changes
Hweinstock Dec 2, 2024
4500cdc
rename test file
Hweinstock Dec 2, 2024
4e4f53f
Merge branch 'sdkv3/startMigration' into sdkv3/middlestack2
Hweinstock Dec 2, 2024
4e2dffe
change name to temp
Hweinstock Dec 2, 2024
470e0f5
switch name to lower case
Hweinstock Dec 2, 2024
6fc31e5
Merge branch 'feature/sdkv3' into sdkv3/startMigration
Hweinstock Jan 8, 2025
6a04c98
add link to relevant issue
Hweinstock Jan 9, 2025
58809be
move dep to core module
Hweinstock Jan 14, 2025
4880c16
improve middleware typing
Hweinstock Jan 14, 2025
6dc2405
remove redundant type
Hweinstock Jan 14, 2025
222ff04
improve typing
Hweinstock Jan 14, 2025
f0adfee
refactor: customerUserAgent -> userAgent
Hweinstock Jan 27, 2025
7bd08b3
refactor: remove blank line
Hweinstock Jan 27, 2025
688899b
refactor: use partialClone over omitIfPresent
Hweinstock Jan 27, 2025
3cec376
Merge branch 'feature/sdkv3' into sdkv3/startMigration
Hweinstock Jan 27, 2025
312c93c
refactor: add depth field to partialClone
Hweinstock Jan 27, 2025
a110c1f
refactor: rename test file
Hweinstock Jan 27, 2025
bee9e06
merge: resolve conflicts with upstream
Hweinstock Jan 27, 2025
9a3aab9
refactor: clean up tests
Hweinstock Jan 27, 2025
79fbbc4
merge: resolve conflicts with upstream
Hweinstock Jan 27, 2025
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
85 changes: 64 additions & 21 deletions packages/core/src/shared/awsClientBuilderV3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import { AwsCredentialIdentityProvider, RetryStrategyV2 } from '@smithy/types'
import { getUserAgent } from './telemetry/util'
import { DevSettings } from './settings'
import {
BuildHandler,
BuildMiddleware,
DeserializeHandler,
DeserializeHandlerOptions,
DeserializeMiddleware,
FinalizeHandler,
FinalizeRequestMiddleware,
HandlerExecutionContext,
MetadataBearer,
MiddlewareStack,
Expand All @@ -21,13 +24,14 @@ import {
RetryStrategy,
UserAgent,
} from '@aws-sdk/types'
import { HttpResponse } from '@aws-sdk/protocol-http'
import { HttpResponse, HttpRequest } from '@aws-sdk/protocol-http'
import { ConfiguredRetryStrategy } from '@smithy/util-retry'
import { telemetry } from './telemetry'
import { getRequestId, getTelemetryReason, getTelemetryReasonDesc, getTelemetryResult } from './errors'
import { extensionVersion } from '.'
import { getLogger } from './logger'
import { partialClone } from './utilities/collectionUtils'
import { selectFrom } from './utilities/tsUtils'

export type AwsClientConstructor<C> = new (o: AwsClientOptions) => C

Expand Down Expand Up @@ -96,8 +100,9 @@ export class AWSClientBuilderV3 {
}

const service = new type(opt)
// TODO: add middleware for logging, telemetry, endpoints.
service.middlewareStack.add(telemetryMiddleware, { step: 'deserialize' } as DeserializeHandlerOptions)
service.middlewareStack.add(telemetryMiddleware, { step: 'deserialize' })
service.middlewareStack.add(loggingMiddleware, { step: 'finalizeRequest' })
service.middlewareStack.add(getEndpointMiddleware(settings), { step: 'build' })
return service
}
}
Expand All @@ -123,29 +128,67 @@ export function recordErrorTelemetry(err: Error, serviceName?: string) {
function logAndThrow(e: any, serviceId: string, errorMessageAppend: string): never {
if (e instanceof Error) {
recordErrorTelemetry(e, serviceId)
const err = { ...e }
delete err['stack']
getLogger().error('API Response %s: %O', errorMessageAppend, err)
getLogger().error('API Response %s: %O', errorMessageAppend, e)
}
throw e
}
/**
* Telemetry logic to be added to all created clients. Adds logging and emitting metric on errors.
*/

const telemetryMiddleware: DeserializeMiddleware<any, any> =
(next: DeserializeHandler<any, any>, context: HandlerExecutionContext) => async (args: any) => {
if (!HttpResponse.isInstance(args.request)) {
return next(args)
}
const serviceId = getServiceId(context as object)
const { hostname, path } = args.request
const logTail = `(${hostname} ${path})`
const result = await next(args).catch((e: any) => logAndThrow(e, serviceId, logTail))
(next: DeserializeHandler<any, any>, context: HandlerExecutionContext) => async (args: any) =>
emitOnRequest(next, context, args)

const loggingMiddleware: FinalizeRequestMiddleware<any, any> = (next: FinalizeHandler<any, any>) => async (args: any) =>
logOnRequest(next, args)

function getEndpointMiddleware(settings: DevSettings = DevSettings.instance): BuildMiddleware<any, any> {
return (next: BuildHandler<any, any>, context: HandlerExecutionContext) => async (args: any) =>
overwriteEndpoint(next, context, settings, args)
}

export async function emitOnRequest(next: DeserializeHandler<any, any>, context: HandlerExecutionContext, args: any) {
if (!HttpResponse.isInstance(args.request)) {
return next(args)
}
const serviceId = getServiceId(context as object)
const { hostname, path } = args.request
const logTail = `(${hostname} ${path})`
try {
const result = await next(args)
if (HttpResponse.isInstance(result.response)) {
// TODO: omit credentials / sensitive info from the logs / telemetry.
// TODO: omit credentials / sensitive info from the telemetry.
const output = partialClone(result.output, 3)
getLogger().debug('API Response %s: %O', logTail, output)
getLogger().debug(`API Response %s: %O`, logTail, output)
}

return result
} catch (e: any) {
logAndThrow(e, serviceId, logTail)
}
}

export async function logOnRequest(next: FinalizeHandler<any, any>, args: any) {
if (HttpRequest.isInstance(args.request)) {
const { hostname, path } = args.request
// TODO: omit credentials / sensitive info from the logs.
const input = partialClone(args.input, 3)
getLogger().debug(`API Request (%s %s): %O`, hostname, path, input)
}
return next(args)
}

export function overwriteEndpoint(
next: BuildHandler<any, any>,
context: HandlerExecutionContext,
settings: DevSettings,
args: any
) {
if (HttpRequest.isInstance(args.request)) {
const serviceId = getServiceId(context as object)
const endpoint = serviceId ? settings.get('endpoints', {})[serviceId] : undefined
if (endpoint) {
const url = new URL(endpoint)
Object.assign(args.request, selectFrom(url, 'hostname', 'port', 'protocol', 'pathname'))
args.request.path = args.request.pathname
}
}
return next(args)
}
4 changes: 4 additions & 0 deletions packages/core/src/test/globalSetup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ async function writeLogsToFile(testName: string) {
await fs.appendFile(testLogOutput, entries?.join('\n') ?? '')
}

export function assertLogsContainAllOf(keywords: string[], exactMatch: boolean, severity: LogLevel) {
return keywords.map((k) => assertLogsContain(k, exactMatch, severity))
}

// TODO: merge this with `toolkitLogger.test.ts:checkFile`
export function assertLogsContain(text: string, exactMatch: boolean, severity: LogLevel) {
const logs = getTestLogger().getLoggedEntries(severity)
Expand Down
151 changes: 124 additions & 27 deletions packages/core/src/test/shared/awsClientBuilderV3.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,28 @@
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

import sinon from 'sinon'
import assert from 'assert'
import { version } from 'vscode'
import { getClientId } from '../../shared/telemetry/util'
import { FakeMemento } from '../fakeExtensionContext'
import { FakeAwsContext } from '../utilities/fakeAwsContext'
import { GlobalState } from '../../shared/globalState'
import { AWSClientBuilderV3, getServiceId, recordErrorTelemetry } from '../../shared/awsClientBuilderV3'
import {
AWSClientBuilderV3,
emitOnRequest,
getServiceId,
logOnRequest,
overwriteEndpoint,
recordErrorTelemetry,
} from '../../shared/awsClientBuilderV3'
import { Client } from '@aws-sdk/smithy-client'
import { extensionVersion } from '../../shared'
import { DevSettings, extensionVersion } from '../../shared'
import { assertTelemetry } from '../testUtil'
import { telemetry } from '../../shared/telemetry'
import { HttpRequest, HttpResponse } from '@aws-sdk/protocol-http'
import { assertLogsContain, assertLogsContainAllOf } from '../globalSetup.test'
import { TestSettings } from '../utilities/testSettingsConfiguration'

describe('AwsClientBuilderV3', function () {
let builder: AWSClientBuilderV3
Expand All @@ -22,39 +32,126 @@ describe('AwsClientBuilderV3', function () {
builder = new AWSClientBuilderV3(new FakeAwsContext())
})

describe('createAndConfigureSdkClient', function () {
it('includes Toolkit user-agent if no options are specified', async function () {
const service = await builder.createAwsService(Client)
const clientId = getClientId(new GlobalState(new FakeMemento()))

assert.ok(service.config.userAgent)
assert.strictEqual(
service.config.userAgent![0][0].replace('---Insiders', ''),
`AWS-Toolkit-For-VSCode/testPluginVersion Visual-Studio-Code/${version} ClientId/${clientId}`
)
assert.strictEqual(service.config.userAgent![0][1], extensionVersion)
it('includes Toolkit user-agent if no options are specified', async function () {
const service = await builder.createAwsService(Client)
const clientId = getClientId(new GlobalState(new FakeMemento()))

assert.ok(service.config.userAgent)
assert.strictEqual(
service.config.userAgent![0][0].replace('---Insiders', ''),
`AWS-Toolkit-For-VSCode/testPluginVersion Visual-Studio-Code/${version} ClientId/${clientId}`
)
assert.strictEqual(service.config.userAgent![0][1], extensionVersion)
})

it('adds region to client', async function () {
const service = await builder.createAwsService(Client, { region: 'us-west-2' })

assert.ok(service.config.region)
assert.strictEqual(service.config.region, 'us-west-2')
})

it('adds Client-Id to user agent', async function () {
const service = await builder.createAwsService(Client)
const clientId = getClientId(new GlobalState(new FakeMemento()))
const regex = new RegExp(`ClientId/${clientId}`)
assert.ok(service.config.userAgent![0][0].match(regex))
})

it('does not override custom user-agent if specified in options', async function () {
const service = await builder.createAwsService(Client, {
userAgent: [['CUSTOM USER AGENT']],
})

assert.strictEqual(service.config.userAgent[0][0], 'CUSTOM USER AGENT')
})

describe('middlewareStack', function () {
let args: { request: { hostname: string; path: string }; input: any }
let context: { clientName?: string; commandName?: string }
let response: { response: { statusCode: number }; output: { message: string } }
let httpRequestStub: sinon.SinonStub
let httpResponseStub: sinon.SinonStub

before(function () {
httpRequestStub = sinon.stub(HttpRequest, 'isInstance')
httpResponseStub = sinon.stub(HttpResponse, 'isInstance')
httpRequestStub.callsFake(() => true)
httpResponseStub.callsFake(() => true)
})

it('adds region to client', async function () {
const service = await builder.createAwsService(Client, { region: 'us-west-2' })
beforeEach(function () {
args = {
request: {
hostname: 'testHost',
path: 'testPath',
},
input: {
testKey: 'testValue',
},
}
context = {
clientName: 'fooClient',
}
response = {
response: {
statusCode: 200,
},
output: {
message: 'test output',
},
}
})
after(function () {
sinon.restore()
})

assert.ok(service.config.region)
assert.strictEqual(service.config.region, 'us-west-2')
it('logs messages on request', async function () {
await logOnRequest((_: any) => _, args as any)
assertLogsContainAllOf(['testHost', 'testPath'], false, 'debug')
})

it('adds Client-Id to user agent', async function () {
const service = await builder.createAwsService(Client)
const clientId = getClientId(new GlobalState(new FakeMemento()))
const regex = new RegExp(`ClientId/${clientId}`)
assert.ok(service.config.userAgent![0][0].match(regex))
it('adds telemetry metadata and logs on error failure', async function () {
const next = (_: any) => {
throw new Error('test error')
}
await telemetry.vscode_executeCommand.run(async (span) => {
await assert.rejects(emitOnRequest(next, context, args))
})
assertLogsContain('test error', false, 'error')
assertTelemetry('vscode_executeCommand', { requestServiceType: 'foo' })
})

it('does not override custom user-agent if specified in options', async function () {
const service = await builder.createAwsService(Client, {
userAgent: [['CUSTOM USER AGENT']],
it('does not emit telemetry, but still logs on successes', async function () {
const next = async (_: any) => {
return response
}
await telemetry.vscode_executeCommand.run(async (span) => {
assert.deepStrictEqual(await emitOnRequest(next, context, args), response)
})
assertLogsContainAllOf(['testHost', 'testPath'], false, 'debug')
assert.throws(() => assertTelemetry('vscode_executeCommand', { requestServiceType: 'foo' }))
})

it('custom endpoints overwrite request url', async function () {
const settings = new TestSettings()
await settings.update('aws.dev.endpoints', { foo: 'http://example.com:3000/path' })
const next = async (args: any) => args
const newArgs: any = await overwriteEndpoint(next, context, new DevSettings(settings), args)

assert.strictEqual(newArgs.request.hostname, 'example.com')
assert.strictEqual(newArgs.request.protocol, 'http:')
assert.strictEqual(newArgs.request.port, '3000')
assert.strictEqual(newArgs.request.pathname, '/path')
})

it('custom endpoints are not overwritten if not specified', async function () {
const settings = new TestSettings()
const next = async (args: any) => args
const newArgs: any = await overwriteEndpoint(next, context, new DevSettings(settings), args)

assert.strictEqual(service.config.userAgent[0][0], 'CUSTOM USER AGENT')
assert.strictEqual(newArgs.request.hostname, 'testHost')
assert.strictEqual(newArgs.request.path, 'testPath')
})
})
})
Expand Down
Loading