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

[exporter/logging] Add syscall.EBADF to list of known sync errors #5585

Merged

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Jun 23, 2022

Description: Adds syscall.EBADF ('bad file descriptor') to the list of known sync errors.

Link to tracking Issue: Fixes #4153

Testing: I tested that this fixes the test skipped in DataDog/datadog-agent#12502

@codecov
Copy link

codecov bot commented Jun 23, 2022

Codecov Report

Merging #5585 (90252fa) into main (d63aea3) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #5585      +/-   ##
==========================================
- Coverage   91.27%   91.24%   -0.04%     
==========================================
  Files         191      191              
  Lines       11304    11308       +4     
==========================================
  Hits        10318    10318              
- Misses        785      789       +4     
  Partials      201      201              
Impacted Files Coverage Δ
exporter/loggingexporter/known_sync_error.go 0.00% <0.00%> (ø)

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 d63aea3...90252fa. Read the comment docs.

Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

Is there a performance impact to run through a loop versus the hard-coded OR statement?

@mx-psi
Copy link
Member Author

mx-psi commented Jun 28, 2022

Is there a performance impact to run through a loop versus the hard-coded OR statement?

@TylerHelmuth AIUI this is only called here

func loggerSync(logger *zap.Logger) func(context.Context) error {
which in turn is only called when shutting down the exporter, so I would argue readability is more important than performance here.

@TylerHelmuth
Copy link
Member

AIUI this is only called here

func loggerSync(logger *zap.Logger) func(context.Context) error {

which in turn is only called when shutting down the exporter, so I would argue readability is more important than performance here.

In that case I would agree.

@bogdandrutu bogdandrutu merged commit 5ead47c into open-telemetry:main Jun 30, 2022
@mx-psi mx-psi deleted the mx-psi/sync-error-logging-exporter branch June 30, 2022 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants