Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Remove redundant perf client and function wrapper logging #8079",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Remove redundant perf client and function wrapper logging #8079",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
20 changes: 12 additions & 8 deletions lib/msal-browser/src/controllers/StandardController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2303,16 +2303,20 @@ export class StandardController implements IController {
private getPreGeneratedPkceCodes(
correlationId: string
): PkceCodes | undefined {
this.logger.verbose(
"Attempting to pick up pre-generated PKCE codes",
correlationId
);
const res = this.pkceCode ? { ...this.pkceCode } : undefined;
this.pkceCode = undefined;
this.logger.verbose(
`'${res ? "Found" : "Did not find"}' pre-generated PKCE codes`,
correlationId
);
if (res) {
this.logger.verbose(
`Pre-generated PKCE codes were found`,
correlationId
);
} else {
this.logger.verbose(
`Pre-generated PKCE codes were not found`,
correlationId
);
}

this.performanceClient.addFields(
{ usePreGeneratedPkce: !!res },
correlationId
Expand Down
12 changes: 6 additions & 6 deletions lib/msal-common/apiReview/msal-common.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4663,12 +4663,12 @@ const X_MS_LIB_CAPABILITY_VALUE: string;
// src/response/ResponseHandler.ts:340:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/response/ResponseHandler.ts:341:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/response/ResponseHandler.ts:342:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/telemetry/performance/PerformanceClient.ts:705:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/telemetry/performance/PerformanceClient.ts:705:15 - (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
// src/telemetry/performance/PerformanceClient.ts:717:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/telemetry/performance/PerformanceClient.ts:717:27 - (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
// src/telemetry/performance/PerformanceClient.ts:718:24 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag
// src/telemetry/performance/PerformanceClient.ts:718:17 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@"
// src/telemetry/performance/PerformanceClient.ts:664:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/telemetry/performance/PerformanceClient.ts:664:15 - (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
// src/telemetry/performance/PerformanceClient.ts:676:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/telemetry/performance/PerformanceClient.ts:676:27 - (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
// src/telemetry/performance/PerformanceClient.ts:677:24 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag
// src/telemetry/performance/PerformanceClient.ts:677:17 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@"
// src/telemetry/performance/PerformanceEvent.ts:37:21 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag
// src/telemetry/performance/PerformanceEvent.ts:37:14 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@"
// src/telemetry/performance/PerformanceEvent.ts:37:8 - (tsdoc-undefined-tag) The TSDoc tag "@type" is not defined in this configuration
Expand Down
41 changes: 0 additions & 41 deletions lib/msal-common/src/telemetry/performance/PerformanceClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,17 +350,6 @@ export abstract class PerformanceClient implements IPerformanceClient {
): InProgressPerformanceEvent {
// Generate a placeholder correlation if the request does not provide one
const eventCorrelationId = correlationId || this.generateId();
if (!correlationId) {
this.logger.info(
`PerformanceClient: No correlation id provided for '${measureName}', generating`,
eventCorrelationId
);
}

this.logger.trace(
`PerformanceClient: Performance measurement started for '${measureName}'`,
eventCorrelationId
);

const inProgressEvent: PerformanceEvent = {
eventId: this.generateId(),
Expand Down Expand Up @@ -460,11 +449,6 @@ export abstract class PerformanceClient implements IPerformanceClient {
rootEvent.incompleteSubMeasurements?.delete(event.eventId);
}

this.logger.trace(
`PerformanceClient: Performance measurement ended for '${event.name}': '${event.durationMs}' ms`,
event.correlationId
);

if (error) {
addError(error, this.logger, rootEvent);
}
Expand Down Expand Up @@ -526,10 +510,6 @@ export abstract class PerformanceClient implements IPerformanceClient {
fields: { [key: string]: {} | undefined },
correlationId: string
): void {
this.logger.trace(
"PerformanceClient: Updating static fields",
correlationId
);
const event = this.eventsByCorrelationId.get(correlationId);
if (event) {
this.eventsByCorrelationId.set(correlationId, {
Expand All @@ -553,10 +533,6 @@ export abstract class PerformanceClient implements IPerformanceClient {
fields: { [key: string]: number | undefined },
correlationId: string
): void {
this.logger.trace(
"PerformanceClient: Updating counters",
correlationId
);
const event = this.eventsByCorrelationId.get(correlationId);
if (event) {
for (const counter in fields) {
Expand Down Expand Up @@ -587,21 +563,13 @@ export abstract class PerformanceClient implements IPerformanceClient {
protected cacheEventByCorrelationId(event: PerformanceEvent): void {
const rootEvent = this.eventsByCorrelationId.get(event.correlationId);
if (rootEvent) {
this.logger.trace(
`PerformanceClient: Performance measurement for '${event.name}' added/updated`,
event.correlationId
);
rootEvent.incompleteSubMeasurements =
rootEvent.incompleteSubMeasurements || new Map();
rootEvent.incompleteSubMeasurements.set(event.eventId, {
name: event.name,
startTimeMs: event.startTimeMs,
});
} else {
this.logger.trace(
`PerformanceClient: Performance measurement for '${event.name}' started`,
event.correlationId
);
this.eventsByCorrelationId.set(event.correlationId, { ...event });
this.eventStack.set(event.correlationId, []);
}
Expand All @@ -613,16 +581,7 @@ export abstract class PerformanceClient implements IPerformanceClient {
* @param {string} correlationId
*/
discardMeasurements(correlationId: string): void {
this.logger.trace(
"PerformanceClient: Performance measurements discarded",
correlationId
);
this.eventsByCorrelationId.delete(correlationId);

this.logger.trace(
"PerformanceClient: Event stack discarded",
correlationId
);
this.eventStack.delete(correlationId);
}

Expand Down
7 changes: 0 additions & 7 deletions lib/msal-common/src/utils/FunctionWrappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export const invoke = <T extends Array<any>, U>(
correlationId: string
) => {
return (...args: T): U => {
logger.trace(`Executing function '${eventName}'`, correlationId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

For customers not using the performance client how do these things get logged?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not going to be logged at all.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's going to be a problem when we receive issues/ICMs from external customers not using the 1P lib

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This isn't going to be a problem for 3p customers unless they inject their own performanceClient.

Copy link
Collaborator

Choose a reason for hiding this comment

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

3P customers aren't using performanceClient at all, I think we need to leave these logs here for those scenarios

const inProgressEvent = telemetryClient.startMeasurement(
eventName,
correlationId
Expand All @@ -41,7 +40,6 @@ export const invoke = <T extends Array<any>, U>(
inProgressEvent.end({
success: true,
});
logger.trace(`Returning result from '${eventName}'`, correlationId);
return result;
} catch (e) {
logger.trace(`Error occurred in '${eventName}'`, correlationId);
Expand Down Expand Up @@ -82,7 +80,6 @@ export const invokeAsync = <T extends Array<any>, U>(
correlationId: string
) => {
return (...args: T): Promise<U> => {
logger.trace(`Executing function '${eventName}'`, correlationId);
const inProgressEvent = telemetryClient.startMeasurement(
eventName,
correlationId
Expand All @@ -94,10 +91,6 @@ export const invokeAsync = <T extends Array<any>, U>(
}
return callback(...args)
.then((response) => {
logger.trace(
`Returning result from '${eventName}'`,
correlationId
);
inProgressEvent.end({
success: true,
});
Expand Down
8 changes: 2 additions & 6 deletions lib/msal-common/test/utils/FunctionWrappers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ describe("FunctionWrappers Unit Tests", () => {
end = jest.spyOn(inProgressMeasurement, "end");
return inProgressMeasurement;
});
const loggerSpy = jest.spyOn(logger, "trace");

const testCallback = (arg1: string, arg2: number): string => {
expect(arg1).toBe("arg1");
Expand All @@ -50,7 +49,6 @@ describe("FunctionWrappers Unit Tests", () => {
"testCorrelationId"
);
expect(end).toHaveBeenCalledWith({ success: true });
expect(loggerSpy).toHaveBeenCalledTimes(2);
});

it("failure", () => {
Expand Down Expand Up @@ -93,7 +91,7 @@ describe("FunctionWrappers Unit Tests", () => {
"testCorrelationId"
);
expect(end).toHaveBeenCalledWith({ success: false }, error);
expect(loggerSpy).toHaveBeenCalledTimes(3);
expect(loggerSpy).toHaveBeenCalledTimes(2);
});
});

Expand All @@ -110,7 +108,6 @@ describe("FunctionWrappers Unit Tests", () => {
end = jest.spyOn(inProgressMeasurement, "end");
return inProgressMeasurement;
});
const loggerSpy = jest.spyOn(logger, "trace");

const testCallback = (
arg1: string,
Expand All @@ -135,7 +132,6 @@ describe("FunctionWrappers Unit Tests", () => {
"testCorrelationId"
);
expect(end).toHaveBeenCalledWith({ success: true });
expect(loggerSpy).toHaveBeenCalledTimes(2);
});

it("failure", async () => {
Expand Down Expand Up @@ -180,7 +176,7 @@ describe("FunctionWrappers Unit Tests", () => {
"testCorrelationId"
);
expect(end).toHaveBeenCalledWith({ success: false }, error);
expect(loggerSpy).toHaveBeenCalledTimes(3);
expect(loggerSpy).toHaveBeenCalledTimes(2);
});
});
});