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

Reduced logging level for collector shutdown due to context cancel #5258

Merged
merged 4 commits into from
Apr 26, 2022

Conversation

cpheps
Copy link
Contributor

@cpheps cpheps commented Apr 26, 2022

Description: <Describe what has changed.
Resolves #5242

Reduced the logging level when the collector exits due to a context cancel. When the collector is exited with a canceled context it would print an error with a stack trace. This lead to confusing logs when the collector correctly shut down due to a context that was closed by signal intercepts.

Kept the context error log if the error is context.DeadlineExceeded as it's much more likely if the collector exceeded some deadline the user should be notified of such.

Link to tracking Issue: #5242

Testing: Tested locally with signal intercepts. Current unit tests are valid.

@cpheps cpheps requested review from a team and dmitryax April 26, 2022 00:17
@codecov
Copy link

codecov bot commented Apr 26, 2022

Codecov Report

Merging #5258 (277413c) into main (a8ad007) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5258      +/-   ##
==========================================
- Coverage   90.60%   90.57%   -0.04%     
==========================================
  Files         188      188              
  Lines       11092    11093       +1     
==========================================
- Hits        10050    10047       -3     
- Misses        822      825       +3     
- Partials      220      221       +1     
Impacted Files Coverage Δ
service/collector.go 77.57% <100.00%> (+0.13%) ⬆️
pdata/internal/common.go 94.16% <0.00%> (-0.78%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8ad007...277413c. Read the comment docs.

@cpheps
Copy link
Contributor Author

cpheps commented Apr 26, 2022

@bogdandrutu Refactored the logging to print the error string from the context in the log so we still see the error but not the stack trace. I also removed the special deadline case.

Corbin Phelps added 4 commits April 26, 2022 15:53

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Signed-off-by: Corbin Phelps <corbin.phelps@bluemedora.com>
Signed-off-by: Corbin Phelps <corbin.phelps@bluemedora.com>
Signed-off-by: Corbin Phelps <corbin.phelps@bluemedora.com>
Signed-off-by: Corbin Phelps <corbin.phelps@bluemedora.com>
@cpheps cpheps force-pushed the ctx-done-logging branch from 8197f12 to 277413c Compare April 26, 2022 19:54
@bogdandrutu bogdandrutu merged commit 15d79e6 into open-telemetry:main Apr 26, 2022
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.

Collector Stopping Due to Context Cancel is Noisy
3 participants