feat(add data collect proto in go agent) #231

cqhasy · 2025-09-06T14:10:13Z

No description provided.

mrproliu · 2025-09-06T14:15:07Z

tools/go-agent/instrument/agentcore/instrument.go

 		pkgUpdates[newKey] = newVal
 	}
+
+	// 添加protocols导入路径的处理


Please delete the Chinese comment.

mrproliu · 2025-09-06T14:15:40Z

tools/go-agent/instrument/reporter/instrument.go

 			val := strings.ReplaceAll(filepath.Join(agentcore.EnhanceBasePackage, p), `\`, `/`)
 			pkgUpdates[key] = val
 		}
+		// 添加protocols导入路径的处理


Please delete Chinese comment.

mrproliu · 2025-09-06T14:18:32Z

tools/protocols/pull-proto.sh

+#!/bin/bash
+
+# Configuration parameters
+REPO_URL="https://github.com/apache/skywalking-data-collect-protocol.git"


Why you don't use the submodule to import the data collect protocol dependency repo? It easy to control the version, rather than using git command.

mrproliu · 2025-09-06T14:20:04Z

Makefile

+##@ General
+
+.PHONY: generate-proto
+generate-proto: ##generate data collect proto


This command should be invoke before the plugin or E2E test? Please make sure the all the test can be executed successfully

mrproliu · 2025-09-07T02:38:18Z

tools/protocols/pull-proto.sh

+# -----------------------------
+if [ ! -d "$SUBMODULE_PATH" ]; then
+    echo "Initializing submodule..."
+    git submodule update --init --recursive "$SUBMODULE_PATH"


Please remove this update, the git submodule already lock the version.

mrproliu · 2025-09-07T02:39:15Z

tools/protocols/pull-proto.sh

+cd "$SUBMODULE_PATH"
+git fetch --all
+git checkout "$SUBMODULE_COMMIT"
+cd - > /dev/null


You don't need to checkout, just generate the gRPC code is enough.

…x in windows-plugin-test)

…ealth check)

…cenarios)

wu-sheng · 2025-09-11T07:33:45Z

I think you could slightly change the CI control file to run this in your fork repo. It will cost time for you to do this through a PR.

mrproliu

Please also update the CHANGES.md to declare that the agent has replaced the goapi and generates it by itself.

mrproliu · 2025-09-11T08:16:07Z

.github/workflows/e2e.yaml

          submodules: true
+      - name: Install unzip
+        run: sudo apt-get update && sudo apt-get install -y unzip
+      - name: Check Go version


The setup Go already define the go version, why there still needs to check again?

mrproliu · 2025-09-11T08:18:15Z

.github/workflows/e2e.yaml

        with:
          submodules: true
+      - name: Install unzip
+        run: sudo apt-get update && sudo apt-get install -y unzip


Is the unzip not define in the original runner? If exist, please remove this install step.

unzip is not install before

mrproliu · 2025-09-11T08:19:58Z

.github/workflows/e2e.yaml

            path: test/e2e/case/kafka/e2e.yaml
    steps:
-      - uses: actions/checkout@v2
+      - name: Set up Go


In the Job, the agent have already built, so don't needs to generate the protocol again.

mrproliu · 2025-09-11T08:22:15Z

.github/workflows/plugin-tests.yaml

      - uses: actions/checkout@v2
        with:
          submodules: true
+      - name: Set up Go 1.19


The agent binary is already built. Please remove unnecessary generated code.

mrproliu · 2025-09-11T08:23:02Z

.github/workflows/plugin-tests.yaml

        uses: actions/setup-go@v2
        with:
          go-version: 1.19
+      - name: Install unzip


Same here, the agent already built.

.gitignore

mrproliu · 2025-09-11T08:40:19Z

tools/protocols/pull-proto.sh

+export GOPROXY=https://goproxy.cn,direct
+PROTOC_VERSION=3.14.0
+export PATH="/home/runner/go/bin:$PATH"
+export GOPATH="${GOPATH:-$HOME/go}"


Why do you need to declare GOPROXY, PATH, and GOPATH? It should use the user system configuration.
User system doesn't have /home/runner, it should only work for the GitHub Runner.

mrproliu · 2025-09-11T08:42:05Z

tools/protocols/pull-proto.sh

+INCLUDE_DIR="$TEMPDIR"/include
+
+SUBMODULE_PATH="skywalking-data-collect-protocol"
+OUTPUT_BASE_DIR="../../protocols"


You should use the absolute directory. If the user executes this command in another directory, the output directory is not expected.

mrproliu · 2025-09-11T08:42:50Z

tools/protocols/pull-proto.sh

+go version
+if ! command -v protoc-gen-go &>/dev/null; then
+    echo "Installing protoc-gen-go v1.26..."
+    GO111MODULE=on GOPROXY=https://goproxy.cn,direct GOSUMDB=sum.golang.google.cn go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.26.0


Please remove these env, just do go install.

mrproliu · 2025-09-11T08:43:07Z

tools/protocols/pull-proto.sh

+
+if ! command -v protoc-gen-go-grpc &>/dev/null; then
+    echo "Installing protoc-gen-go-grpc v1.1..."
+    GO111MODULE=on GOPROXY=https://goproxy.cn,direct GOSUMDB=sum.golang.google.cn go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.1.0


Please remove these env, just do go install.

…nges in change.md)

mrproliu · 2025-09-11T12:15:12Z

tools/protocols/generate.sh

+# -----------------------------
+# Configuration
+# -----------------------------
+#export GOPROXY=https://goproxy.cn,direct


Please delete it rather than comment it.

mrproliu · 2025-09-11T12:17:26Z

CHANGES.md


 #### Documentation
-
+* Replace external `goapi` dependency with in-repo generated protocols.


It should be declared in the feature selection.

feat(add data collect proto in go agent)

e4c9e6b

mrproliu reviewed Sep 6, 2025

View reviewed changes

cqhasy added 3 commits September 6, 2025 22:46

Lock skywalking-data-collect-protocol to specific version

a6952b6

fix(change pull-proto.sh to adapt git submodule)

151cadd

feat(add generate-proto in workflows)

4e1e9aa

mrproliu reviewed Sep 7, 2025

View reviewed changes

cqhasy added 24 commits September 7, 2025 10:46

fix (remove git submodule update in pull-proto.sh)

595a35b

feat(add license in file and check if install protoc in make proto)

d317ecf

fix(solve tar protoc no power in CI)

14eade2

fix(lower protoc-gen-go-grpc version in CI,fix expoter for protoc)

1645ab7

fix(keep pull-proto.sh protoc path same with user download)

95219ac

feat(ignore tools/protocols/temp)

f111ad4

fix(add goPROXY,add unzip process info)

2ca045c

fix(lower /protoc-gen-go-grpc /protoc-gen-go version, install dos2uni…

f9c18e5

…x in windows-plugin-test)

fix(back golang.org/x/net, add go version in pull-proto)

2f1a379

fix(ignore protocols license,lower go mod to adapt go 1.19)

4ccf44f

fix(ignore proto license,add make proto for test,extend wait time )

2efd447

fix(change protocol import path to access lint,add some log info in h…

2a0ae41

…ealth check)

fix(adapt to lint,add GOPONXY in kafka E2E,extend sleep time in wsl-s…

301beb8

…cenarios)

fix(force e2e test use 1.19,add generate proto in build)

3e11a49

fix(change infra-e2e version,devide zip install in windows and linux)

111f712

fix(change infra-e2e)

8f25978

fix(advance install docker)

8fe61bf

fix(add env /bin/bash)

dac5c6c

fix(add PATH user/bin)

4942066

fix(add go-version)

bc6a8a5

fix(force go-version)

062c4f8

roll back

d6486ab

use 1.24 to adapt apache/skywalking-infra-e2e

0ff9580

fix(fix e2e.yaml)

64f08f0

cqhasy added 4 commits September 10, 2025 21:47

role back windows host

d190906

select could use windows host

61a7727

add more log info,extend curl time

ab84e58

roll workflows on

abcf0d2

cqhasy reopened this Sep 11, 2025

cqhasy closed this Sep 11, 2025

cqhasy added 3 commits September 11, 2025 10:37

test

3cfa652

add resolv info

7a3c2aa

roll workflows on

6f8fc85

cqhasy reopened this Sep 11, 2025

cqhasy closed this Sep 11, 2025

dont use resolv.conf

8deff0d

fix(add full log when get ip in windows)

8983f85

cqhasy reopened this Sep 11, 2025

roll back workflows on

a849a09

mrproliu reviewed Sep 11, 2025

View reviewed changes

cqhasy closed this Sep 11, 2025

cqhasy added 3 commits September 11, 2025 18:48

delete generate proto in e2e

35035fa

fix(delete generate proto in CI)

4c9ec42

fix(rename pull-proto.sh,delete no use shell in pull-proto.sh,add cha…

941a29d

…nges in change.md)

cqhasy reopened this Sep 11, 2025

mrproliu reviewed Sep 11, 2025

View reviewed changes

cqhasy added 3 commits September 11, 2025 20:23

fix(mov changes from doc to feature,delete some common)

328a144

fix(delete make generate proto in publish docker)

d697977

feat(add delete temp in generate.sh)

5595008

mrproliu added the enhancement New feature or request label Sep 11, 2025

mrproliu added this to the 0.7.0 milestone Sep 11, 2025

mrproliu approved these changes Sep 11, 2025

View reviewed changes

mrproliu merged commit 0d8a5be into apache:main Sep 11, 2025
43 checks passed


		#### Documentation

		* Replace external `goapi` dependency with in-repo generated protocols.

feat(add data collect proto in go agent) #231

feat(add data collect proto in go agent) #231

Uh oh!

Conversation

cqhasy commented Sep 6, 2025

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!

wu-sheng commented Sep 11, 2025

Uh oh!

mrproliu left a comment

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants