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 stats option to analyze command for performance insights #1237

Merged
merged 7 commits into from
Oct 30, 2024

Conversation

matthisholleville
Copy link
Contributor

Closes #

📑 Description

Introduced a new feature to the analyze command that enables users to print detailed performance statistics of each analyzer. This enhancement aids in debugging and understanding the time taken by various components during analysis, providing valuable insights for performance optimization.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

(matthisholleville) ➜  k8sgpt git:(main) ✗ time ./bin/k8sgpt analyze -s
The stats mode allows for debugging and understanding the time taken by an analysis by displaying the statistics of each analyzer.
- Analyzer Ingress took 47.125583ms 
- Analyzer PersistentVolumeClaim took 53.009167ms 
- Analyzer CronJob took 57.517792ms 
- Analyzer Deployment took 156.6205ms 
- Analyzer Node took 160.109833ms 
- Analyzer ReplicaSet took 245.938333ms 
- Analyzer StatefulSet took 448.0455ms 
- Analyzer Pod took 5.662594708s 
- Analyzer Service took 38.583359166s 

Introduced a new feature to the analyze command that enables users to print detailed performance statistics of each analyzer. This enhancement aids in debugging and understanding the time taken by various components during analysis, providing valuable insights for performance optimization.

Signed-off-by: Matthis Holleville <[email protected]>
@matthisholleville matthisholleville requested review from a team as code owners August 26, 2024 12:15
@matthisholleville
Copy link
Contributor Author

matthisholleville commented Aug 26, 2024

Linked to this issue #1236

@AlexsJones
Copy link
Member

If we do -o json -s what will happen, will it add a field into the json object?

@matthisholleville
Copy link
Contributor Author

The current implementation does not support json as an output.

@matthisholleville
Copy link
Contributor Author

When this option is selected, the results are not displayed. Only the statistics are shown. If we try to display both, in the case of an analysis with many results, it becomes very difficult to visualize the statistics.

pkg/analysis/analysis.go Outdated Show resolved Hide resolved
@AlexsJones
Copy link
Member

The current implementation does not support json as an output.

Might be worth considering if we want to show the results and time taken

@matthisholleville
Copy link
Contributor Author

@AlexsJones stats implementation is global to analyzer. The results display the errors reported by an analyzer. In my opinion, these are two different notions. We could also display the stats in json format, if that's useful.

README.md Show resolved Hide resolved
AlexsJones and others added 5 commits August 30, 2024 21:46
Refactored the analysis command to support an enhanced statistics option, enabling users to opt-in for detailed performance metrics of the analysis process. This change introduces a more flexible approach to handling statistics, allowing for a clearer separation between the analysis output and performance metrics, thereby improving the usability and insights provided to the user.

Signed-off-by: Matthis Holleville <[email protected]>
Copy link
Member

@AlexsJones AlexsJones left a comment

Choose a reason for hiding this comment

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

Overall this is a really great PR and an enormous step in the right direction. Thanks for this @matthisholleville

@arbreezy arbreezy self-requested a review October 30, 2024 12:30
@AlexsJones AlexsJones merged commit 3eec9bb into k8sgpt-ai:main Oct 30, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants