chore(parameters): apply UA to AWS SDK clients used by Parameters

[am29d]

Description of your changes

Add user agent to clients user by parameters utility.

Related issues, RFCs

Issue number: closes #1537

Checklist

• My changes meet the tenets criteria
• I have performed a self-review of my own code
• I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my change is effective and works
• The PR title follows the conventional commit semantics

Breaking change checklist

Is it a breaking change?: NO

• I have documented the migration process
• I have added, implemented necessary warnings (if it can live side by side)

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[am29d]

e2e tests ✅https://github.com/aws-powertools/powertools-lambda-typescript/actions/runs/5454144683

[dreamorosi]

Thanks for the PR, looking also at #1575, what happens if as a customer I do this:

import { Tracer } from '@aws-lambda-powertools/tracer';
import { SecretsProvider } from '@aws-lambda-powertools/parameters/secrets';
import { SecretsManagerClient } from '@aws-sdk/client-secrets-manager';

const tracer = new Tracer({});
const customClient = new SecretsManagerClient({});
tracer.captureAWSv3Client(customClient);

const provider = new SecretsProvider({ awsSdkV3Client: customClient });

This is an use case that we suggest/call out in the docs (see callout box).

Does this attach the middleware twice? If so, does this result in the UA being appended twice? Same applies to Idempotency and all other providers.

[am29d]

Good catch with duplicate attachment.

Does this attach the middleware twice? If so, does this result in the UA being appended twice? Same applies to Idempotency and all other providers.

It fails to add two middlewares with identical name, we catch the error and continue with user code execution.

We could extend the header for each utility, similar how .NET does. For example PT/parameters/ PT/idempotency, but it needs more consideration.

[dreamorosi]

It fails to add two middlewares with identical name, we catch the error and continue with user code execution.

It's good that we don't halt execution, but we still log a warning that could be misleading. I'm going to open an issue to remember working on this asap.

[dreamorosi]

I have left a comment on the testing to improve the assertion on the newly added middleware.

Also I have opened an issue #1578 to continue the discussion above about multiple addUserAgentMiddleware calls.

[dreamorosi]

The client inside the provider is public and can be accessed via provider.client. This means you should be able to test like you did in the commons module by doing:

expect(provider.client.middlewareStack.identify()).toContain(
  'addPowertoolsToUserAgent: POWERTOOLS,USER_AGENT'
);

I think that would be preferable to changing the import (* as ..) and selecting the module in the spy.

[am29d]

I think that would be preferable to changing the import (* as ..) and selecting the module in the spy.

Can you elaborate? Curious to hear about the benefits.

I want to avoid to test the internals of the commons module and the dependencies behaviour. Imho, we should only check the call has happened downstream, otherwise we replicate the assertion logic in different places which makes it harder to change in the future. For instance, if we rename the middleware from addPowertoolsUserAgent to something else, we'd need to revisit every utility that called this method.

[dreamorosi]

Based on the way that the Jest spy is written, if we rename addPowertoolsUserAgent to something else the test would also break:

const userAgentSpy = jest.spyOn(
  UserAgentMiddleware,
  'addUserAgentMiddleware' // if function name changes, this spy also breaks
);

However I see your point and agree. I tend to avoid setting spies on imports but I can't think of a better way of doing it without reworking the constructor (which si not worth it), so let's leave it this way!

[am29d]

In case of #1578 we agreed to attach only one user agent middleware for a client. If the client is used in multiple utilities, we check if a middleware is present and skip the second call, first come first serve.

We will revisit the strategy at later points to align with other languages.