Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[receiver/skywalking]: Refactor code structure/directory #20344

Merged
merged 20 commits into from
May 19, 2023

Conversation

aheling11
Copy link
Contributor

Description:

1st PR: In order to add metrics receiver to skywalking/receiver, I found that I need to adjust the code directory/code structure a bit. So this PR is just a code refactoring, referring to the structure of otlp/receiver. After that, it will be easier for skywalking/receiver to add metrics or logs to the receiver.

The new swReceiver struct is shown below.

type swReceiver struct {
	config *configuration

	grpc            *grpc.Server
	collectorServer *http.Server

	goroutines sync.WaitGroup

	settings receiver.CreateSettings

	traceReceiver *trace.Receiver
        metricsReceiver *metrics.Receiver // will be implemented in following PR
        

	dummyReportService *dummyReportService
}

Link to tracking Issue:
#20315

Testing:
The original unit tests have been passed, and a test for HTTP reception has been added. Because of some restructuring, the unit test code has changed a bit.

Documentation:

1. Restructuring the directory/structure
2. Adding an HTTP trace Reception unit test
@aheling11 aheling11 requested a review from a team as a code owner March 27, 2023 14:40
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 27, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: aheling11 / name: JeffreyHe (56fb2aa)

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 27, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@jpkrohling
Copy link
Member

Tests started.

@JaredTan95
Copy link
Member

@aheling11 CI passed, Let's wait for the approver to merge this PR.

@aheling11
Copy link
Contributor Author

There were some conflicts in the go.sum, but they have been resolved.

…y-collector-contrib into skywalking_add_metric

# Conflicts:
#	receiver/skywalkingreceiver/go.mod
#	receiver/skywalkingreceiver/go.sum
#	receiver/skywalkingreceiver/tracing_report_service.go
@aheling11
Copy link
Contributor Author

Could you please rerun the tests? I have resolved some conflicts. @jpkrohling

@JaredTan95
Copy link
Member

@aheling11 new conflicts :-P

…y-collector-contrib into skywalking_add_metric

# Conflicts:
#	receiver/skywalkingreceiver/factory.go
#	receiver/skywalkingreceiver/factory_test.go
@aheling11
Copy link
Contributor Author

@aheling11 new conflicts :-P

resolved again ~

@aheling11 aheling11 requested a review from JaredTan95 May 15, 2023 11:27
@aheling11
Copy link
Contributor Author

Why doesn't GitHub have a feature to retract review requests? I accidentally clicked the "Request review" button. =-=

@JaredTan95

@JaredTan95
Copy link
Member

JaredTan95 commented May 16, 2023

That's ok, take easy~, let's waiting maintainer to merge it.
@jpkrohling @atoulme @fatsheep9146

…y-collector-contrib into skywalking_add_metric

# Conflicts:
#	receiver/skywalkingreceiver/tracing_report_service.go
@aheling11
Copy link
Contributor Author

resolve conflict again.

@fatsheep9146
Copy link
Contributor

ping @open-telemetry/collector-contrib-maintainer I think this pr is ready to be merged

@atoulme atoulme added the ready to merge Code review completed; ready to merge by maintainers label May 19, 2023
@dmitryax dmitryax merged commit ad466f0 into open-telemetry:main May 19, 2023
89 checks passed
@github-actions github-actions bot added this to the next release milestone May 19, 2023
@aheling11 aheling11 deleted the skywalking_add_metric branch May 20, 2023 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers receiver/skywalking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants