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.

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

Jump to bottom

Add logging level guarding #1118

Open
11 tasks
Nana-EC opened this issue Apr 22, 2023 · 6 comments · May be fixed by #3154
Open
11 tasks

Add logging level guarding #1118

Nana-EC opened this issue Apr 22, 2023 · 6 comments · May be fixed by #3154
Assignees
@belloibrahv
Labels
enhancement New feature or request good first issue candidate Issues that can become a good first issue but need more description/context. hacktoberfest Issues shown by lists for the Hacktoberfest and made for newcomers to do the first contribution. P3 perf performance gains

Comments

@Nana-EC
Copy link
Collaborator

Nana-EC commented Apr 22, 2023 •
edited by hendrikebbers
Loading

🆕🐥 First Timers Only

This issue is reserved for people who have never contributed to Hedera or any open source project in general.
We know that creating a pull request (PR) is a major barrier for new contributors.
The goal of this issue and all other issues labeled by 'Good First Issue' is to help you make your first contribution to Hedera.

👾 Description of the issue

The relay utilizes debug and trace logs without guarding (if-statements).
This can result in unnecessary string generation when such levels are not applicable

Proposed Solution:

Add log level considerations and guard appropriately
See https://github.com/pinojs/pino/blob/master/docs/api.md#loggerislevelenabledlevel

📋 Step by step guide to do a contribution

If you have never contributed to an open source project at GitHub, the following step-by-step guide will introduce you to the workflow. More information and concrete samples for shell commands for each step can be found in our CONTRIBUTING.md file.
A more detailed general documentation of the GitHub PR workflow can be found here.

  • Claim this issue: Comment below that you are interested in working on the issue
  • Wait for assignment: A community member with the given rights will add you as an assignee of the issue
  • Fork the repository: You can do that in GitHub (by simply clicking the 'fork' button).
  • Check out the forked repository
  • Create a feature branch for the issue. We do not have a hard naming definition for branches but it is best practice to prefix the branch name with the issue id.
  • Solve the issue in your branch.
  • Commit your changes: Here, it is needed to add sign-off information to the commit to accept the "Developer Certificate of Origin" (https://developercertificate.org). More details can be found in our CONTRIBUTING.md
  • Start a Pull Request (PR): We have a pattern for naming pull requests that a GitHub Action checks. We use that pattern to support the creation of automatic release notes.
  • Check GitHub Actions: Several GitHub Actions will be triggered automatically for each PR. If a GitHub Action fails and you do not understand the cause of that error do not hesitate to add a comment to the PR and ask the Hedera developer community for support.
  • Wait for reviews: Members of the Hedera developer community will review your PR. If a reviewer finds any missing pieces or a problem, he or she will start a discussion with you and describe the next steps for solving the problem.
  • You did it 🎉: We will merge the fix in the develop branch. Thanks for being part of the Hedera community as an open-source contributor ❤️

🎉 Contribute to Hacktoberfest

Solve this issue as part of the Hacktoberfest event and get a chance to receive cool goodies like a T-Shirt. 🎽

🤔 Additional Information

If you have any questions, just ask us directly in this issue by adding a comment. You can join our community chat at Discord. A general manual about open-source contributions can be found here.
@Nana-EC Nana-EC added enhancement New feature or request good first issue Good for newcomers P3 perf performance gains labels Apr 22, 2023
@Nana-EC Nana-EC added the good first issue candidate Issues that can become a good first issue but need more description/context. label Sep 16, 2024
@hendrikebbers hendrikebbers removed the good first issue Good for newcomers label Sep 30, 2024
@ebadiere ebadiere added the hacktoberfest Issues shown by lists for the Hacktoberfest and made for newcomers to do the first contribution. label Oct 11, 2024
@belloibrahv
Copy link

belloibrahv commented Oct 17, 2024 •
edited
Loading

Summary of My Understanding:

From what I understand, the issue focuses on improving performance by preventing unnecessary string generation in debug and trace logs when these logging levels are disabled. The proposed solution involves adding log level guards (if-statements) to ensure string generation only occurs when the logging level is enabled. I’m looking to confirm my understanding of the requirements before proceeding.

Here are some questions I’d appreciate clarification on:

  1. Could you please confirm if the main issue is related to performance inefficiencies caused by unnecessary string generation, or are there additional concerns such as incorrect logging levels being used?
  2. Should the log level guards be implemented for both debug and trace logs, or are there additional logging levels we should consider when applying the guards?
  3. Could you guide me to the specific files or modules where the majority of these unguarded logs appear, or should I apply the changes across all components of the relay?
  4. Are there any existing unit or integration tests related to logging behavior that I should reference when adding log level guards? If not, would you recommend adding test coverage for this?
  5. Is there a preferred pattern or convention for adding if-statement guards in this project to maintain consistency, or should I follow the example provided in the pino documentation?
  6. I’ve already expressed my interest in this issue. Could you provide a rough timeline for when I might expect to be officially assigned so I can plan accordingly?

@belloibrahv
Copy link

@ebadiere @hendrikebbers @Nana-EC could please assign this issue for me to work on? thank you.

@Ashu1823
Copy link

Hi @Nana-EC @hendrikebbers @ebadiere . Could you please assign this task to me.

@Nana-EC
Copy link
Collaborator Author

Nana-EC commented Oct 23, 2024

First come first serve, @belloibrahv assigning to you.
Thanks @Ashu1823

@belloibrahv
Copy link

@Nana-EC thanks for assigning the task to me. Already working it.

@belloibrahv belloibrahv linked a pull request Oct 24, 2024 that will close this issue
Open
2 tasks
@belloibrahv
Copy link

Hi @ebadiere @hendrikebbers @Nana-EC , could you please review my new PR #3154 when you have a moment? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@belloibrahv belloibrahv

Labels
enhancement New feature or request good first issue candidate Issues that can become a good first issue but need more description/context. hacktoberfest Issues shown by lists for the Hacktoberfest and made for newcomers to do the first contribution. P3 perf performance gains
Projects
Smart Contract Sprint Board
Status: Tasks In Progress
Milestone
No milestone
5 participants
@ebadiere @hendrikebbers @Nana-EC @Ashu1823 @belloibrahv