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

feat: add elapsed time decorator to apex node class methods #349

Merged
merged 6 commits into from
Mar 6, 2024

Conversation

peternhale
Copy link
Collaborator

@peternhale peternhale commented Feb 19, 2024

This PR add detailed elapsed time debug log data for most of the class methods in this module.
The log records are typically enabled using debug log level, with a few methods that do a lot of string manipulation and tagged with debug log level trace.

This is an investigative step in trying to diagnose customer reported delays in commands not finishing (actually hanging) after the command in question has finished rendering the results, or commands that are taking an inordinately log time to run, such as test runs in a few seconds but command take 35 minutes to complete.

@W-15130974@

There should be no perceived change in running commands, however with log level set to debug or trace, the CLI log files will include elapsed time data gathered during command execution

Copy link
Contributor

Choose a reason for hiding this comment

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

why are pretty much all of these tests removed?

@CristiCanizales
Copy link
Contributor

nit: is the WI in the description the right one?

@peternhale
Copy link
Collaborator Author

nit: is the WI in the description the right one?

Not really, I can create another and update the PR

@peternhale
Copy link
Collaborator Author

nit: is the WI in the description the right one?

Not really, I can create another and update the PR

@CristiCanizales corrected work item.

Copy link
Contributor

@CristiCanizales CristiCanizales left a comment

Choose a reason for hiding this comment

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

LGTM!
:shipit:

import { Logger, LoggerLevel } from '@salesforce/core';
import { LoggerLevelValue } from '@salesforce/core/lib/logger/logger';

const logThis = (
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this always makes for confusion in a js/ts world. Maybe just call this log? I could see this getting exported for use as need across the module. (doesn't have to be now, just when needed.)

Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

This looks awesome. great stuff.

Copy link
Member

@mingxuanzhangsfdx mingxuanzhangsfdx left a comment

Choose a reason for hiding this comment

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

I tested it locally and it works fine! Thanks for the change.

@peternhale peternhale merged commit 5e9134e into main Mar 6, 2024
9 checks passed
@peternhale peternhale deleted the phale/W-14990874-elapsed-time branch March 6, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants