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

抽象监控上报,增加推送到Prometheus的模块 #167

Closed
wants to merge 3 commits into from

Conversation

konglz
Copy link

@konglz konglz commented Oct 9, 2018

Describe what this PR does / why we need it

  • 抽象监控上报

  • 增加推送到Prometheus的模块

Does this pull request fix one issue?

Describe how you did it

  • 抽象监控上报,修改了sentinel-core

  • 增加推送到Prometheus的模块:sentinel-transport-prometheus-push

Describe how to verify it

  • 配置好Prometheus Server和Prometheus PushGateway

  • 增加一对JVM参数,例如:-Dsentinel.prometheus.gateway=127.0.0.1:9091 -Dsentinel.web.port=8080

    • sentinel.prometheus.gateway是Prometheus PushGateway的ip和端口号,不传默认取127.0.0.1:9091

    • sentinel.web.port是应用的web端口号,如无web端口,随便传1个即可,不传默认取0

  • 在Prometheus Server上收到5个标签,分别是:

    • SENTINEL_BLOCKED_QPS

    • SENTINEL_EXCEPTION_COUNT

    • SENTINEL_PASSED_QPS

    • SENTINEL_RT

    • SENTINEL_SUCCESS_QPS

  • 在Prometheus Server上看到,每个标签包含如下label:

    • language=java

    • appName

    • host

    • port

    • instance=ip:port

    • resource

Special notes for reviews

@CLAassistant
Copy link

CLAassistant commented Oct 9, 2018

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

codecov-io commented Oct 9, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@257dde1). Click here to learn what that means.
The diff coverage is 36.39%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #167   +/-   ##
=========================================
  Coverage          ?   50.76%           
  Complexity        ?      822           
=========================================
  Files             ?      140           
  Lines             ?     4759           
  Branches          ?      674           
=========================================
  Hits              ?     2416           
  Misses            ?     2059           
  Partials          ?      284
Impacted Files Coverage Δ Complexity Δ
...entinel/node/metric/MetricSearcherDefaultImpl.java 10.46% <10.46%> (ø) 3 <3> (?)
.../csp/sentinel/node/metric/MetricTimerListener.java 92% <100%> (ø) 8 <1> (?)
.../sentinel/node/metric/MetricWriterDefaultImpl.java 48.34% <48.34%> (ø) 19 <19> (?)
...libaba/csp/sentinel/node/metric/MetricManager.java 52.17% <52.17%> (ø) 2 <2> (?)

Continue to review full report at Codecov.

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

@sczyh30 sczyh30 added the to-review To review label Oct 9, 2018
@sczyh30
Copy link
Member

sczyh30 commented Oct 9, 2018

Hi, thanks for your contribution. As this PR can bring breaking changes, you'll need to open an issue related to this feature, then illustrate the thoughts and design. It's not recommended to submit a large PR directly without discussion. A new feature must be well designed and discussed with project members in relevant issue. Regarding this feature we have to consider with standardization of metrics and monitoring described in our roadmap.

By the way, you can add your own fork to Awesome Sentinel first :-)

@CarpenterLee
Copy link
Contributor

Thanks for your contribution. It's a good idea to store metrics to some existing monitoring systems, but before that, we should first to standardize our monitoring metrics. Now the metric's name is ambiguous and the monitor indexes are not rich enough, for example the MetricNode.exception is in fact exceptionQps other than total exception count. A standardization of metrics and monitoring will be designed in the latter version, welcome to join us for further discussion.

@sczyh30
Copy link
Member

sczyh30 commented Nov 26, 2018

Hi, discussions for metrics and monitoring is welcomed in #211.

@sentinel-bot
Copy link
Collaborator

Ping @konglz . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.

@sentinel-bot sentinel-bot added the merge-conflict Category prs with merge conflict. label Jan 18, 2019
@pouchrobot
Copy link

ping @konglz
Conflict happens after merging a previous commit.
Please rebase the branch against master and push it back again. Thanks a lot.

@sczyh30 sczyh30 closed this Jan 21, 2019
@sczyh30 sczyh30 removed merge-conflict Category prs with merge conflict. to-review To review labels Jan 21, 2019
@sczyh30 sczyh30 added the area/metrics Issues or PRs related to metrics and monitoring label Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Issues or PRs related to metrics and monitoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants