[Feature] GraalVM Native Image support (Experimental). #11365

yswdqz · 2023-09-29T07:47:25Z

If this is non-trivial feature, paste the links/URLs to the design doc.
Update the documentation to include this new feature.
Tests(including UT, IT, E2E) are added to verify the new feature.
If it's UI related, attach the screenshots below.
If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes [Feature] GraalVM Build for OAP server #11279 .
Update the CHANGES log.

wu-sheng · 2023-10-19T13:59:16Z

...nt/src/main/java/org/apache/skywalking/library/elasticsearch/response/search/SearchHits.java

@@ -48,7 +48,7 @@ public Iterator<SearchHit> iterator() {
        return getHits().iterator();
    }

-    static final class TotalDeserializer extends JsonDeserializer<Integer> {
+    public static final class TotalDeserializer extends JsonDeserializer<Integer> {


Is this an unexpected change? Why public?

II want to simply reference this class in another class, but maybe I should use a different approach.

As an inner class, it should be accessible outside. Where do you use?

In SkyWalkingFeature, but I'm hesitating on whether to keep this class. I'll take a look after the CI for my repository finishes running.

OK. Let's continue the discussion after you have more conclusions.

wu-sheng · 2023-10-19T14:00:06Z

graal/graal-server-starter/pom.xml

+        <dependency>
+            <groupId>org.graalvm.sdk</groupId>
+            <artifactId>graal-sdk</artifactId>
+            <version>23.0.1</version>


Please manage the version from the bom.

wu-sheng · 2023-10-19T14:01:43Z

graal/graal-server-starter/src/main/java/org/apache/skywalking/graal/SkywalkingFeature.java

+import java.util.Arrays;
+import java.util.List;
+
+public class SkywalkingFeature implements Feature {


Please add comments to explain what this is about.

Suggested change

public class SkywalkingFeature implements Feature {

public class SkyWalkingFeature implements Feature {

skywalking should always be SkyWalking.

graal/graal-server-starter/src/main/resources/META-INF/native-image/main/jni-config.json

…alking into native-image-support

wu-sheng · 2023-10-20T11:41:07Z

docs/en/setup/backend/backend-graalvm.md

+
+## Current Limitations
+Native-image supports reflection and other dynamic features through some JSON-formatted configuration files. SkyWalking currently provides a set of configuration files for basic support. You can find them [here](https://github.com/apache/skywalking/tree/master/graal/graal-server-starter/src/main/resources/META-INF/native-image).


The path you are using here should use the relative path. The website compiling could translate it to the correct path binding in the version(commit ID).

wu-sheng · 2023-10-29T07:35:02Z

Notice, that the OSPP is reaching the deadline. SkyWalking community would not accept the PR until it is qualified.
Only two days left, and I can't be sure your PR can be merged at the last minute.

…alking into native-image-support

wu-sheng · 2023-10-29T14:25:35Z

...rg/apache/skywalking/oap/server/analyzer/agent/kafka/provider/KafkaFetcherModuleStartIT.java

+        Mockito.when(telemetryProvider.getService(MetricsCreator.class))
+                .thenReturn(new MetricsCreatorNoop());
+        TelemetryModule telemetryModule = Mockito.spy(TelemetryModule.class);
+        Whitebox.setInternalState(telemetryModule, "loadedProvider", telemetryProvider);
+        Mockito.when(moduleManager.find(TelemetryModule.NAME)).thenReturn(telemetryModule);
+
+        CoreModule coreModule = Mockito.spy(CoreModule.class);
+        Mockito.when(coreModuleProvider.getService(SourceReceiver.class)).thenReturn(new SourceReceiverImpl());


I believe a lot of mock codes are repeated in many UTs, right?

Could you move these mock codes into server-testing?

Meanwhile, due to all these tests are for Graal mostly, as you would not put the verification codes in these UTs, right?
You should reference UT-IT, and separate these UTs to run in the Graal package(such as GRAAL-PACKAGE-IT) only.
For now, you seem to make every UT-IT developer suffer extra time for nothing.

kezhenxu94

Some nits, otherwise look good to me

kezhenxu94 · 2023-10-30T11:03:38Z

.github/workflows/skywalking.yaml

@@ -22,7 +22,7 @@ on:
    - cron: "0 18 * * *" # TimeZone: UTC 0

 concurrency:
-  group: skywalking-${{ github.event.pull_request.number || github.ref }}
+  group: skywalking-graalvm-${{ github.event.pull_request.number || github.ref }}


I think you changed the wrong place, this file is skywalking.yaml, you should change the one in file .github/workflows/native-image.yaml

Suggested change

group: skywalking-graalvm-${{ github.event.pull_request.number || github.ref }}

group: skywalking-${{ github.event.pull_request.number || github.ref }}

kezhenxu94 · 2023-10-30T11:03:49Z

.github/workflows/native-image.yaml

+  push:
+
+concurrency:
+  group: skywalking-${{ github.event.pull_request.number || github.ref }}


Suggested change

group: skywalking-${{ github.event.pull_request.number || github.ref }}

group: skywalking-graalvm-${{ github.event.pull_request.number || github.ref }}

kezhenxu94 · 2023-10-30T11:04:22Z

.github/workflows/native-image.yaml

+    name: E2E test
+    needs: [build]
+    runs-on: ${{  'ubuntu-latest' }}


Suggested change

runs-on: ${{ 'ubuntu-latest' }}

runs-on: ubuntu-latest

kezhenxu94 · 2023-10-30T11:12:50Z

graal/docker/oap/Dockerfile

+
+FROM apache/skywalking-cli:$SKYWALKING_CLI_VERSION as cli
+
+FROM  --platform=linux/amd64 ubuntu:latest


So we only support amd64 architecture in our Docker images? We don't have to add arm support in this PR, just wondering whether there is any blocker to support arm64 arch

ARM architecture should can be supported, but for the sake of passing tests stably, I specified AMD (some Docker images do not support ARM architecture operating systems).

Let's open another PR to verify this.
Considering timing of OSPP, I could skip this.

yswdqz added 3 commits September 28, 2023 22:11

[Feature] Preliminary support native-image

e2ad8fb

[optimize] optimize config file size.

7982230

[change] update changes

9211ae3

yswdqz added core feature Core and important feature. Sometimes, break backwards compatibility. backend OAP backend related. labels Sep 29, 2023

yswdqz added this to the 9.7.0 milestone Sep 29, 2023

yswdqz requested review from wu-sheng and kezhenxu94 September 29, 2023 07:47

yswdqz added 3 commits October 19, 2023 18:44

test e2e

0ab6b79

[config] add e2e config

fb4cd26

[config] add e2e config file

46443b5

wu-sheng reviewed Oct 19, 2023

View reviewed changes

graal/graal-server-starter/src/main/resources/META-INF/native-image/main/jni-config.json Show resolved Hide resolved

yswdqz added 3 commits October 20, 2023 00:34

[e2e/doc] reduce e2e casees, add docs for native-image configs

b5fad86

[CI] fix native-image args file

617d4cb

[e2e] fix e2e

094e4ab

yswdqz marked this pull request as ready for review October 19, 2023 18:50

yswdqz and others added 5 commits October 20, 2023 02:50

Merge branch 'master' into native-image-support

44e47ce

[license] add license

1f8df26

Merge branch 'master' into native-image-support

3338207

[license] add license

da3d1c3

Merge branch 'native-image-support' of https://github.com/yswdqz/skyw…

ec6dc79

…alking into native-image-support

wu-sheng reviewed Oct 20, 2023

View reviewed changes

yswdqz and others added 2 commits October 28, 2023 19:23

[Native config] run ut/it by native-tracing-agent to generate configs

bfaf1dc

Merge branch 'master' into native-image-support

94bb2a5

yswdqz added 2 commits October 29, 2023 18:19

[e2e] test python agent e2e

d27fc18

Merge branch 'native-image-support' of https://github.com/yswdqz/skyw…

1ad623f

…alking into native-image-support

Update NativeConfigFilter.java

20447ef

wu-sheng reviewed Oct 29, 2023

View reviewed changes

yswdqz added 8 commits October 30, 2023 00:34

[Fix] fix error when generate oal classes

b99f565

[Ready] update e2e, optimize IT

6517a67

[fix] fix docs

615b3a6

[Fix] fix e2e/license

799f9bd

[doc] change doc link to relative path

a7036b5

[license/doc] add license, fix typo

2501791

[typo] fix typo

4df1860

[doc] add graal jdk install docs

2ab226f

kezhenxu94 previously approved these changes Oct 30, 2023

View reviewed changes

[typo] fix typo in ci config files

6e11010

yswdqz dismissed kezhenxu94’s stale review via 6e11010 October 30, 2023 12:18

kezhenxu94 approved these changes Oct 30, 2023

View reviewed changes

wu-sheng merged commit 92af797 into apache:master Oct 30, 2023
166 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] GraalVM Native Image support (Experimental). #11365

[Feature] GraalVM Native Image support (Experimental). #11365

yswdqz commented Sep 29, 2023 •

edited

Loading

wu-sheng Oct 19, 2023

yswdqz Oct 19, 2023

wu-sheng Oct 19, 2023

yswdqz Oct 19, 2023

wu-sheng Oct 19, 2023

wu-sheng Oct 19, 2023

wu-sheng Oct 19, 2023

wu-sheng Oct 20, 2023

wu-sheng commented Oct 29, 2023

wu-sheng Oct 29, 2023

wu-sheng Oct 29, 2023

kezhenxu94 left a comment

kezhenxu94 Oct 30, 2023

kezhenxu94 Oct 30, 2023

kezhenxu94 Oct 30, 2023

kezhenxu94 Oct 30, 2023

yswdqz Oct 30, 2023

wu-sheng Oct 30, 2023

	public class SkywalkingFeature implements Feature {
	public class SkyWalkingFeature implements Feature {


		## Current Limitations
		Native-image supports reflection and other dynamic features through some JSON-formatted configuration files. SkyWalking currently provides a set of configuration files for basic support. You can find them [here](https://github.com/apache/skywalking/tree/master/graal/graal-server-starter/src/main/resources/META-INF/native-image).

	group: skywalking-graalvm-${{ github.event.pull_request.number \|\| github.ref }}
	group: skywalking-${{ github.event.pull_request.number \|\| github.ref }}


		FROM apache/skywalking-cli:$SKYWALKING_CLI_VERSION as cli

		FROM --platform=linux/amd64 ubuntu:latest

[Feature] GraalVM Native Image support (Experimental). #11365

[Feature] GraalVM Native Image support (Experimental). #11365

Conversation

yswdqz commented Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wu-sheng commented Oct 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kezhenxu94 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yswdqz commented Sep 29, 2023 •

edited

Loading