Support JSON format logs in `file-metrics-collector` #1765

tenzen-y · 2022-01-01T03:33:07Z

What this PR does / why we need it:
Support JSON format logs in file-metrics-collector.
Also modified the script for E2E so that the test does not fail even if the Trial status becomes EarlyStopped.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1744

Checklist:

Docs included if any changes are user facing

coveralls · 2022-01-01T03:54:57Z

Coverage decreased (-0.4%) to 73.802% when pulling cb456dd on tenzen-y:support-json-format-in-file-metrics-collector into 36d0a57 on kubeflow:master.

tenzen-y · 2022-01-01T13:04:48Z

/retest

tenzen-y · 2022-01-01T20:13:52Z

/retest

tenzen-y · 2022-01-01T23:07:54Z

This PR is ready for review, please take a look.
/assign @andreyvelich @gaocegege @johnugeorge

cmd/metricscollector/v1beta1/file-metricscollector/main.go

tenzen-y · 2022-01-04T03:16:57Z

I addressed your suggestion. @gaocegege
Please take another look.

tenzen-y · 2022-01-11T22:29:33Z

I'll re-run Charmed Katib / Test (pull_request) by force push.

gaocegege

Sorry for the late reply.

The PR LGTM!

Thanks for your contribution! 🎉 👍

/cc @andreyvelich @johnugeorge

andreyvelich

@tenzen-y Thanks a lot for implementing this feature!
I left some comments.
Also, please create PR to update Katib Website documentation about the new API and the new Metrics Collector functionality.

As we discussed on our meeting today, we will deliver this feature in the Katib 0.14 release, since it requires some additional testing and discussion.

/hold

pkg/webhook/v1beta1/pod/inject_webhook_test.go

pkg/metricscollector/v1beta1/file-metricscollector/testdata/JSON/good.json

cmd/metricscollector/v1beta1/file-metricscollector/main.go

andreyvelich · 2022-01-12T15:00:01Z

pkg/metricscollector/v1beta1/common/const.go

@@ -46,6 +46,8 @@ const (
 	// Score=1.23E10
 	DefaultFilter = `([\w|-]+)\s*=\s*([+-]?\d*(\.\d+)?([Ee][+-]?\d+)?)`

+	TimeStampJsonKey = "timestamp"


I think we should document this somewhere.

Does this mean we need to add documentation other than the website documentation? @andreyvelich

I meant just point about it in the website documentation.

pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector.go

pkg/webhook/v1beta1/experiment/validator/validator.go

pkg/webhook/v1beta1/pod/inject_webhook_test.go

test/e2e/v1beta1/run-e2e-experiment.go

johnugeorge · 2022-04-04T17:20:30Z

Katib CI is broken due to #1842

johnugeorge · 2022-04-05T11:54:34Z

LGTM. Can you rebase the PR and we can then merge

tenzen-y · 2022-04-05T14:21:32Z

LGTM. Can you rebase the PR and we can then merge

Sure.

Co-authored-by: Andrey Velichkevich <andrey.velichkevich@gmail.com>

tenzen-y · 2022-04-05T14:38:51Z

@johnugeorge
I have rebased this PR to fix the broken CI.

johnugeorge · 2022-04-05T15:19:20Z

Thanks @tenzen-y for your patience. Sorry for the delay

/lgtm
/approve

google-oss-prow · 2022-04-05T15:19:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge, tenzen-y

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [johnugeorge]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

johnugeorge · 2022-04-05T15:20:08Z

/unhold

google-oss-prow bot added do-not-merge/work-in-progress size/XL labels Jan 1, 2022

google-oss-prow bot requested review from gaocegege, hougangliu and sperlingxx January 1, 2022 03:33

tenzen-y force-pushed the support-json-format-in-file-metrics-collector branch 2 times, most recently from 0dfb912 to 543d245 Compare January 1, 2022 03:48

tenzen-y force-pushed the support-json-format-in-file-metrics-collector branch from 543d245 to 2e6eff1 Compare January 1, 2022 11:47

tenzen-y force-pushed the support-json-format-in-file-metrics-collector branch 3 times, most recently from fcb2f0a to c5bd812 Compare January 1, 2022 16:11

tenzen-y changed the title ~~[WIP] Support JSON format logs in file-metrics-collector~~ Support JSON format logs in file-metrics-collector Jan 1, 2022

google-oss-prow bot removed the do-not-merge/work-in-progress label Jan 1, 2022

tenzen-y force-pushed the support-json-format-in-file-metrics-collector branch from c5bd812 to 1fb26bb Compare January 1, 2022 21:55

google-oss-prow bot assigned andreyvelich, gaocegege and johnugeorge Jan 1, 2022

gaocegege reviewed Jan 3, 2022

View reviewed changes

cmd/metricscollector/v1beta1/file-metricscollector/main.go Outdated Show resolved Hide resolved

tenzen-y force-pushed the support-json-format-in-file-metrics-collector branch from 4eb6d6c to 87d2a39 Compare January 11, 2022 22:30

gaocegege reviewed Jan 12, 2022

View reviewed changes

google-oss-prow bot requested review from andreyvelich and johnugeorge January 12, 2022 02:46

andreyvelich reviewed Jan 12, 2022

View reviewed changes

google-oss-prow bot added the do-not-merge/hold label Jan 12, 2022

tenzen-y and others added 15 commits April 5, 2022 23:22

support JSON format logs in file-metrics-collector

c82fdf5

review: convert fileFormat to type FileSystemFileFormat

572f071

Update cmd/metricscollector/v1beta1/file-metricscollector/main.go

0265372

Co-authored-by: Andrey Velichkevich <andrey.velichkevich@gmail.com>

review: remove func (f FileSystemFileFormat) String()

c38dd5a

review: get metricRegList only when the format is TEXT

703bed8

review: change var name in a script for e2e

d95f2a0

review: explict specify the cloudml-hypyertune in the Dockerfile

68ffbcc

review: use reflect.DeepEqual instead of go-cmp.Diff

2c8aaa1

review: stop using 'JSON' directly in error statements

8fb51d4

review: install specific version cloudml-hypertune

6930468

review: get objType in the updateStopRules function

774dd4b

review: save optimalObjValue across multiple stopRules

051952a

review: add warning messages to parseTimestamp func

35cfcff

review: generate test files with go test command

26f04ef

review: change api for new feature

cb456dd

tenzen-y force-pushed the support-json-format-in-file-metrics-collector branch from e5f2ae4 to cb456dd Compare April 5, 2022 14:22

google-oss-prow bot added the lgtm label Apr 5, 2022

google-oss-prow bot added the approved label Apr 5, 2022

google-oss-prow bot removed the do-not-merge/hold label Apr 5, 2022

google-oss-prow bot merged commit d443ed3 into kubeflow:master Apr 5, 2022

tenzen-y deleted the support-json-format-in-file-metrics-collector branch April 5, 2022 22:28

tenzen-y mentioned this pull request Apr 10, 2022

Add documentation on Katib's JSON format logs kubeflow/website#3219

Merged

andreyvelich mentioned this pull request Feb 24, 2023

Katib metrics collector fails at end of trial #2124

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support JSON format logs in `file-metrics-collector` #1765

Support JSON format logs in `file-metrics-collector` #1765

tenzen-y commented Jan 1, 2022 •

edited

Loading

coveralls commented Jan 1, 2022 •

edited

Loading

tenzen-y commented Jan 1, 2022

tenzen-y commented Jan 1, 2022

tenzen-y commented Jan 1, 2022

tenzen-y commented Jan 4, 2022

tenzen-y commented Jan 11, 2022 •

edited

Loading

gaocegege left a comment

andreyvelich left a comment

andreyvelich Jan 12, 2022

tenzen-y Jan 13, 2022

andreyvelich Jan 13, 2022

johnugeorge commented Apr 4, 2022

johnugeorge commented Apr 5, 2022 •

edited

Loading

tenzen-y commented Apr 5, 2022

tenzen-y commented Apr 5, 2022

johnugeorge commented Apr 5, 2022

google-oss-prow bot commented Apr 5, 2022

johnugeorge commented Apr 5, 2022

Support JSON format logs in file-metrics-collector #1765

Support JSON format logs in file-metrics-collector #1765

Conversation

tenzen-y commented Jan 1, 2022 • edited Loading

coveralls commented Jan 1, 2022 • edited Loading

tenzen-y commented Jan 1, 2022

tenzen-y commented Jan 1, 2022

tenzen-y commented Jan 1, 2022

tenzen-y commented Jan 4, 2022

tenzen-y commented Jan 11, 2022 • edited Loading

gaocegege left a comment

Choose a reason for hiding this comment

andreyvelich left a comment

Choose a reason for hiding this comment

andreyvelich Jan 12, 2022

Choose a reason for hiding this comment

tenzen-y Jan 13, 2022

Choose a reason for hiding this comment

andreyvelich Jan 13, 2022

Choose a reason for hiding this comment

johnugeorge commented Apr 4, 2022

johnugeorge commented Apr 5, 2022 • edited Loading

tenzen-y commented Apr 5, 2022

tenzen-y commented Apr 5, 2022

johnugeorge commented Apr 5, 2022

google-oss-prow bot commented Apr 5, 2022

johnugeorge commented Apr 5, 2022

Support JSON format logs in `file-metrics-collector` #1765

Support JSON format logs in `file-metrics-collector` #1765

tenzen-y commented Jan 1, 2022 •

edited

Loading

coveralls commented Jan 1, 2022 •

edited

Loading

tenzen-y commented Jan 11, 2022 •

edited

Loading

johnugeorge commented Apr 5, 2022 •

edited

Loading