feat(logging): add isLoggable util #12977
Conversation
ashwinkumar6
changed the title
ashwinkumar6
force-pushed
the
logging/isLoggable
branch
from
dfc2d2c to
bafa9c1
Compare
ashwinkumar6
changed the base branch from
logging/PR-12907
to
central-logger
ashwinkumar6
force-pushed
the
logging/isLoggable
branch
from
bafa9c1 to
c7f481f
Compare
|
|
||
| jest.mock('@aws-amplify/core', () => { | ||
| return { | ||
| ...jest.requireActual('@aws-amplify/core'), |
There was a problem hiding this comment.
Nit: Can we get by without the requireActual? Not sure what else from core we might need to mock for this
There was a problem hiding this comment.
Makes sense, we can add additional mocks to items imported from core if required to make it more evident on what's being used. Removed requireActual
packages/logging/__tests__/providers/cloudwatch/utils/loggableHelpers.test.ts
Outdated
Show resolved
Hide resolved
packages/logging/__tests__/providers/cloudwatch/utils/loggableHelpers.test.ts
Outdated
Show resolved
Hide resolved
packages/logging/__tests__/providers/cloudwatch/utils/loggableHelpers.test.ts
Outdated
Show resolved
Hide resolved
| categoryLogLevel: { Auth: 'WARN', Storage: 'DEBUG' }, | ||
| }; | ||
| expect( | ||
| await resolveLoggingConstraints(authErrorLog, loggingConstraints), |
There was a problem hiding this comment.
Nit: resolves.toBe might be a little cleaner for this use-case (here and elsewhere in this PR)
There was a problem hiding this comment.
Thanks good callout, makes sense to use this when dealing with async functions.
Updated in all places
| const { defaultLogLevel, categoryLogLevel, userLogLevel } = | ||
| loggingConstraints; | ||
|
|
||
| const { userSub = '' } = await fetchAuthSession(); |
There was a problem hiding this comment.
Wouldn't undefined be a more appropriate default value here? We could check that it is set before checking userLogLevel below
There was a problem hiding this comment.
Makes sense, we could keep userSub as string | undefined instead of defaulting to a empty string just to be sure. And add an additional condition to the if block as shown below
const { userSub } = await fetchAuthSession();
if (userSub && userLogLevel?.[userSub]) {
return {
defaultLogLevel: userLogLevel[userSub].defaultLogLevel,
categoryLogLevel: userLogLevel[userSub]?.categoryLogLevel,
};
}
…Helpers.test.ts Co-authored-by: Jim Blanchard <[email protected]>
…Helpers.test.ts Co-authored-by: Jim Blanchard <[email protected]>
…Helpers.test.ts Co-authored-by: Jim Blanchard <[email protected]>
Description of changes
LoggingConstraintsin cloudwatch configuration.PS: WIP, to be rebased on top of #12907
Description of how you validated changes
Unit tests with all scenarios
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.