Passing message to record malformed event #965
Conversation
|
still testing, but I think the race condition will be fixed by #958 |
|
please rebase |
Signed-off-by: Muhammad Raza <[email protected]>
raza-matillion
force-pushed
the
964-pass-message-to-malformed-event
branch
from
bda8308
to
5e1a96b
Compare
|
@duglin Rebase done! |
Bumps [go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp](https://github.com/open-telemetry/opentelemetry-go-contrib) from 0.23.0 to 0.44.0. - [Release notes](https://github.com/open-telemetry/opentelemetry-go-contrib/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-go-contrib@v0.23.0...zpages/v0.44.0) --- updated-dependencies: - dependency-name: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Muhammad Raza <[email protected]>
Bumps [go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp](https://github.com/open-telemetry/opentelemetry-go-contrib) from 0.23.0 to 0.44.0. - [Release notes](https://github.com/open-telemetry/opentelemetry-go-contrib/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-go-contrib@v0.23.0...zpages/v0.44.0) --- updated-dependencies: - dependency-name: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Muhammad Raza <[email protected]>
Signed-off-by: Muhammad Raza <[email protected]>
|
@duglin Resolved conflicts and updated PR. Sneaked one more tiny change in |
|
@duglin test seems flakey. Its passing locally on my machine so probably need a rerun. |
|
rebase again and let's see how it goes... |
|
@duglin done! |
|
I'm hoping #970 fixes the errors you're seeing.... |
|
ok one more rebase... hopefully for the win! |
|
I'm confused as to why this is associated with the Observability service |
|
@duglin For logging purpose. Basically I want to get malformed message and log it. If there is better approach i.e. if we can pass error handler as an extra client option, I am ok with that. At the moment, there is no way I can get hold of malformed event so that I can investigate it further. |
|
I shouldn't admit this, but I'm not as up to speed on the SDK as I should be :-) but I just can't help but wonder why we're not doing something like extending the @embano1 @lionelvillard any thoughts on whether extending the Obs. service is the best approach? |
|
Yeah, we should think about how we want to give users an option to consume events/requests throughout the various stages of event handling logic in the SDK. We can log, use callbacks, change signatures, or pass objects instead of nil. So we should discuss this a bit more. Perhaps we also need to distinguish for the implementation/PR(s) between error and success cases. |
|
Doing some digging/learning and I have a few questions for @embano1 and @lionelvillard ... It appears that people are supposed to setup a ceClient and then call StartReceiver() with the call-back Fn. In StartReceiver() it'll setup a receiveInvoker (https://github.com/cloudevents/sdk-go/blob/main/v2/client/client.go#L206) with all of the various options for processing the incoming event. However, I'm wondering why StarrtReceiver() takes the call-back Fn as a parameter instead of it being one of the ceClient options? Is this because there's an assumption that there can be more than one StartReceiver() running at the same time and each might need a different event processor? This line: https://github.com/cloudevents/sdk-go/blob/main/v2/client/client.go#L202 would seem to indicate "no", it's a singleton. It's probably not a big deal, I'm just wondering why this one option (the call-back Fn) is special from all of the other ceClient options (like Decorators) and not setup the same way? And, this popped in my head because I was trying to think about how we might want to setup a DLQ - to me a DLQ isn't much different from the call-back Fn so for consistency I thought we should follow the same pattern. But, I'm not sure I'd want to extend the StartReceiver() signature with another (optional) arg. |
|
Or maybe it's special because it's the only required param? I could buy that. So, either way, what do you guys think about adding a DQL field in receiveInvoker? |
Issue: #964