add feature: java agent injector by dashanji · Pull Request #30 · apache/skywalking-swck

dashanji · 2021-06-16T13:58:52Z

PR_Proposal.pdf

hanahmily · 2021-06-17T00:15:38Z

@dashanji Glade to see this happen so soon. But the process is wrong.

Before this weekend, I will select one student from the candidates, and didn't promise you will be selected.

If you are selected, we will discuss the details of this feat. After that, you can move the current PR step.

I suggest you transfer this to a "draft" before the above steps are done.

wu-sheng · 2021-06-17T00:31:10Z

Before this weekend, I will select one student from the candidates, and didn't promise you will be selected.

@hanahmily I need to point out, from the foundation and PMC perspective, we don't care about whether this student is selected. Even it isn't, PR in public should be considered as a key point. We are going to accept this PR if it is good enough from a tech perspective.
Notice, we have accepted PMC's PR as it is open in public and good. Read this for the context, https://lists.apache.org/thread.html/r1b84b61347ea87792692fd5653ae23c982283cb4dc1c9c2943b51897%40%3Cdev.skywalking.apache.org%3E.
I have informed the Summer 2021 committee, this kind of thing could happen, please don't do a statement like this.

In the practice way, you could select the student you prefer, but in the open-source and the Apache way, this PR should not be asked to be drafted, which violate the transparency policy.

hanahmily · 2021-06-17T00:40:29Z

In the practice way, you could select the student you prefer, but in the open-source and the Apache way, this PR should not be asked to be drafted, which violate the transparency policy.

All right. I would like to withdraw my statement about the candidate's selection. But I need this to be drafted because this PR is not ready to be reviewed without any design document review process. That's from the tech perspective, not my personal preference.

wu-sheng · 2021-06-17T00:44:38Z

In the practice way, you could select the student you prefer, but in the open-source and the Apache way, this PR should not be asked to be drafted, which violate the transparency policy.

All right. I would like to withdraw my statement about the candidate's selection. But I need this to be drafted because this PR is not ready to be reviewed without any design document review process. That's from the tech perspective, not my personal preference.

It is fine from any tech requirement you need as usual.

wu-sheng · 2021-08-10T03:02:46Z

Do we have any progress here?

kezhenxu94 · 2021-08-10T03:43:29Z

A head up: this patch seems to assume that Java agent is the only one agent injectable, with apache/skywalking#7381 we will be able to inject Python agent too.

So in this PR the label for pod swck-agent-injected should be renamed to swck-java-agent-injected

hanahmily · 2021-08-10T03:59:56Z

Do we have any progress here?

The new design has passed, @dashanji has been writing codes.

hanahmily · 2021-08-10T04:02:26Z

A head up: this patch seems to assume that Java agent is the only one agent injectable, with apache/skywalking#7381 we will be able to inject Python agent too.

So in this PR the label for pod swck-agent-injected should be renamed to swck-java-agent-injected

Updated the title according to this situation

rainbend · 2021-08-10T07:19:12Z

Use feedback :)

wu-sheng · 2021-08-10T07:21:13Z

OK, after this is ready and merged, I expect the student could write up an English blog about how to use this.

* add a default configmap to save the default agent config * when create operator, the configmap will be created

* add some annotation override template * add sidecar fields' default value * add some func to validate the agent config annotation

* users can choose different injection strategies through annotations * sidecar coverage is enabled by default, and agent coverage is the user's choice * agent.config will be overwritten by configmap * failure to inject will return an error annotation

hanahmily

I just take a look at the implementation roughly. The main issues are to select container which need to inject and how to get the default configmap.

apis/operator/v1alpha1/oapserver_webhook.go

cmd/manager/manager.go

config/operator/webhook/kustomization.yaml

main.go

pkg/operator/injector/annotation.go

pkg/operator/injector/injector.go

* fix naming and version. * fix the bug about inject container. * use annotations.yaml to generate configmap and overlay the value of sidecar and agent * create a controller to watch the default configmap * add function to validate configmap's key and value

dashanji · 2021-08-22T15:38:36Z

The CI error may cause by controller-gen, the reason is the link controller-gen error. For this special case, can we delete
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./apis/..." in Makefile and CI?
I think it doesn't affect the use . Please see if it is feasible.

hanahmily · 2021-09-13T01:05:48Z

The CI error may cause by controller-gen, the reason is the link controller-gen error. For this special case, can we delete
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./apis/..." in Makefile and CI?
I think it doesn't affect the use . Please see if it is feasible.

It will insert the license header to generated APIs. If it causes these failures, we can remove it temporarily. Skywalking-eye will guarantee all headers exist.

hanahmily · 2021-09-13T01:06:42Z

@dashanji What's your plan for this PR, you didn't push anything recently?

dashanji · 2021-09-13T02:59:26Z

@dashanji What's your plan for this PR, you didn't push anything recently?

Next , I think I will add a CRD and a controller that used to watch the injected pod and then create CR. User can get the final agent configuration through the CR. I'm still writing code currently.

* remove most of the validate function and retain the functions that validate service_name and backend_service. * add overridable annotations to use optioncal plugins.

…k into feature-injector

hanahmily

Some nits.

pkg/operator/injector/injector.go

hanahmily

LGTM

hanahmily · 2021-09-15T05:48:04Z

@dashanji you need to document how to use this injector in follow-up PRs.

dashanji · 2021-09-15T15:01:55Z

@dashanji you need to document how to use this injector in follow-up PRs.

OK.

dashanji added 2 commits June 16, 2021 21:49

add feature-injector

8e04fce

add license

4a1e02d

wu-sheng requested a review from hanahmily June 16, 2021 14:55

wu-sheng added this to the 0.3.0 milestone Jun 16, 2021

fix naming errors

c752198

dashanji marked this pull request as draft June 17, 2021 00:36

hanahmily changed the title ~~add feature-injector~~ add feature: java agent injector Aug 10, 2021

dashanji added 4 commits August 14, 2021 19:02

add configmap

b52d3da

* add a default configmap to save the default agent config * when create operator, the configmap will be created

add default annotation and validate func

ee06d5d

* add some annotation override template * add sidecar fields' default value * add some func to validate the agent config annotation

sidecar inject

d93f76d

* users can choose different injection strategies through annotations * sidecar coverage is enabled by default, and agent coverage is the user's choice * agent.config will be overwritten by configmap * failure to inject will return an error annotation

add func test

4cf223b

dashanji marked this pull request as ready for review August 14, 2021 13:47

update label

0e01a8f

dashanji force-pushed the feature-injector branch from 81e2a67 to 0e01a8f Compare August 15, 2021 04:07

wu-sheng modified the milestones: 0.3.0, 0.4.0 Aug 15, 2021

fix lint error

09e60b0

hanahmily requested changes Aug 15, 2021

View reviewed changes

dashanji and others added 2 commits August 23, 2021 01:24

fix bug

31789d8

Merge branch 'master' into feature-injector

e0e1a50

kezhenxu94 mentioned this pull request Sep 9, 2021

[Feature] inject Python agent automatically in SWCK apache/skywalking#7682

Open

3 tasks

dashanji added 2 commits September 14, 2021 21:13

remove validate function and add optional plugins

93119ee

* remove most of the validate function and retain the functions that validate service_name and backend_service. * add overridable annotations to use optioncal plugins.

Merge branch 'feature-injector' of github.com:dashanji/skywalking-swc…

b67f4a7

…k into feature-injector

hanahmily reviewed Sep 14, 2021

View reviewed changes

pkg/operator/injector/injector.go Outdated Show resolved Hide resolved

pkg/operator/injector/injector.go Outdated Show resolved Hide resolved

pkg/operator/injector/injector.go Outdated Show resolved Hide resolved

add optional-reporter

8da6320

hanahmily approved these changes Sep 15, 2021

View reviewed changes

hanahmily merged commit a7ea13c into apache:master Sep 15, 2021

kezhenxu94 mentioned this pull request Sep 16, 2021

[OPERATOR]Agent injection webhook apache/skywalking#6029

Closed

4 tasks

Comments

Conversation

dashanji commented Jun 16, 2021

Uh oh!

hanahmily commented Jun 17, 2021

Uh oh!

wu-sheng commented Jun 17, 2021

Uh oh!

hanahmily commented Jun 17, 2021

Uh oh!

wu-sheng commented Jun 17, 2021

Uh oh!

wu-sheng commented Aug 10, 2021

Uh oh!

kezhenxu94 commented Aug 10, 2021

Uh oh!

hanahmily commented Aug 10, 2021

Uh oh!

hanahmily commented Aug 10, 2021

Uh oh!

rainbend commented Aug 10, 2021

Uh oh!

wu-sheng commented Aug 10, 2021

Uh oh!

hanahmily left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dashanji commented Aug 22, 2021

Uh oh!

hanahmily commented Sep 13, 2021

Uh oh!

hanahmily commented Sep 13, 2021

Uh oh!

dashanji commented Sep 13, 2021

Uh oh!

hanahmily left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hanahmily left a comment

Choose a reason for hiding this comment

Uh oh!

hanahmily commented Sep 15, 2021

Uh oh!

dashanji commented Sep 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hanahmily left a comment •

edited

Loading