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

feat: add go-kratos plugin #28

Merged
merged 10 commits into from
Aug 10, 2021
Merged

feat: add go-kratos plugin #28

merged 10 commits into from
Aug 10, 2021

Conversation

kagaya85
Copy link
Member

@kagaya85 kagaya85 commented Aug 6, 2021

add a plugin for go-kratos framework

@kagaya85 kagaya85 marked this pull request as draft August 6, 2021 15:05
@wu-sheng
Copy link
Member

wu-sheng commented Aug 6, 2021

Take a reference about how to do plugin e2e testing.

  1. https://github.com/SkyAPM/go2sky-plugins/blob/master/.github/workflows/plugin_test.yaml
  2. Add all plugin test #24

@mrproliu We really need skywalking-infra-e2e doc ready and get to release ASAP.

@wu-sheng wu-sheng requested a review from arugal August 6, 2021 15:13
@wu-sheng wu-sheng added enhancement New feature or request plugin labels Aug 6, 2021
@mrproliu
Copy link
Contributor

mrproliu commented Aug 6, 2021

@mrproliu We really need skywalking-infra-e2e doc ready and get to release ASAP.

OK. I'll start it in this weekend.

@kagaya85 kagaya85 marked this pull request as ready for review August 7, 2021 13:25
@wu-sheng wu-sheng added this to the 1.2.0 milestone Aug 7, 2021
@wu-sheng wu-sheng requested a review from arugal August 9, 2021 06:48
@wu-sheng
Copy link
Member

wu-sheng commented Aug 9, 2021

@arugal Please check.

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

@arugal arugal merged commit f77cfce into SkyAPM:master Aug 10, 2021

const (
componentIDGoKratosServer = 5010
componentIDGoKratosClient = 5011
Copy link
Member

@arugal arugal Aug 10, 2021

Choose a reason for hiding this comment

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

@kagaya85 The componentID need to be define in the component-libraries.yml.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@kagaya85 Let's merge these two, and you could pick one component ID.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request plugin
Projects
None yet
4 participants