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

add error checks for elasticsearch exporters #9966

Closed

Conversation

DiptoChakrabarty
Copy link
Contributor

Description:

adds error check in elasticsearch exporters

Link to tracking Issue: #9749

Testing:

Documentation:

Comment on lines 152 to 153
indexererr := e.bulkIndexer.Add(ctx, item)
if indexererr != nil {
Copy link
Member

@bogdandrutu bogdandrutu May 12, 2022

Choose a reason for hiding this comment

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

I prefer this (and everywhere else) to inline this:

if indexererr := e.bulkIndexer.Add(ctx, item); indexererr != nil {
   ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have done this will push as all improvements suggested are finalised

@@ -250,17 +250,28 @@ func (doc *Document) iterJSON(v *json.Visitor, dedot bool) error {
}

func (doc *Document) iterJSONFlat(w *json.Visitor) error {
Copy link
Member

Choose a reason for hiding this comment

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

Name the return "argument" err and in the defer func do err = w.OnObjectFinished(), then instead of log.Fatal you will actually return an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if this is done the error returned from the defers will trigger the errcheck lint check as the errors returned from the defers are not checked

These are some related issues found
kisielk/errcheck#101
kubernetes-sigs/cluster-api#4586

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, can you show me?

func (doc *Document) iterJSONFlat(w *json.Visitor) (err error) {
	err = w.OnObjectStart(-1, structform.AnyType)
	if err != nil {
		return 
	}
	defer func() {
		err = w.OnObjectFinished()
	}()
        ....
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done like this

defer func() error {
		err = w.OnObjectFinished()
		if err != nil {
			return err
		}
		return nil
	}()

getting this
Selection_005

Copy link
Member

Choose a reason for hiding this comment

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

@DiptoChakrabarty, please take a closer look at Bogdan's example. You cannot return the error from the deferred function, but you can assign the error to a return value on the calling function.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants