contrib/labstack/echo.v4: add option to ignore errors

[mrkagelui]

What does this PR do?

add option to ignore errors in tracing for echo framework

Motivation

I want to control which error should be tagged in spans and which should not

Describe how to test/QA your changes

unit tests should be able to cover this change

Reviewer's Checklist

• If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
• Changed code has unit tests for its functionality.
• If this interacts with the agent in a new way, a system test has been added.

[mrkagelui]

@gbbr sorry, I'm just tagging a random internal member. do you think I can get any review of this?

[katiehockman]

Hello! Would you please file an issue describing your precise use case so we know what problem you're running into?
Based on your issue, we'd like to hear whether or not #865 could meet your needs.

[mrkagelui]

Hello! Would you please file an issue describing your precise use case so we know what problem you're running into? Based on your issue, we'd like to hear whether or not #865 could meet your needs.

Hi thanks for replying! primarily for me this is to control the status code going in error-tagged spans, but I think it's also useful in other areas such as controlling what types of custom errors should/shouldn't be tagged. also there doesn't seem to be much progress on that issue, I'd like this to be handled separately.

[rarguelloF]

@mrkagelui thanks for your PR! the feature looks good, just some minor nits to keep the option consistent with other contribs.

[mrkagelui]

@mrkagelui thanks for your PR! the feature looks good, just some minor nits to keep the option consistent with other contribs.

ok, will rebase and address these soon (in one or two day)

[mrkagelui]

done @rarguelloF

[mrkagelui]

By the way, if it's not for consistency reasons, I'd rather name it ignoreError, because it's clearer what the return of function means (ignore if true), errCheck is very ambiguous.

[mrkagelui]

Be great if we can have some review/approval before it gets outdated again 😀 @rarguelloF

[mrkagelui]

thanks for running and let me look into the failed tests

[rarguelloF]

@mrkagelui thanks for addressing the comments from last review, it's looking good!

I found a small issue that I missed in the last review, please take a look at my comment and let me know what you think!

By the way, if it's not for consistency reasons, I'd rather name it ignoreError, because it's clearer what the return of function means (ignore if true), errCheck is very ambiguous.

I agree the name is probably improvable 😅 but in this case we prefer to give a consistent experience to the library users. Let's just make sure the public docs make it clear what the behaviour is (as you already did 🙂 ).

[rarguelloF]

👋 @mrkagelui

For the sake of moving forward with the PR, I went ahead and I added the changes discussed in here: #1567 (comment)

It will be merged as soon as its reviewed and approved by someone from @DataDog/tracing-go

Thanks so much for your contribution and for your patience!