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

Metrics #322

Merged
merged 16 commits into from
Dec 6, 2023
Merged

Metrics #322

merged 16 commits into from
Dec 6, 2023

Conversation

s0nicboOm
Copy link
Contributor

@s0nicboOm s0nicboOm commented Nov 16, 2023 •
edited

MAJOR:
Introduce metrics to the udfs.

MINOR:

  1. Fix logs for insufficient data
  2. Add logs in the scenario preproc model is absent and we forward the message to trainer directly.

@s0nicboOm s0nicboOm changed the title fix: logging for insufficient data Metrics Nov 16, 2023
@codecov Codecov
Copy link

codecov bot commented Nov 16, 2023 •
edited

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (dfc383a) 93.78% compared to head (a2e7cbf) 93.42%.

Files Patch % Lines
numalogic/monitoring/metrics.py 83.78% 3 Missing and 3 partials ⚠️
numalogic/udfs/trainer/_prom.py 16.66% 5 Missing ⚠️
numalogic/udfs/trainer/_base.py 84.21% 3 Missing ⚠️
numalogic/monitoring/__init__.py 71.42% 2 Missing ⚠️
numalogic/udfs/preprocess.py 86.66% 2 Missing ⚠️
numalogic/udfs/__main__.py 75.00% 1 Missing ⚠️
numalogic/udfs/postprocess.py 87.50% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
- Coverage   93.78%   93.42%   -0.36%     
==========================================
  Files          79       82       +3     
  Lines        3570     3712     +142     
  Branches      292      294       +2     
==========================================
+ Hits         3348     3468     +120     
- Misses        176      194      +18     
- Partials       46       50       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@s0nicboOm
fix: logging for insufficient data
a87c927 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
s0nicboOm and others added 6 commits November 29, 2023 11:44
@s0nicboOm
Merge branch 'main' into metrics
e76e5c4 
Signed-off-by: Kushal Batra <34571348+s0nicboOm@users.noreply.github.com>
@s0nicboOm
fix: logging
7db5131 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm
fix: typos
b2ce065 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm
fix: rename metrics to _metrics
9c761a1 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm
Merge branch 'main' into metrics
05c6587 
Signed-off-by: Kushal Batra <34571348+s0nicboOm@users.noreply.github.com>
@s0nicboOm
fix: add a TODO for metrics server port
908f161 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm
update port number for metric server
3915f13 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm
add another metric for measuring fetch time
a6e9985 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm
add: readme for metrics
aebe32c 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm
fix: typo
43e014b 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm s0nicboOm marked this pull request as ready for review December 4, 2023 19:44
@s0nicboOm
fix: update toml to 0.6.1.dev6
b114067 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
ab93
ab93 reviewed Dec 4, 2023
View reviewed changes
Copy link
Member

@ab93 ab93 left a comment

Choose a reason for hiding this comment

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

Few comments

numalogic/udfs/__main__.py Outdated Show resolved Hide resolved
numalogic/udfs/inference.py Outdated Show resolved Hide resolved
numalogic/udfs/inference.py Outdated Show resolved Hide resolved
@s0nicboOm
fix: comments
119675d 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
ab93
ab93 requested changes Dec 4, 2023
View reviewed changes
numalogic/monitoring/metrics.py Outdated Show resolved Hide resolved
numalogic/monitoring/metrics.py Outdated Show resolved Hide resolved
numalogic/monitoring/metrics.py Outdated Show resolved Hide resolved
numalogic/monitoring/metrics.py Outdated Show resolved Hide resolved
numalogic/monitoring/metrics.py Outdated Show resolved Hide resolved
numalogic/monitoring/metrics.py Outdated Show resolved Hide resolved
numalogic/udfs/README.md Show resolved Hide resolved
numalogic/udfs/_metrics.py Outdated Show resolved Hide resolved
numalogic/udfs/preprocess.py Outdated Show resolved Hide resolved
@s0nicboOm
fix: comments
5581af7 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm
fix: error
fc1711a 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
ab93
ab93 approved these changes Dec 5, 2023
View reviewed changes
Copy link
Member

@ab93 ab93 left a comment

Choose a reason for hiding this comment

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

Approved, with a few comments

numalogic/monitoring/metrics.py Outdated Show resolved Hide resolved
numalogic/udfs/_metrics.py Outdated Show resolved Hide resolved
numalogic/udfs/_metrics.py Outdated Show resolved Hide resolved
@s0nicboOm
fix: docstrings
a2e7cbf 
Signed-off-by: s0nicboOm <i.kushalbatra@gmail.com>
@s0nicboOm s0nicboOm merged commit 28fa28f into numaproj:main Dec 6, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ab93 ab93 ab93 approved these changes

@qhuai qhuai Awaiting requested review from qhuai qhuai is a code owner

@nkoppisetty nkoppisetty Awaiting requested review from nkoppisetty nkoppisetty is a code owner

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@s0nicboOm @ab93