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

Only log errors and above when benchmarking #261

Merged
merged 1 commit into from
Mar 2, 2022

Conversation

AndrewSisley
Copy link
Contributor

@AndrewSisley AndrewSisley commented Mar 1, 2022

Closes #260

Is very easy to disable it entirely if preferred, setting the config.outputPaths to empty instead (with a couple of tweaks in the config code) - let me know

@AndrewSisley AndrewSisley added bug Something isn't working area/logging Related to the logging/logger system labels Mar 1, 2022
@AndrewSisley AndrewSisley self-assigned this Mar 1, 2022
@codecov
Copy link

codecov bot commented Mar 1, 2022

Codecov Report

Merging #261 (92b6166) into develop (38c4b2c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #261   +/-   ##
========================================
  Coverage    58.49%   58.49%           
========================================
  Files          103      103           
  Lines         9834     9836    +2     
========================================
+ Hits          5752     5754    +2     
  Misses        3454     3454           
  Partials       628      628           
Impacted Files Coverage Δ
bench/bench_util.go 10.60% <100.00%> (+1.37%) ⬆️

@orpheuslummis
Copy link
Contributor

@shahzadlone I'm wondering if errors being outputted in benchmarking matter for #232 ?

@shahzadlone
Copy link
Member

#232 is simply the comparison of benchmarks. If more errors are hit and emitted then it will likely show a hit in performance.

Copy link
Contributor

@orpheuslummis orpheuslummis left a comment

Choose a reason for hiding this comment

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

LGTM

@orpheuslummis
Copy link
Contributor

Given this PR doesn't have a conventional commit prefix: Just remember to add the fix: prefix to the commit to be merged

@AndrewSisley AndrewSisley merged commit c5108c4 into develop Mar 2, 2022
@AndrewSisley AndrewSisley deleted the sisley/fix/I260-disable-bench-logger branch March 2, 2022 14:24
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logging Related to the logging/logger system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop benchmarks from spamming logs
3 participants