Add Java agent injector feature by dashanji · Pull Request #14 · apache/skywalking-showcase

dashanji · 2021-12-24T16:41:53Z

The PR is related to apache/skywalking#8195, and do two things:

Add the java agent injector to replace the original method (injected when building the image) so that we can inject the java agent more natively.
Update the doc.

Signed-off-by: dashanji <952129620@qq.com>

kezhenxu94 · 2021-12-24T17:11:14Z

Do we have a more recommended way to use swck, like via CLI?

wu-sheng · 2021-12-24T23:47:04Z

Do we have a more recommended way to use swck, like via CLI?

Could you be more specific? CLI(current) is not responsible for deployment AFAIK

kezhenxu94 · 2021-12-25T01:43:06Z

Do we have a more recommended way to use swck, like via CLI?

Could you be more specific? CLI(current) is not responsible for deployment AFAIK

I remember there is swctl install to install OAP and UI, it's better we can install or generate agent manifest there too, the current usage in this pr looks kindof complex. For example, user provides their original yaml of the application, and pass them into our command, then our command populates the necessary part like annotations, etc., swctl inject java -f deployments/java-app.yaml | kubectl apply -f -

This is not a blocker of this PR but I hope we can simplify further the usage of our agent injector.

wu-sheng · 2021-12-25T01:48:15Z

@hanahmily PTAL.

kezhenxu94 · 2021-12-25T01:53:53Z

 	@kubectl delete --ignore-not-found -f https://raw.githubusercontent.com/kubernetes/kube-state-metrics/v2.2.4/examples/standard/service.yaml
 	@kubectl delete --ignore-not-found -f https://raw.githubusercontent.com/kubernetes/kube-state-metrics/v2.2.4/examples/standard/deployment.yaml
+
+# @feature: java agent injector


Can you use this comment pattern: # @feature: <feature-name>; <what and why we need this step in this feature>. The same for other comments added in this PR.

OK, updated it.

Signed-off-by: dashanji <952129620@qq.com>

dashanji · 2021-12-25T07:57:57Z

Do we have a more recommended way to use swck, like via CLI?

Could you be more specific? CLI(current) is not responsible for deployment AFAIK

I remember there is swctl install to install OAP and UI, it's better we can install or generate agent manifest there too, the current usage in this pr looks kindof complex. For example, user provides their original yaml of the application, and pass them into our command, then our command populates the necessary part like annotations, etc., swctl inject java -f deployments/java-app.yaml | kubectl apply -f -

This is not a blocker of this PR but I hope we can simplify further the usage of our agent injector.

Maybe we can add it in the future.

Signed-off-by: dashanji <952129620@qq.com>

wu-sheng · 2021-12-25T15:16:30Z

@kezhenxu94 We seems don't have any verification in CI process. Should we add some?

wu-sheng · 2021-12-26T02:19:37Z

@kezhenxu94 We seems don't have any verification in CI process. Should we add some?

I think we don't need functionality testing here, but we should make sure k8s and docker-compose deploying successfully, with at least one metric verified.

wu-sheng · 2021-12-27T03:10:12Z

@dashanji I think we need a new doc at here, https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/containerization/#kubernetes

Besides initContainers of a sidecar, SWCK provides another way to deploy agent.

hanahmily · 2021-12-27T03:38:37Z

I remember there is swctl install to install OAP and UI, it's better we can install or generate agent manifest there too, the current usage in this pr looks kindof complex. For example, user provides their original yaml of the application, and pass them into our command, then our command populates the necessary part like annotations, etc., swctl inject java -f deployments/java-app.yaml | kubectl apply -f -

The recommended path to onboard java agent injection is to install all SW backends through the SWCK operator. The operator provided several CRDs to manage OAP, UI, and other critical components. This model should be our canonical way.

We could provide a manual-style injection to swctl to help users don't want to inject init container by webhooks automatically. But from experiences of istio, vault, and other projects who have similar injection workflows, the manual way is not widely used, because this style brings a steep learning curve to users since the operator reduced it.

dashanji · 2021-12-27T04:39:32Z

@dashanji I think we need a new doc at here, https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/containerization/#kubernetes

Besides initContainers of a sidecar, SWCK provides another way to deploy agent.

Yep, I'm writing the blog about how to use SWCK, when it is finished, I will change the doc related to it.

dashanji · 2021-12-27T09:25:48Z

I add an e2e test to ensure that all components can run normally after we build the image.

wu-sheng · 2021-12-27T09:27:35Z

+        resource: service/rocket-bot 
+        port: 8080
+  timeout: 20m
+


Could you add verification at least checking the service list?

kezhenxu94 · 2021-12-27T09:33:47Z

+  e2e-tests:
+    name: e2e tests(kubenretes platform)
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.17
+        id: go
+      - name: Check out code into the Go module directory
+        uses: actions/checkout@v2
+      - name: Run E2E Test
+        uses: apache/skywalking-infra-e2e@996ed8902e941e2883fcf0ac5b3090364942d205
+        with:
+          e2e-file: test/e2e/e2e.yaml


Put this job to a separate file, this file is only for publishing images and it's only executed on main branch, not pull requests

ok, get it.

kezhenxu94 · 2021-12-27T09:34:59Z

+          command: ["java"]
+          # @feature: java-agent-injector; we need to add $(AGENT_OPTS) to enable the injected configuration 
+          args: ${JAVA_AGENT}


Why do we need this? Does swck requires users to modify these?

Yep, the injector needs to add the AGENT_OPTS to JVM argument like ["$(AGENT_OPTS)","-jar","/app.jar"].

Yep, the injector needs to add the AGENT_OPTS to JVM argument like ["$(AGENT_OPTS)","-jar","/app.jar"].

Sorry I didn't follow the related pull request in swck repo, but why don't we use environment name JAVA_TOOL_OPTIONS so that JVM can automatically pick this up and users don't need to modify their command?

Make sense to me, we can improve it in the next version.

Make sense to me, we can improve it in the next version.

Nice 🎉

Now the injector is using the way to inject. I don't know much about java, can it be used as an environment variable? if possible, then we don’t need to add it to the startup parameters.

Now the injector is using the way to inject. I don't know much about java, can it be used as an environment variable? if possible, then we don’t need to add it to the startup parameters.

@dashanji yes, take a look at this https://github.com/apache/skywalking-java/blob/96ea77cf8f1fa922ed4f1e8c4bf9796e33790bdf/Dockerfile#L31 , this env var is automatically picked up by JVM and it's the same effect as java $JAVA_TOOL_OPTIONS /app.jar for all Java applications in that container

I learn from it, thanks a lot.

dashanji · 2021-12-27T12:25:32Z

Updated it.

kezhenxu94 · 2021-12-27T12:26:19Z

+  push:
+    branches:
+      - main


I think we only need this in pull request, WDYT? @wu-sheng

kezhenxu94 · 2021-12-27T12:29:07Z

+	@curl -Ls https://dlcdn.apache.org/skywalking/swck/${SWCK_OPERATOR_VERSION}/skywalking-swck-${SWCK_OPERATOR_VERSION}-bin.tgz | tar -zxf - -O ./config/operator-bundle.yaml | kubectl apply -f -
+	@kubectl label namespace --overwrite $(NAMESPACE) swck-injection=enabled
+	# @feature: java-agent-injector; we can update the agent's backend address in a single-node cluster firstly so that we don't need to add the same backend env for every java agent
+	@kubectl get configmap skywalking-swck-java-agent-configmap -n skywalking-swck-system -oyaml | sed 's/127.0.0.1/default-oap.default/' | kubectl apply -f -


Suggested change

@kubectl get configmap skywalking-swck-java-agent-configmap -n skywalking-swck-system -oyaml | sed 's/127.0.0.1/default-oap.default/' | kubectl apply -f -

@kubectl get configmap skywalking-swck-java-agent-configmap -n skywalking-swck-system -oyaml | sed "s/127.0.0.1/default-oap.$(NAMESPACE)/" | kubectl apply -f -

ok, get it. By the way, the CI may be wrong as we don't push the image.

Oh yes. Then in CI you need to build the images first (make docker -j 5), and then load them into the KinD using infra-e2e.

OK,thanks a lot.

hanahmily · 2022-01-04T02:04:49Z

@dashanji any update?

wu-sheng · 2022-01-06T01:50:53Z

Any progress to update?

dashanji · 2022-01-06T03:34:13Z

Still testing, I have failed many times on the Github action, I am still looking for the reason.

Signed-off-by: dashanji <952129620@qq.com>

dashanji · 2022-01-18T06:40:53Z

Could you please re-run the CI job? I feel very strange that the CI running results are different from my personal repository and the test results in my personal repository are good.

wu-sheng · 2022-01-18T06:50:57Z

Running.

kezhenxu94 · 2022-01-18T09:05:49Z

+  push:
+    branches:
+      - main


Suggested change

push:

branches:

- main

kezhenxu94 · 2022-01-18T09:07:44Z

+env:
+  IMAGE_HUB: ghcr.io/apache/skywalking-showcase
+
+jobs:
+  e2e-tests:
+    name: e2e tests(kubenretes platform)
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.17
+        id: go
+      - name: Check out code into the Go module directory
+        uses: actions/checkout@v2
+      - name: set image_tag to env
+        shell: bash
+        run: |
+          echo "IMAGE_TAG=$(git rev-parse --short HEAD)" >> $GITHUB_ENV


Suggested change

env:

IMAGE_HUB: ghcr.io/apache/skywalking-showcase

jobs:

e2e-tests:

name: e2e tests(kubenretes platform)

runs-on: ubuntu-latest

steps:

- name: Install Go

uses: actions/setup-go@v2

with:

go-version: 1.17

id: go

- name: Check out code into the Go module directory

uses: actions/checkout@v2

- name: set image_tag to env

shell: bash

run: |

echo "IMAGE_TAG=$(git rev-parse --short HEAD)" >> $GITHUB_ENV

env:

IMAGE_HUB: ghcr.io/apache/skywalking-showcase

IMAGE_TAG: ${{ github.sha }}

jobs:

e2e-tests:

name: e2e tests(kubenretes platform)

runs-on: ubuntu-latest

steps:

- name: Install Go

uses: actions/setup-go@v2

with:

go-version: 1.17

- name: Check out source code

uses: actions/checkout@v2

kezhenxu94 · 2022-01-18T09:09:35Z

+          imagePullPolicy: IfNotPresent
          ports:
            - containerPort: 80
+          # @feature: als; set env to configure java agent, If we use injector, we don't need to set env, only to configure java gent through annotation


Suggested change

# @feature: als; set env to configure java agent, If we use injector, we don't need to set env, only to configure java gent through annotation

# @feature: agent; set env to configure java agent, If we use injector, we don't need to set env, only to configure java gent through annotation

kezhenxu94 · 2022-01-18T09:09:58Z

+          imagePullPolicy: IfNotPresent
          ports:
            - containerPort: 80
+          # @feature: als; set env to configure java agent, If we use injector, we don't need to set env, only to configure java gent through annotation


Suggested change

# @feature: als; set env to configure java agent, If we use injector, we don't need to set env, only to configure java gent through annotation

# @feature: agent; set env to configure java agent, If we use injector, we don't need to set env, only to configure java gent through annotation

kezhenxu94 · 2022-01-18T09:12:45Z

 .PHONY: deploy.feature-als
 deploy.feature-als: prerequisites
-	$(MAKE) deploy FEATURE_FLAGS=agent TAG=$(TAG)-agentless NAMESPACE=$(NAMESPACE)-agentless AGENTLESS=true SHOW_TIPS=false
+	$(MAKE) deploy FEATURE_FLAGS=agent TAG=$(TAG)-agentless NAMESPACE=$(NAMESPACE)-agentless AGENTLESS=true JAVA_AGENT=$(JAVA_AGENT_OPTS_AGENTLESS) SHOW_TIPS=false


Suggested change

$(MAKE) deploy FEATURE_FLAGS=agent TAG=$(TAG)-agentless NAMESPACE=$(NAMESPACE)-agentless AGENTLESS=true JAVA_AGENT=$(JAVA_AGENT_OPTS_AGENTLESS) SHOW_TIPS=false

$(MAKE) deploy FEATURE_FLAGS=agent TAG=$(TAG)-agentless NAMESPACE=$(NAMESPACE)-agentless AGENTLESS=true SHOW_TIPS=false

kezhenxu94 · 2022-01-18T09:20:04Z

+# @feature: java-agent-injector; uninstall the swck operator and cert-manager
+.PHONY: undeploy.feature-java-agent-injector
+undeploy.feature-java-agent-injector:
+	@curl -Ls https://dlcdn.apache.org/skywalking/swck/${SWCK_OPERATOR_VERSION}/skywalking-swck-${SWCK_OPERATOR_VERSION}-bin.tgz | tar -zxf - -O ./config/operator-bundle.yaml | kubectl delete -f -


Suggested change

@curl -Ls https://dlcdn.apache.org/skywalking/swck/${SWCK_OPERATOR_VERSION}/skywalking-swck-${SWCK_OPERATOR_VERSION}-bin.tgz | tar -zxf - -O ./config/operator-bundle.yaml | kubectl delete -f -

@curl -Ls https://archive.apache.org/dist/skywalking/swck/${SWCK_OPERATOR_VERSION}/skywalking-swck-${SWCK_OPERATOR_VERSION}-bin.tgz | tar -zxf - -O ./config/operator-bundle.yaml | kubectl delete -f -

kezhenxu94 · 2022-01-18T09:20:20Z

+
+

Suggested change

kezhenxu94 · 2022-01-18T09:21:33Z

 	docker build . -t $(HUB)/load-gen:$(TAG)
+	docker tag $(HUB)/load-gen:$(TAG) $(HUB)/load-gen:$(TAG)-agentless


Suggested change

docker build . -t $(HUB)/load-gen:$(TAG)

docker tag $(HUB)/load-gen:$(TAG) $(HUB)/load-gen:$(TAG)-agentless

docker build . -t $(HUB)/load-gen:$(TAG) -t $(HUB)/load-gen:$(TAG)-agentless

kezhenxu94 · 2022-01-18T09:24:57Z

+    name: test-selfsigned
+EOF
+
+timeout $TIMEOUT bash -c -- "\


timeout command is not available in my MacOS, please try to use other means that are generally available, I don't want (myself or others) to struggle for installing this just for testing.

OK, I will change it.

kezhenxu94 · 2022-01-18T09:26:48Z

+      tar xz && cd skywalking-cli-${SWCTL_VERSION} && make ${OS} && mv bin/swctl-*-${OS}-amd64 ${INSTALL_DIR}/swctl 
+      if [ $? -ne 0 ]; then
+        echo "install swctl error, please check"
+        $prepare_ok=false


I don't think this is a valid assignment, please also check other places

Suggested change

$prepare_ok=false

prepare_ok=false

Yep, the swck is the same, I will correct it, thanks a lot.

Signed-off-by: dashanji <952129620@qq.com>

dashanji · 2022-01-18T14:47:03Z

Updated it, thanks for the correction. @kezhenxu94

Signed-off-by: dashanji <952129620@qq.com>

kezhenxu94 · 2022-01-19T02:04:48Z

Hi @dashanji , I updated something and the codes LGTM, can you please check why the tests failed? Once the tests passed we can merge this

dashanji · 2022-01-19T04:20:56Z

Hi @dashanji , I updated something and the codes LGTM, can you please check why the tests failed? Once the tests passed we can merge this

OK

Add a readinessProbe to make sure the UI has something listening on port 8080.

dashanji · 2022-01-19T05:09:30Z

I added a readinessProbe, hope this can make the CI successful.

wu-sheng · 2022-01-19T07:56:15Z

@kezhenxu94 I run this CI twice, both passed but require 30+ minutes.

kezhenxu94

@kezhenxu94 I run this CI twice, both passed but require 30+ minutes.

It's reasonable to me it takes so long as long as the tests passed

kezhenxu94 · 2022-01-19T07:59:38Z

Thanks @dashanji for the work!!

add java agent injector

a919a02

Signed-off-by: dashanji <952129620@qq.com>

kezhenxu94 reviewed Dec 25, 2021

View reviewed changes

update the comment

51ec7ef

Signed-off-by: dashanji <952129620@qq.com>

wu-sheng reviewed Dec 25, 2021

View reviewed changes

Comment thread deploy/platform/kubernetes/features.mk Outdated

wu-sheng requested a review from hanahmily December 25, 2021 12:42

seperate the version

25ce1f0

Signed-off-by: dashanji <952129620@qq.com>

wu-sheng added the enhancement New feature or request label Dec 25, 2021

wu-sheng reviewed Dec 27, 2021

View reviewed changes

Comment thread deploy/platform/kubernetes/feature-agent/resources.yaml

add e2e test

f57a2d1

wu-sheng reviewed Dec 27, 2021

View reviewed changes

kezhenxu94 reviewed Dec 27, 2021

View reviewed changes

dashanji mentioned this pull request Jan 10, 2022

use JAVA_TOOL_OPTIONS to replace AGENT_OPTS apache/skywalking-swck#58

Merged

fix issue

2cd79f6

Signed-off-by: dashanji <952129620@qq.com>

dashanji force-pushed the add-java-agent-injector branch from 09bc229 to 2cd79f6 Compare January 17, 2022 17:02

wu-sheng requested a review from kezhenxu94 January 18, 2022 07:32

kezhenxu94 requested changes Jan 18, 2022

View reviewed changes

fix issue

75dd754

Signed-off-by: dashanji <952129620@qq.com>

wu-sheng requested a review from kezhenxu94 January 18, 2022 15:21

dashanji and others added 3 commits January 19, 2022 00:06

fix sha error

cfc373c

Signed-off-by: dashanji <952129620@qq.com>

Update go.yml

4209c22

Update go.yml

0d8fed0

Update resources.yaml

00efea0

Add a readinessProbe to make sure the UI has something listening on port 8080.

kezhenxu94 approved these changes Jan 19, 2022

View reviewed changes

kezhenxu94 changed the title ~~add java agent injector~~ Add Java agent injector feature Jan 19, 2022

kezhenxu94 merged commit 95e9576 into apache:main Jan 19, 2022

dashanji deleted the add-java-agent-injector branch January 19, 2022 08:06

	@kubectl get configmap skywalking-swck-java-agent-configmap -n skywalking-swck-system -oyaml \| sed 's/127.0.0.1/default-oap.default/' \| kubectl apply -f -
	@kubectl get configmap skywalking-swck-java-agent-configmap -n skywalking-swck-system -oyaml \| sed "s/127.0.0.1/default-oap.$(NAMESPACE)/" \| kubectl apply -f -

	# @feature: als; set env to configure java agent, If we use injector, we don't need to set env, only to configure java gent through annotation
	# @feature: agent; set env to configure java agent, If we use injector, we don't need to set env, only to configure java gent through annotation

	$(MAKE) deploy FEATURE_FLAGS=agent TAG=$(TAG)-agentless NAMESPACE=$(NAMESPACE)-agentless AGENTLESS=true JAVA_AGENT=$(JAVA_AGENT_OPTS_AGENTLESS) SHOW_TIPS=false
	$(MAKE) deploy FEATURE_FLAGS=agent TAG=$(TAG)-agentless NAMESPACE=$(NAMESPACE)-agentless AGENTLESS=true SHOW_TIPS=false

	@curl -Ls https://dlcdn.apache.org/skywalking/swck/${SWCK_OPERATOR_VERSION}/skywalking-swck-${SWCK_OPERATOR_VERSION}-bin.tgz \| tar -zxf - -O ./config/operator-bundle.yaml \| kubectl delete -f -
	@curl -Ls https://archive.apache.org/dist/skywalking/swck/${SWCK_OPERATOR_VERSION}/skywalking-swck-${SWCK_OPERATOR_VERSION}-bin.tgz \| tar -zxf - -O ./config/operator-bundle.yaml \| kubectl delete -f -

		docker build . -t $(HUB)/load-gen:$(TAG)
		docker tag $(HUB)/load-gen:$(TAG) $(HUB)/load-gen:$(TAG)-agentless


		No newline at end of file

Uh oh!

Conversation

dashanji commented Dec 24, 2021

Uh oh!

kezhenxu94 commented Dec 24, 2021

Uh oh!

wu-sheng commented Dec 24, 2021

Uh oh!

kezhenxu94 commented Dec 25, 2021

Uh oh!

wu-sheng commented Dec 25, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dashanji commented Dec 25, 2021

Uh oh!

Uh oh!

wu-sheng commented Dec 25, 2021

Uh oh!

wu-sheng commented Dec 26, 2021

Uh oh!

wu-sheng commented Dec 27, 2021

Uh oh!

Uh oh!

hanahmily commented Dec 27, 2021

Uh oh!

dashanji commented Dec 27, 2021

Uh oh!

dashanji commented Dec 27, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dashanji commented Dec 27, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanahmily commented Jan 4, 2022

Uh oh!

wu-sheng commented Jan 6, 2022

Uh oh!

dashanji commented Jan 6, 2022

Uh oh!

dashanji commented Jan 18, 2022

Uh oh!

wu-sheng commented Jan 18, 2022

Uh oh!