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.

Sign up for GitHub

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

Jump to bottom

refactor: split metrics module into multiple files #2310

Merged
merged 2 commits into from
May 30, 2023
Merged

refactor: split metrics module into multiple files #2310

merged 2 commits into from
May 30, 2023

Conversation

ev1lQuark
Copy link
Contributor

I have read the code related to the metrics module and plan to refactor the existing code before adding new metric instrumentation. This will make it easier for future development work. Could you please review the current file structure design to see if it conforms to community standards?

我阅读了 metrics 模块相关的代码，在添加新的指标埋点之前，先对已有的代码进行重构，便于后续的开发工作。请查看目前的文件结构设计是否符合社区规范？

@sonarcloud
Copy link

sonarcloud bot commented Apr 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@ev1lQuark
Copy link
Contributor Author

What happend in CI? How should I modify it? @FinalT @justxuewei

@justxuewei
Copy link
Member

justxuewei commented May 5, 2023 •
edited
Loading

What happend in CI? How should I modify it? @FinalT @justxuewei

I reran those CI tests.

@ev1lQuark
Copy link
Contributor Author

What happend in CI? How should I modify it? @FinalT @justxuewei

I reran those CI tests.

Thank you. But it still went wrong.

2023-05-05T06:26:35.213Z	ERROR	polaris/service_discovery.go:193	Could not query the instances for service: myApp . It happened err Polaris-1004(ErrCodeAPITimeoutError): retry times exceed 1 in SyncGetResources, serviceKey: {namespace: "dubbogo", service: "myApp"}, timeout is 1s
dubbo.apache.org/dubbo-go/v3/registry/polaris.(*polarisServiceDiscovery).GetInstances
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/polaris/service_discovery.go:193
dubbo.apache.org/dubbo-go/v3/registry/servicediscovery.(*ServiceDiscoveryRegistry).LoadSubscribeInstances
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/servicediscovery/service_discovery_registry.go:256
dubbo.apache.org/dubbo-go/v3/registry/directory.NewRegistryDirectory
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/directory/directory.go:97
dubbo.apache.org/dubbo-go/v3/common/extension.GetDefaultRegistryDirectory
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/common/extension/registry_directory.go:40
dubbo.apache.org/dubbo-go/v3/registry/protocol.(*registryProtocol).Refer
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/protocol/protocol.go:152
dubbo.apache.org/dubbo-go/v3/config.(*ReferenceConfig).Refer
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/config/reference_config.go:238
dubbo.apache.org/dubbo-go/v3/config.(*ConsumerConfig).Load
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/config/consumer_config.go:132
dubbo.apache.org/dubbo-go/v3/config.(*RootConfig).Start.func1
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/config/root_config.go:212
sync.(*Once).doSlow
	/opt/hostedtoolcache/go/1.17.13/x64/src/sync/once.go:68
sync.(*Once).Do
	/opt/hostedtoolcache/go/1.17.13/x64/src/sync/once.go:59
dubbo.apache.org/dubbo-go/v3/config.(*RootConfig).Start
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/config/root_config.go:210
dubbo.apache.org/dubbo-go/v3/config.(*RootConfig).Init
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/config/root_config.go:205
dubbo.apache.org/dubbo-go/v3/config.Load
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/config/config_loader.go:59
github.com/apache/dubbo-go-samples/integrate_test/polaris/registry/tests/integration.TestMain
	/home/runner/work/dubbo-go/dubbo-go/samples/integrate_test/polaris/registry/tests/integration/main_test.go:60
main.main
	_testmain.go:45
runtime.main
	/opt/hostedtoolcache/go/1.17.13/x64/src/runtime/proc.go:255
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x12a8695]

goroutine 84 [running]:
dubbo.apache.org/dubbo-go/v3/metadata/mapping/metadata.(*MetadataServiceNameMapping).Get(0x30, 0x30, {0x1eda438, 0xc0008c9740})
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/metadata/mapping/metadata/service_name_mapping.go:83 +0x75
dubbo.apache.org/dubbo-go/v3/registry/servicediscovery.(*ServiceDiscoveryRegistry).findMappedServices(0xc000295d40, 0xc0007fa000, {0x1eda438, 0xc0008c9740})
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/servicediscovery/service_discovery_registry.go:343 +0x5f
dubbo.apache.org/dubbo-go/v3/registry/servicediscovery.(*ServiceDiscoveryRegistry).getServices(0xc000295d40, 0xc00061beb8, {0x1eda438, 0xc0008c9740})
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/servicediscovery/service_discovery_registry.go:334 +0xc5
dubbo.apache.org/dubbo-go/v3/registry/servicediscovery.(*ServiceDiscoveryRegistry).Subscribe(0xc000295d40, 0xc0007fa000, {0x1eda2f8, 0xc0007fa780})
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/servicediscovery/service_discovery_registry.go:210 +0x250
dubbo.apache.org/dubbo-go/v3/registry/directory.(*RegistryDirectory).Subscribe(0xc0007fa780, 0xc0007fa000)
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/directory/directory.go:107 +0xbe
created by dubbo.apache.org/dubbo-go/v3/registry/protocol.(*registryProtocol).Refer
	/home/runner/go/pkg/mod/github.com/ev1l!quark/dubbo-go/v3@v3.0.0-20230427052823-322a9cdbded9/registry/protocol/protocol.go:164 +0x21d
FAIL	github.com/apache/dubbo-go-samples/integrate_test/polaris/registry/tests/integration	4.553s
FAIL

@justxuewei
Copy link
Member

2023-05-05T06:26:35.213Z ERROR polaris/service_discovery.go:193 Could not query the instances for service: myApp . It happened err Polaris-1004(ErrCodeAPITimeoutError): retry times exceed 1 in SyncGetResources, serviceKey: {namespace: "dubbogo", service: "myApp"}, timeout is 1s

@chuntaojun Could you take a look?

@sonarcloud
Copy link

sonarcloud bot commented May 29, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jasondeng1997
Copy link
Member

有设计文档吗

@chickenlj
Copy link
Contributor

https://cn.dubbo.apache.org/zh-cn/overview/reference/metrics/standard_metrics/

https://cn.dubbo.apache.org/zh-cn/overview/core-features/observability/#metrics

chickenlj
chickenlj approved these changes May 30, 2023
View reviewed changes
Copy link
Contributor

@chickenlj chickenlj left a comment

Choose a reason for hiding this comment

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

LGTM.

@chickenlj chickenlj merged commit 66a152c into apache:main May 30, 2023
@ev1lQuark ev1lQuark mentioned this pull request May 30, 2023
Merged
@ev1lQuark ev1lQuark deleted the metrics branch June 5, 2023 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FinalT FinalT FinalT left review comments

@chickenlj chickenlj chickenlj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ev1lQuark @justxuewei @jasondeng1997 @chickenlj @FinalT