Introduce ElasticSearch event logger for server events #5488

bowenliang123 · 2023-10-20T06:51:59Z

Why are the changes needed?

introduce new event logger type ELASTICSEARCH for server
send server events to the ElasticSearch index
support auto-created index with dynamic mapping type
use elastic4s 8.9.1 as ElasticSearch client for Scala, which depends on the ElasticSearch Java Client elasticsearch-rest-client 8.9.x.
- License: elastic4s and elasticsearch-rest-client released with Apache Licence 2.0. elasticsearch-rest-client does not depends on the ElasticSearch server library.
Tested with the latest release version of ElasticSearch server 7.x and 8.x

How was this patch tested?

Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

No.

cfmcgrady · 2023-10-20T07:24:45Z

@wForget has made similar contributions in #1689, and we have several questions regarding the Elasticsearch license, see #1689 (comment).

bowenliang123 · 2023-10-20T07:37:49Z

@wForget has made similar contributions in #1689, and we've got some queries regarding the Elasticsearch license. see #1689 (comment)

Oh, thanks for raising this. Sounds like more investigation is required in the scope of licence to clarify the impact for high/low-level client and the runtime usage of ES server for testing.

bowenliang123 · 2023-10-21T12:58:21Z

Hi @tisonkun, may I have your opinion on these licence concerns?
From your experience in different ASF projects, is it all right to use and ship with the ElasticSearch client (high-level Java client may depend on ElasticSearch common modules) , while ElasticSearch may declare a modified version of Apache Licence?

tisonkun · 2023-10-23T02:00:38Z

@bowenliang123 Thanks for reaching out to me.

I'm sure that elasticsearch-rest-client can be used under Apache License 2.0.

For elasticsearch-java you referred, Elastic released it under Apache License 2.0. From the dependencies it includes, the final JAR should be compatible with Apache License 2.0 to use and convey.

tisonkun · 2023-10-23T02:12:03Z

FYI Pulsar Elastic connector uses this API client - apache/pulsar#14805.

Although, you may check the dependencies doesn't include any ELv2 licensed code with a certain version.

pan3793 · 2023-10-23T02:27:22Z

Currently, the PR only considers ES 8, right? Actually, bunches of vendors are sticky on ES 7.10 (the last version released under AL2) or older versions, I think we should at least support ES 7 too.

bowenliang123 · 2023-10-23T02:32:02Z

Currently, the PR only considers ES 8, right? Actually, bunches of vendors are sticky on ES 7.10 (the last version released under AL2) or older versions, I think we should at least support ES 7 too.

Sure. This PR can be changed to base on ES 7 client and testing.

codecov-commenter · 2023-10-27T10:28:45Z

Codecov Report

Attention: Patch coverage is 0% with 110 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (afc21d3) to head (dbb5bb1).
Report is 132 commits behind head on master.

Files with missing lines	Patch %	Lines
...nts/handler/ElasticSearchLoggingEventHandler.scala	0.00%	60 Missing ⚠️
...in/scala/org/apache/kyuubi/config/KyuubiConf.scala	0.00%	25 Missing ⚠️
...che/kyuubi/events/ServerEventHandlerRegister.scala	0.00%	14 Missing ⚠️
...ndler/ServerElasticSearchLoggingEventHandler.scala	0.00%	7 Missing ⚠️
...rg/apache/kyuubi/events/EventHandlerRegister.scala	0.00%	3 Missing ⚠️
...ala/org/apache/kyuubi/events/EventLoggerType.scala	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #5488    +/-   ##
=======================================
  Coverage    0.00%   0.00%            
=======================================
  Files         677     679     +2     
  Lines       41902   42010   +108     
  Branches     5721    5730     +9     
=======================================
- Misses      41902   42010   +108

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bowenliang123 · 2023-10-27T13:28:56Z

Tested with ElasticSearch server 7.x and 8.x. Now use the elastic4s lib for ES scala client shipping with ElasticSearch Java Client. Both elastic4s 7.x or 8.x uses elasticsearch-rest-client 8.x. So elastic4s 8.x is preferred.
Have a check on the implementation and clarification in PR description. cc @pan3793 @wForget

wForget · 2023-10-31T01:57:09Z

...vents/src/main/scala/org/apache/kyuubi/events/handler/ElasticSearchLoggingEventHandler.scala

+  private val objectMapper = new ObjectMapper().registerModule(DefaultScalaModule)
+
+  override def apply(event: KyuubiEvent): Unit = {
+    val fields = {


This does not seem to be a good solution that serializes the event into a json string and then deserializes it again into a map. elastic4s provides the indexInto(indexId).doc(XXX) method to directly index json string.

Tried, but not working. Due to the nested fields that I have to serialize-deserialize-map again. Tried to implement a method for generating flat json, but not serializing properly.

but not serializing properly.

Are there any specific errors?

...vents/src/main/scala/org/apache/kyuubi/events/handler/ElasticSearchLoggingEventHandler.scala

pan3793 · 2024-08-09T07:25:16Z

...est/scala/org/apache/kyuubi/events/handler/ServerElasticSearchLoggingEventHandlerSuite.scala

+
+class ServerElasticSearchLoggingEventHandlerSuiteForEs7
+  extends ServerElasticSearchLoggingEventHandlerSuite {
+  override val imageTag = "7.17.14"


does it support ES 6?

a survey from another team in my company shows that we have bunches of customers still using ES 6

github-actions bot added module:server module:common kind:build module:events kind:documentation Documentation is a feature! labels Oct 20, 2023

bowenliang123 force-pushed the es-logger branch 2 times, most recently from 602cd77 to a7c9932 Compare October 25, 2023 07:51

bowenliang123 requested a review from pan3793 October 26, 2023 15:08

bowenliang123 force-pushed the es-logger branch from 09b5fb2 to 39adfff Compare October 27, 2023 13:17

github-actions bot added the kind:infra license, community building, project builds, asf infra related, etc. label Oct 28, 2023

bowenliang123 force-pushed the es-logger branch from ed65bd3 to 8ead671 Compare October 28, 2023 00:48

bowenliang123 marked this pull request as ready for review October 28, 2023 00:52

wForget reviewed Oct 31, 2023

View reviewed changes

...vents/src/main/scala/org/apache/kyuubi/events/handler/ElasticSearchLoggingEventHandler.scala Outdated Show resolved Hide resolved

wForget reviewed Oct 31, 2023

View reviewed changes

...vents/src/main/scala/org/apache/kyuubi/events/handler/ElasticSearchLoggingEventHandler.scala Show resolved Hide resolved

wForget reviewed Oct 31, 2023

View reviewed changes

...vents/src/main/scala/org/apache/kyuubi/events/handler/ElasticSearchLoggingEventHandler.scala Outdated Show resolved Hide resolved

wForget reviewed Oct 31, 2023

View reviewed changes

...vents/src/main/scala/org/apache/kyuubi/events/handler/ElasticSearchLoggingEventHandler.scala Outdated Show resolved Hide resolved

bowenliang123 mentioned this pull request Nov 3, 2023

[KYUUBI #5464] JDBC Engine supports MySQL #5588

Closed

3 tasks

bowenliang123 closed this Dec 5, 2023

bowenliang123 reopened this Dec 6, 2023

bowenliang123 force-pushed the es-logger branch from 2dad5a3 to 2c2ae23 Compare August 9, 2024 05:57

es event logger

5c01675

bowenliang123 force-pushed the es-logger branch from 2c2ae23 to 5c01675 Compare August 9, 2024 06:30

update elastic4s to 8.11.5

dbb5bb1

bowenliang123 force-pushed the es-logger branch from 956219c to dbb5bb1 Compare August 9, 2024 07:05

pan3793 reviewed Aug 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce ElasticSearch event logger for server events #5488

Introduce ElasticSearch event logger for server events #5488

bowenliang123 commented Oct 20, 2023 •

edited

Loading

cfmcgrady commented Oct 20, 2023 •

edited

Loading

bowenliang123 commented Oct 20, 2023

bowenliang123 commented Oct 21, 2023

tisonkun commented Oct 23, 2023 •

edited

Loading

tisonkun commented Oct 23, 2023

pan3793 commented Oct 23, 2023

bowenliang123 commented Oct 23, 2023

codecov-commenter commented Oct 27, 2023 •

edited

Loading

bowenliang123 commented Oct 27, 2023

wForget Oct 31, 2023

bowenliang123 Nov 3, 2023

wForget Nov 7, 2023

pan3793 Aug 9, 2024

Introduce ElasticSearch event logger for server events #5488

Are you sure you want to change the base?

Introduce ElasticSearch event logger for server events #5488

Conversation

bowenliang123 commented Oct 20, 2023 • edited Loading

Why are the changes needed?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

cfmcgrady commented Oct 20, 2023 • edited Loading

bowenliang123 commented Oct 20, 2023

bowenliang123 commented Oct 21, 2023

tisonkun commented Oct 23, 2023 • edited Loading

tisonkun commented Oct 23, 2023

pan3793 commented Oct 23, 2023

bowenliang123 commented Oct 23, 2023

codecov-commenter commented Oct 27, 2023 • edited Loading

Codecov Report

bowenliang123 commented Oct 27, 2023

wForget Oct 31, 2023

Choose a reason for hiding this comment

bowenliang123 Nov 3, 2023

Choose a reason for hiding this comment

wForget Nov 7, 2023

Choose a reason for hiding this comment

pan3793 Aug 9, 2024

Choose a reason for hiding this comment

bowenliang123 commented Oct 20, 2023 •

edited

Loading

cfmcgrady commented Oct 20, 2023 •

edited

Loading

tisonkun commented Oct 23, 2023 •

edited

Loading

codecov-commenter commented Oct 27, 2023 •

edited

Loading