Propagate error to observability service #715
Conversation
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com> Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
joaopgrassi
force-pushed
the
obs-empty-errors
branch
from
bfea7ac
to
5ec17a2
Compare
| @@ -186,7 +184,7 @@ func (c *ceClient) Request(ctx context.Context, e event.Event) (*event.Event, pr | |||
| } else { | |||
| resp = rs | |||
| } | |||
|
|
|||
| defer cb(err, resp) | |||
There was a problem hiding this comment.
this change should not do anything as err and resp are defined here: https://github.com/cloudevents/sdk-go/pull/715/files#diff-bfa445e7f19a76da55d2f4a1f51a4dd9cb7e2bf54efd28fd96428078c1453732R139-R140 and are never recreated in the function
There was a problem hiding this comment.
I was wrong. It seems to matter for error type. which is annoying...
|
|
||
| resp, result = r.fn.invoke(ctx, e) | ||
| defer cb(result) |
There was a problem hiding this comment.
same here, can you confirm the new test fails when the defer order is changed to the original?
|
You are correct. https://play.golang.org/p/PyVqhb3LVKM Interesting! I expected the interface pointers to not be reassigned, but clearly there is a go-ism I don't understand on that one. Could you please resort the imports, and then LGTM Thanks for the PR and the issue. |
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
joaopgrassi
force-pushed
the
obs-empty-errors
branch
from
beb9ce1
to
447e15e
Compare
|
@n3wscott not sure why the integration tests failed, they also failed once in our fork and re-triggering it worked. I guess it's some network issue? Since it seems it was waiting for 12min.. |
|
LGTM Thanks a ton for the fix @joaopgrassi !! |
Call defer in the appropriate location, so the observability service callback is invoked with the error object populated. This allows to record the error into the span, or set the span as failed.
Fixes #713