Skip to content

Log most expensive predicates and timings to query log #1349

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.

Sign up for GitHub

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

Merged
angelapwen merged 5 commits into from
May 20, 2022
Merged

Log most expensive predicates and timings to query log #1349

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
df61cac
Log most expensive predicates and timings to query log
angelapwen May 16, 2022
d4a04a4
Update CHANGELOG.md
angelapwen May 16, 2022
07a8c1b
Stop printing end of query summaries to Extension Log
angelapwen May 20, 2022
b2fe057
Merge remote-tracking branch 'upstream/main' into add-summary-to-quer…
angelapwen May 20, 2022
326b213
Merge branch 'main' into add-summary-to-query-log
angelapwen May 20, 2022
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
3 changes: 3 additions & 0 deletions extensions/ql-vscode/src/run-queries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ export class QueryEvaluationInfo {
}
void qs.logger.log(' --- Evaluator Log Summary --- ');
void qs.logger.log(buffer.toString());
// Write summary to Query Log as well, as this information was present here before structured logging.
void logger.log(' --- Evaluator Log Summary --- ', { additionalLogLocation: this.logPath });
void logger.log(buffer.toString(), { additionalLogLocation: this.logPath });
Copy link
Copy Markdown
Contributor Author

@angelapwen angelapwen May 16, 2022

Choose a reason for hiding this comment

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

Note that this will also log to the "CodeQL Extension Log" console view as well, so this information is showing in many different places (Query Server console and Structured Evaluator Log (Summary), and now Query Log and Extension Log). Does this seem too noisy?

Copy link
Copy Markdown
Contributor

@aeisenberg aeisenberg May 17, 2022

Choose a reason for hiding this comment

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

Hmmm...not sure. It seems like query information is best kept in the query log. But, if people are asking for it in the extension log as well, that's probably fine.

Copy link
Copy Markdown
Contributor Author

@angelapwen angelapwen May 17, 2022

Choose a reason for hiding this comment

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

Nobody is actually asking for it in the extension log, only in the query log. But I was under the impression that in order to log to the query log, I had to also log it to the extension log like above. logger.log() without the additionalLogLocation only writes to the extension log, I believe?

Copy link
Copy Markdown
Contributor

@adityasharad adityasharad May 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Which log it writes to depends on the logger object at construction time. We have three global logger objects, one each for the extension log, the query server log, and the language server log. Where does this variable come from?

I also think it is best kept in the query server log.

Copy link
Copy Markdown
Contributor Author

@angelapwen angelapwen May 17, 2022

Choose a reason for hiding this comment

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

A ha, I see. I can just continue to use the query server logger object and log to an additional location of the query log (as requested).

});
} else {
void showAndLogWarningMessage(`Failed to write structured evaluator log to ${this.evalLogPath}.`);
Expand Down