Skip to content
This repository has been archived by the owner on Jun 14, 2023. It is now read-only.

supported dubbo-go v3.0.2 #49

Merged
merged 19 commits into from
Jul 18, 2022
Merged

supported dubbo-go v3.0.2 #49

merged 19 commits into from
Jul 18, 2022

Conversation

dgqypl
Copy link
Contributor

@dgqypl dgqypl commented Jul 8, 2022

No description provided.

@arugal arugal added this to the 1.5.0 milestone Jul 8, 2022
@arugal arugal added the plugin label Jul 8, 2022
@arugal
Copy link
Member

arugal commented Jul 8, 2022

Hi @dgqypl , please help to replace the link with https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh :)

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 8, 2022

Hi @dgqypl , please help to replace the link with https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh :)

curl -L https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.45.2 -> curl -L https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.45.2
right?

Comment on lines +136 to +139
if attachment, ok := invocation.GetAttachment(key); ok {
return attachment, nil
}
return "", nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like an API change of invocation. Does this mean a breaking change originally from dubbo-go project?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the current dubbo-go version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see e0804e6 go.mod

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so move from RC to GA release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, what should I do for the next?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure CI passed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the current dubbo-go version?

v3.0.3. the old version 3.0.0-rc2 is out of time now.

@arugal
Copy link
Member

arugal commented Jul 8, 2022

Hi @dgqypl , please help to replace the link with https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh :)

curl -L https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.45.2 -> curl -L https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.45.2

right?

Yes

@AlexStocks
Copy link

@dgqypl thank you for your great work. pls attention that the current dubbogo version is v3.0.2, pls upgrade dubbogo version to the latest version. thank you.

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 8, 2022

@dgqypl thank you for your great work. pls attention that the current dubbogo version is v3.0.2, pls upgrade dubbogo version to the latest version. thank you.

I'll upgrade that tomorrow 😴.

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 9, 2022

@dgqypl thank you for your great work. pls attention that the current dubbogo version is v3.0.2, pls upgrade dubbogo version to the latest version. thank you.

already up to date

@arugal
Copy link
Member

arugal commented Jul 9, 2022

@dgqypl Also need to look at why golang-lint fails, are you interested?

I was successful on go 1.18.3, which is probably related to the golang version

@AlexStocks
Copy link

@dgqypl thank you for your great work. pls attention that the current dubbogo version is v3.0.2, pls upgrade dubbogo version to the latest version. thank you.

I'll upgrade that tomorrow 😴.

Please change this pr's title, thx.

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 9, 2022

@dgqypl Also need to look at why golang-lint fails, are you interested?

I was successful on go 1.18.3, which is probably related to the golang version

Actually, I don't know how to do right now, can you give me some tips, thx.

@dgqypl dgqypl changed the title supported dubbo-go v3.0.1 supported dubbo-go v3.0.2 Jul 9, 2022
@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 9, 2022

@dgqypl thank you for your great work. pls attention that the current dubbogo version is v3.0.2, pls upgrade dubbogo version to the latest version. thank you.

I'll upgrade that tomorrow 😴.

Please change this pr's title, thx.

changed

@AlexStocks
Copy link

AlexStocks commented Jul 9, 2022

@dgqypl Also need to look at why golang-lint fails, are you interested?

I was successful on go 1.18.3, which is probably related to the golang version

as a basic lib, the old version is perfect for many users. the older, the better.

@wu-sheng
Copy link
Member

wu-sheng commented Jul 9, 2022

Dubbo go case seems running on go 1.14. Is 14 still working for this latest Dubbo version?

@wu-sheng
Copy link
Member

wu-sheng commented Jul 9, 2022

Take a look at the GitHub control file.

@wu-sheng
Copy link
Member

wu-sheng commented Jul 9, 2022

I mean CI control file, I am not sure which part controls env initialization.

@arugal
Copy link
Member

arugal commented Jul 9, 2022

https://github.com/SkyAPM/go2sky-plugins/blob/master/.github/workflows/ci.yaml#L42

@dgqypl Please change the golang version here to 1.15

@arugal
Copy link
Member

arugal commented Jul 9, 2022

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 9, 2022

https://github.com/SkyAPM/go2sky-plugins/blob/master/.github/workflows/ci.yaml#L42

@dgqypl Please change the golang version here to 1.15

ok, I'll modify later.

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 9, 2022

https://github.com/SkyAPM/go2sky-plugins/runs/7260518151?check_suite_focus=true

E2E test also failed, please check

right now left only this fails, how can I fix it?

@arugal
Copy link
Member

arugal commented Jul 9, 2022

Details

https://github.com/SkyAPM/go2sky-plugins/runs/7264192077?check_suite_focus=true#step:4:248

e2e logs show that the connection between dubbo-go and zookeeper is abnormal

@wu-sheng
Copy link
Member

@dgqypl Could you fix the test?

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 11, 2022

@dgqypl Could you fix the test?

I had already fix it right now.

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 11, 2022

from the latest container log, it shows:

2022-07-11T01:57:01.764Z	ERROR	config/provider_config.go:156	Dubbogo do not read service interface name with registeredTypeName = UserProvider.Please run go install github.com/dubbogo/tools/cmd/protoc-gen-go-triple@latest to update your protoc-gen-go-triple and re-generate your pb file again.If you are not using pb serialization, please set 'interface' field in service config.

I really don't know how to fix it. Is there anyone who can directly tell me how to do.

@wu-sheng
Copy link
Member

This seems a dubbo go error? I think it is better to ask them.

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 11, 2022

@AlexStocks can you help me to fix it? thx

@zhaoyunxing92
Copy link

@AlexStocks can you help me to fix it? thx

how should i contact you

@AlexStocks
Copy link

@AlexStocks can you help me to fix it? thx

Pls join the dingding group 23331795 and contact with this group owner yuyu[me] and then let's have a discussion with @zhaoyunxing92 .

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 14, 2022

I had already fix CI error associated with dubbogo, so the only thing left is CI error with expected.data.yml, I don't know how to modify this file and let the CI passed, anybody can help me to resolve this problem?

@wu-sheng
Copy link
Member

This is an e2e test framework go2sky is using.

https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/plugin-test/

This section is about the expected file, https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/plugin-test/#expecteddatayaml

Generally, it is about what the traces you collected should be

@wu-sheng
Copy link
Member

I think you forgot to revert GHA file changes.

@dgqypl
Copy link
Contributor Author

dgqypl commented Jul 18, 2022

The latest commit is reverted , please try again the workflows, I'm tested CI passed in my forked repository.

@wu-sheng wu-sheng requested a review from arugal July 18, 2022 08:57
@wu-sheng
Copy link
Member

@arugal @AlexStocks Please take a look.

Copy link
Member

@arugal arugal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thank you for your contribution :)

Copy link

@AlexStocks AlexStocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wu-sheng wu-sheng merged commit cb8f743 into SkyAPM:master Jul 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
5 participants