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

[ISSUE #10662]Prometheus-sd add namespace and service api #10663

Merged
merged 8 commits into from
Jul 17, 2023

Conversation

Joey777210
Copy link
Contributor

@Joey777210 Joey777210 commented Jun 18, 2023

Please do not create a Pull Request without creating an issue first.

#10662

What is the purpose of the change

Promethesu sd api returns all service in once. But in lot scenes, Nacos use by alot systems. Needs isolation.
Add two interfaces, and specify Namespace and Service.

Brief changelog

Add two interfaces, and specify Namespace and Service.

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • [Y] Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • [Y] Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • [Y] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • [Y] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • [Y] Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

@KomachiSion
Copy link
Collaborator

As I comment in issue, does prometheus sd protocol can call the specified api?

And when you implement, Please pay attention to the auth feature.

* @throws NacosException NacosException.
*/
@GetMapping(value = ApiConstants.PROMETHEUS_CONTROLLER_NAMESPACE_PATH, produces = "application/json; charset=UTF-8")
public ResponseEntity metricNamespace(@PathVariable("namespace") String namespace) throws NacosException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we use namespaceId? nacos has namespaceShowName and namespaceId, directly use namespace may be let users wrong input namespaceShowName.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought "namespace" is "namespaceId".. Isn't it? If it is, may be variable name in Service.class should be changed, If not, I can try find a way to get namespaceId....

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes it's, but I mean users may be confused and may use showName

Copy link
Collaborator

Choose a reason for hiding this comment

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

Only change the parameters name

Copy link

Choose a reason for hiding this comment

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

Should we use namespaceId? nacos has namespaceShowName and namespaceId, directly use namespace may be let users wrong input namespaceShowName.

KomachiSion
KomachiSion previously approved these changes Jul 5, 2023
@Joey777210
Copy link
Contributor Author

@KomachiSion plz approve again

@KomachiSion
Copy link
Collaborator

It seems ci in prometheus module can't pass.

@KomachiSion
Copy link
Collaborator

It seems ci in prometheus module can't pass.

Error:    PrometheusControllerTest.testMetricNamespace:112 » IllegalArgument Not enough ...
Error:    PrometheusControllerTest.testMetricNamespaceService:125 » IllegalArgument Not ...

Please fix the CI first.

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2023

Codecov Report

Merging #10663 (de2dfe4) into develop (8fa83ce) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop   #10663      +/-   ##
=============================================
+ Coverage      53.41%   53.43%   +0.01%     
  Complexity      5620     5620              
=============================================
  Files            923      923              
  Lines          29431    29432       +1     
  Branches        3239     3239              
=============================================
+ Hits           15722    15727       +5     
+ Misses         12334    12328       -6     
- Partials        1375     1377       +2     
Impacted Files Coverage Δ
.../alibaba/nacos/common/remote/client/RpcClient.java 37.85% <100.00%> (+1.08%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 156cd62...de2dfe4. Read the comment docs.

@KomachiSion KomachiSion merged commit f986916 into alibaba:develop Jul 17, 2023
6 checks passed
@KomachiSion
Copy link
Collaborator

Welcome to submit document to nacos-group

realJackSun added a commit that referenced this pull request Aug 10, 2023
…lop branch (#10942)

* [ISSUE #10734] Implement http request param check filter and http param extractors (#10758)

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,alter the test case

* For #10734,delete the ConnectionSetupRequestParamExtractor

* For #10734,add the naming http request param check filter and implement the naming http request param extractors

* For #10734,add unit test for naming http request param extractors

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,delete the ConnectionSetupRequestParamExtractor

* For #10734,add the naming http request param check filter and implement the naming http request param extractors

* For #10734,add unit test for naming http request param extractors

* For #10734,add the config http request param check filter and implement the config http request param extractors and unit test

* For #10734,add the console http request param check filter and implement the console http request param extractors and unit test

* For #10734,fix code style

* For #10734,alter the logic of exception handle in filter

* For #10734,fix code style

* dump change check task submit (#10755)

* dump change check task submit

* delete config nid convert error fix

* fix test case

* checkstyle

* Refactor grpc tls (#10759)

* Move Tls negotiator to GrpcSdkServer.

* use protocol negotiator builder replace directly create.

* use SPI load negotiator and set tls as default negotiator.

* Remove tlsconfig in BaseRpcServer.

* Add some ut.

* For checkstyle.

* fix word spelling in `AuthenticationManagerDelegator` (#10777)

* fix(#10427): When the execution of handleServerRequest() encounters an exception, record the log and throw an exception, then quickly response to the server errResponse (#10770)

* fix(#10585): selectInstances and selectOneHealthyInstance methods, if the parameter subscribe is true. Subscription is required when clientProxy.isSubscribe() is false. (#10805)

* disable check port input when use registered port (#10799)

* Add the handle to overload connection (#10783)

* add the handle to overload connection

* fast return

* [ISSUE #10662]Prometheus-sd add namespace and service api (#10663)

* add apis

* add tests

* fix checkstyle

* param namespace to namespaceId

* namespace to namespaceId

* fix test case

* fix test case

* Service instance should display related color when healthy or unhealthy (#10811)

* fix a couple of invaild props (#10810)

* feat(#10539): When the operation configuration fails, log. (#10804)

* [ISSUE #10744]feat:Add HealthControllerV2 and HealthControllerV2Test (#10786)

* [ISSUE #10744]feat:Add HealthControllerV2 and HealthControllerV2Test

* fix:V2 api return Result

* [ISSUE #10734] Refactor the ParamChecker and ParamExtractor (#10775)

* For #10734,refactor the paramextractor and ParamChecker

* For #10734,alter the rules of ParamCheck

* For #10734,alter the rules of ParamCheck

* For #10734,fix bug

* For #10734,fix bug and alter the ParamCheckRules.java

* For #10734,fix code style

* For #10734,fix the param check rules

* For #10734,implement the server param check config

* For #10734,optimize the logic

* For #10734,optimize the logic

* For #10734,optimize the logic

* Refactor Prometheus Module (#10827)

* Refactor Prometheus Module

* Complete Test Case

* format

* format

* For #10734,fix the param check rule (#10826)

* [ISSUE #10792]When nacos client use endpoint, the registration center should support configuring context-path and cluster-name like the configuration center (#10793)

* Reactor code in datasource-plugin (#10791)

* Reactor code in datasource-plugin

* Fix Abstract Mapper Test Case

* Add Empty Check

* Fix Checkstyle

* fix checkstyle

* fix check style

* fix check style

* Fix CheckStyle

* Fix SQL Blank

* bugfix for PersistentClientOperationServiceImpl log (#10825)

* For #10734,fix the param check rule (#10858)

* fix(#10831): When using the deregisterInstance method to remove one of multiple instances registered by batchRegisterInstance, all instances registered by batchRegisterInstance will be removed. (#10836)

* UnsupportedFeatureError (#10860)

* fix(distro): fix issue#10880. (#10881)

* feat(#5608 && #10223): When the custom instance id is empty, the id will be automatically generated. (#10812)

* fix: test-code branch (#10904)

* add nacos ci

* delete client version of nacos ci

* fix: test-code branch

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题 (#10896)

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题

* 老规矩,要编译一波

* feat(10891): Provide a configuration item for the maximum number of push retries, instead of directly hardcoding it to 50 times in the code. (#10895)

* [ISSUE #10824] Remove udp port param for v1-client (#10914)

* Remove UDP Param

* Fix gRPC client

* fix test case

* fix test case

* fix test case

* fix test case

* Fix login failed when close auth.

---------

Co-authored-by: Sunrisea <[email protected]>
Co-authored-by: nov.lzf <[email protected]>
Co-authored-by: 杨翊 SionYang <[email protected]>
Co-authored-by: ZhangShenao <[email protected]>
Co-authored-by: blake.qiu <[email protected]>
Co-authored-by: Joey777210 <[email protected]>
Co-authored-by: chenyiqin <[email protected]>
Co-authored-by: maoling <[email protected]>
Co-authored-by: DiligenceLai <[email protected]>
Co-authored-by: huhongjie2014 <[email protected]>
Co-authored-by: lu-xiaoshuang <[email protected]>
Co-authored-by: zt9788 <[email protected]>
Co-authored-by: 阿魁 <[email protected]>
Co-authored-by: wuyfee <[email protected]>
Co-authored-by: Darren Luo <[email protected]>
Co-authored-by: xxc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants