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

[Feature] Add HTTP Sink Connector #4824

Closed
2 of 3 tasks
Pil0tXia opened this issue Apr 10, 2024 · 9 comments · Fixed by #4837
Closed
2 of 3 tasks

[Feature] Add HTTP Sink Connector #4824

Pil0tXia opened this issue Apr 10, 2024 · 9 comments · Fixed by #4837
Assignees
Labels
feature help wanted Extra attention is needed

Comments

@Pil0tXia
Copy link
Member

Pil0tXia commented Apr 10, 2024

Search before asking

  • I had searched in the issues and found no similar issues.

Feature Request

Send CloudEvents to a specific URL of a HTTP server (like a SpringBoot application endpoint or more).

Needs to support Webhook payload delivary with a general protocol.

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@Pil0tXia Pil0tXia added help wanted Extra attention is needed feature labels Apr 10, 2024
@cnzakii
Copy link
Contributor

cnzakii commented Apr 10, 2024

2024-04-10_20 09 36
Is completing the above TODO part? I think I can give it a try. Please assign it to me. Thanks~~

@cnzakii
Copy link
Contributor

cnzakii commented Apr 13, 2024

@Pil0tXia

I've used OkHttpClient to build an HTTP Sink Connector.

I have some questions regarding message acknowledgment:
I currently check the HTTP response status code and whether the response body is empty to ensure message delivery success. Should we establish a response template specifying the expected response codes and messages to confirm whether messages were successfully processed, or to retrieve information about the reasons for processing failure?

Do you have any recommendations on this?

ps: The Http Source Connector simply returns an "empty" response with an HTTP status code.

@Pil0tXia
Copy link
Member Author

@cnzakii

I think an HTTP 200 response is enough to represent that the receiver processed the message correctly.

EventMesh is planning in the future to have statistics on the delivery status of events, when it will be necessary to show the results of Webhook deliveries, including what is returned on the sink side. You may leave an extensible design with comments.

Could you please use vertx which has been used in the HTTP Source Connector? Using the same third-party library improves code reusability and later extensibility, and avoids the problem that the Source Connector may produce faster than the Sink Connector can consume.

@cnzakii
Copy link
Contributor

cnzakii commented Apr 13, 2024

@Pil0tXia

OK, I will make the modifications based on your suggestions.

@cnzakii
Copy link
Contributor

cnzakii commented Apr 14, 2024

@Pil0tXia

I have a dependency issue to resolve in the eventmesh-connector-http module. The version io.cloudevents:cloudevents-http-vertx:2.3.0 uses Vert.x version 4.0.0, conflicting with io.vertx:vertx-web:4.4.6, causing issues with vertx.WebClient functionality.

My current approach is:

  • Upgrading io.cloudevents:cloudevents-http-vertx from 2.3.0 to 3.0.0.
  • Downgrading io.vertx:vertx-web from 4.4.6 to 4.3.7.

This is because the latest version of io.cloudevents:cloudevents-http-vertx available is 3.0.0, which uses Vert.x version 4.3.7.

@Pil0tXia
Copy link
Member Author

@cnzakii

This is the current version in master branch:

implementation 'io.cloudevents:cloudevents-http-vertx:2.3.0'
implementation 'io.vertx:vertx-web:4.4.6'

The version io.cloudevents:cloudevents-http-vertx:2.3.0 uses Vert.x version 4.0.0, conflicting with io.vertx:vertx-web:4.4.6

For the same artifact, Gradle automatically uses the latest version in the dependency tree. If the old version is completely unupgradeable (which is important because upgrading a version is almost always better than downgrading), you can exclude transitive dependencies from conflicting artifacts.

When upgrading, read the release note to avoid unintentionally breaking the functionality of the original code.

My understanding of the situation you described is this:

  • While the existing io.vertx:vertx-web:4.4.6 and io.cloudevents:cloudevents-http-vertx:2.3.0 work well with the HTTP Source Connector, the WebClient class where HTTP Sink Connector required to introduce does not work with this version.
  • In order for the WebClient to work properly, io.cloudevents:cloudevents-http-vertx:2.3.0 needs to be upgraded to io.cloudevents:cloudevents-http-vertx:3.0.0.
  • io.clouddevents:clouddevents-http-vertx:3.0.0 and the existing io.vertx:vertx-web:4.4.6 still do not allow WebClient to work properly, and io.vertx:vertx-web:4.4.6 must be downgraded to io.vertx:vertx-web:4.3.7 (is this step correct? For most artifacts, higher versions are backwards compatible)

@Fungx May I ask that what do you think of @cnzakii 's dependency conflict?

@cnzakii
Copy link
Contributor

cnzakii commented Apr 14, 2024

@Pil0tXia

Upon further investigation, I've determined that the dependency issue is unrelated to io.vertx:vertx-web because this dependency does not include vertx-web-client. In the original dependency, only io.cloudevents:cloudevents-http-vertx:4.4.6 includes the dependency io.vertx:vertx-web-client:4.0.0.

Therefore, we have two solutions:

  1. Upgrading io.cloudevents:cloudevents-http-vertx from 2.3.0 to 3.0.0 to ensure it includes io.vertx:vertx-web-client upgraded to 4.3.7.
  2. Adding the dependency io.vertx:vertx-web-client:4.4.6.

@Pil0tXia
Copy link
Member Author

@cnzakii

Therefore, we have two solutions:

Please do both. I've read the release notes of https://github.com/cloudevents/sdk-java, and it seems that there's no breaking change related to vertx between 2.3.0 and 3.0.0.

According to https://vertx.io/docs/vertx-web-client/java/#_using_the_web_client, developers should declare their dependencies explicitly. I think the latest version won't have compatibility issues either.

@cnzakii
Copy link
Contributor

cnzakii commented Apr 14, 2024

@Pil0tXia PTAL

@mxsm mxsm closed this as completed in #4837 May 8, 2024
mxsm pushed a commit that referenced this issue May 8, 2024
* feat: Add HTTP Sink Connector

* refactor: Replace okHttpClient with vertx.WebClient

* fix: Resolving dependency conflicts

* test: Add HttpSinkConnectorTest

* fix: Add License

* fix: Solving dependency issues

* fix: License Check

* feat: Add HTTPS/SSL support

* fix: Optimize logging

* feat: Add webhook functionality

* fix: Fix some bugs

* test: add callback test

* refactor: Add webhook Support

* fix: Optimization tests and configuration additions

* fix: code style

* feat: rebuild WebhookHttpSinkHandler and add RetryHttpSinkHandler

* fix: fix ci

* refactor: Use failsafe alternative resilience4j and optimize webhook functionality

* fix: fix License Check

* fix: update something

* fix: fix ci

* fix: update something

* fix: Optimized naming

* fix: fix ci

* fix: fix style check error

* test: update HttpSinkConnectorTest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants