Skip to content

Logging performance improvements #713

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

Merged
merged 14 commits into from
Oct 14, 2021
Merged

Conversation

zashraf1985
Copy link
Contributor

@zashraf1985 zashraf1985 commented Oct 13, 2021

Summary

Made improvements to logging performance. Log messages were previously being concatenated even if log level didn't apply. modified it to do the concatenation only when log level applies. this improves SDK performance specially in case of log level ERROR.

Test plan

  • All existing tests pass

@coveralls
Copy link

coveralls commented Oct 13, 2021

Coverage Status

Coverage increased (+0.05%) to 97.117% when pulling 8a40563 on zeeshan/logging-performance into d4ff5bd on master.

@zashraf1985 zashraf1985 force-pushed the zeeshan/logging-performance branch from a0a4dd8 to 8a40563 Compare October 13, 2021 21:23
@zashraf1985 zashraf1985 changed the title modifed logging mechanism to improve performance Logging performance improvements Oct 13, 2021
@zashraf1985 zashraf1985 marked this pull request as ready for review October 13, 2021 21:32
@zashraf1985 zashraf1985 requested a review from a team as a code owner October 13, 2021 21:32
@zashraf1985 zashraf1985 removed their assignment Oct 13, 2021
Copy link
Contributor

@yavorona yavorona left a comment

Choose a reason for hiding this comment

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

LGTM!

@zashraf1985 zashraf1985 merged commit e2b30fe into master Oct 14, 2021
@zashraf1985 zashraf1985 deleted the zeeshan/logging-performance branch October 14, 2021 15:46
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.

3 participants