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

BREAK: Introduce otel middleware and require kstream >= 0.17 #7

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

woile
Copy link
Member

@woile woile commented Oct 1, 2024

This PR no longer patches getone, but instead, leverages the middleware system from kstreams.
It injects a middleware at the very beginning of the middleware stack.
In order to do this, we now require kstreams >= 0.17 which introduced middlewares

Unlike confluent_kafka otel we don't wait for the next event to close the span, it's all managed in the middleware, this create accurate timestamps.

Copy link

@marcosschroh marcosschroh left a comment

Choose a reason for hiding this comment

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

Can you add a test which includes more middlewares? For example, a Stream has a S3Middleware and the end users adds the OpenTelemetryMiddleware.

Then the assert Stream.middleware == [ExceptionMiddleware, OpenTelemetryMiddleware, S3Middleware] and the calls stack should be ExceptionMiddleware -> OpenTelemetryMiddleware -> S3Middleware -> UdfHandler

@woile woile force-pushed the fix/updates branch 2 times, most recently from e09868f to ae8a7c0 Compare October 2, 2024 11:10
@woile woile merged commit 1c80551 into main Oct 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants