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

[Metric] Add prometheus metric implementation #2752

Merged
merged 6 commits into from
Feb 28, 2022
Merged

[Metric] Add prometheus metric implementation #2752

merged 6 commits into from
Feb 28, 2022

Conversation

zhongchun
Copy link
Contributor

What do these changes do?

This pr adds prometheus metrics implementation. We can use prometheus metrics as follows:

from mars.metric.api import init_metrics, Metrics

init_metrics({"metric": {"backend": "prometheus"}})
c = Metrics.counter("test_counter", "A test counter", ("service", "tenant"))
c.record(1)

g = Metrics.gauge("test_gauge", "A test gauge")
g.record(1)

m = Metrics.meter("test_meter")
m.record(1)

h = Metrics.histogram("test_histogram")
h.record(1)

Related issue number

Issuse #2743

Check code requirements

  • tests added / passed (if needed)
  • Ensure all linting tests pass, see here for how to run them

@qinxuye qinxuye mentioned this pull request Feb 25, 2022
Closed
4 tasks
wjsi
wjsi reviewed Feb 25, 2022
View reviewed changes
ci/requirements-wheel.txt Outdated
@@ -13,3 +13,4 @@ scipy==1.7.2; python_version>='3.10'
cython==0.29.26
requests>=2.4.0
cloudpickle>=1.5.0
prometheus-client>=0.10.0
Copy link
Member

Choose a reason for hiding this comment

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

Not needed when building wheels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

mars/metric/backends/prometheus/prometheus_metric.py Outdated
# limitations under the License.

from typing import Optional, Dict
from prometheus_client import Gauge as PGauge
Copy link
Member

Choose a reason for hiding this comment

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

We do not intend to depend on prometheus_client, thus use a try..except ImportError to wrap this import line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I'll fix it.

mars/metric/backends/prometheus/tests/test_prometheus_metric.py Outdated
import requests
import time

from prometheus_client import start_http_server
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

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'll fix it.

setup.cfg Outdated
@@ -44,6 +44,7 @@ install_requires =
sqlalchemy>=1.2.0
defusedxml>=0.5.0
uvloop>=0.14.0; sys.platform!="win32" and python_version>="3.7"
prometheus-client>=0.10.0
Copy link
Member

Choose a reason for hiding this comment

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

We do not intend to depend on prometheus_client, thus remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

@zhongchun zhongchun force-pushed the feat-add-prometheus-metric branch from f62374d to 8b430db Compare February 25, 2022 05:04
@qinxuye qinxuye added this to the v0.9.0b2 milestone Feb 25, 2022
@zhongchun zhongchun force-pushed the feat-add-prometheus-metric branch 3 times, most recently from 8d81c2a to 8f0d6d8 Compare February 25, 2022 09:24
@zhongchun zhongchun changed the title Add prometheus metric implementation [Metric]Add prometheus metric implementation Feb 25, 2022
@zhongchun zhongchun force-pushed the feat-add-prometheus-metric branch from 8f0d6d8 to ad8649e Compare February 25, 2022 10:10
wjsi
wjsi reviewed Feb 28, 2022
View reviewed changes
Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

LGTM over all, I left one comment.

mars/metric/api.py Outdated
logger.info("Finished startup prometheus http server and port is %d", port)
except ImportError:
logger.info(
"Do not startup prometheus http server because there is no prometheus_client"
Copy link
Member

Choose a reason for hiding this comment

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

Use logger.warning("Failed to start...").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Thanks.

@zhongchun zhongchun force-pushed the feat-add-prometheus-metric branch from 92d8a6b to 9108be2 Compare February 28, 2022 09:38
wjsi
wjsi approved these changes Feb 28, 2022
View reviewed changes
Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

LGTM

qinxuye
qinxuye approved these changes Feb 28, 2022
View reviewed changes
Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

@qinxuye qinxuye merged commit 2b5c19a into mars-project:master Feb 28, 2022
qinxuye pushed a commit to hekaisheng/mars that referenced this pull request Mar 1, 2022
@zhongchun
[Metric]Add prometheus metric implementation (mars-project#2752)
3c35bb3
@zhongchun zhongchun changed the title [Metric]Add prometheus metric implementation [Metric] Add prometheus metric implementation Mar 4, 2022
chaokunyang pushed a commit to chaokunyang/mars that referenced this pull request May 31, 2022
@zhongchun
[Metric]Add prometheus metric implementation (mars-project#2752)
5522c80
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjsi wjsi wjsi approved these changes

@qinxuye qinxuye qinxuye approved these changes

@hekaisheng hekaisheng Awaiting requested review from hekaisheng

Assignees
No one assigned
Labels
mod: metric type: feature New feature
Projects
None yet
Milestone
v0.9.0b2
Development

Successfully merging this pull request may close these issues.
3 participants
@zhongchun @qinxuye @wjsi