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

Add get prometheus data to run snafu #173

Merged
merged 17 commits into from
Aug 17, 2020
Merged

Add get prometheus data to run snafu #173

merged 17 commits into from
Aug 17, 2020

Conversation

acalhounRH
Copy link
Contributor

Added code to get Prometheus data and index it into Elasticsearch. Note this PR enables the functionality but does not use it. follow-on PRs will address specific tool triggers for Prometheus data collection.

Expected data flow would be that at the conclusion of a sample loop, the tool wrapper would yield a tuple (action, "get_prometheus_trigger"), this would trigger the collection of Prometheus data.

"action"(dict) would contain the following:

                action: {
                          "uuid": <uuid>
                          "user": <user>
                          "clustername": <clustername>
                          "starttime": <datetime> datetime.utcnow().strftime('%s')
                          "endtime": <datetime>
                          test_config: {...}
                        }

test_config would contain the sample specific test parameters that align to the current time window.

@jtaleric @bengland2 @dry923 @aakarshg

@acalhounRH
Copy link
Contributor Author

/rerun all

@aakarshg aakarshg added the ok to test Kick off our CI framework label Apr 20, 2020
@aakarshg
Copy link
Contributor

/rerun all

@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper PASS 00:16:25
fs_drift_wrapper PASS 00:07:11
hammerdb PASS 00:07:20
iperf PASS 00:04:49
pgbench_wrapper PASS 00:03:58
smallfile_wrapper PASS 00:07:17
sysbench PASS 00:03:30
uperf_wrapper PASS 00:18:56
ycsb_wrapper PASS 00:11:14

@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper PASS 00:15:59
fs_drift_wrapper PASS 00:06:56
hammerdb PASS 00:07:27
iperf PASS 00:04:36
pgbench_wrapper PASS 00:04:02
smallfile_wrapper PASS 00:07:12
sysbench PASS 00:02:51
uperf_wrapper FAIL 00:24:02
ycsb_wrapper PASS 00:11:07

@acalhounRH
Copy link
Contributor Author

/rerun all

@bengland2 bengland2 self-assigned this Apr 22, 2020
bengland2
bengland2 suggested changes Apr 22, 2020
View reviewed changes
Copy link
Contributor

@bengland2 bengland2 left a comment

Choose a reason for hiding this comment

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

Overall structure of the code is good, I think. I like get_valid_es_document code reuse.

This is missing documentation - how does the benchmark developer switch over to this? How does the user make use of it? What are advantages of doing it this way for the user? Since most kubernetes users are familiar with Prometheus, I think we have to provide some justification for doing things this way. If the answer is short put it in README.md but if not, put it in a separate .md and reference it in README.md, right?

What kinds of prometheus data can you handle, and if you see data that you can't handle, do you alert the user to it and handle it without crashing?

BTW, I could be wrong in my inline comments but I think others might have similar questions, at a minimum comments or documentation should explain right?

utils/get_prometheus_data.py Outdated Show resolved Hide resolved
utils/get_prometheus_data.py Outdated Show resolved Hide resolved
utils/get_prometheus_data.py Outdated Show resolved Hide resolved
utils/get_prometheus_data.py Outdated Show resolved Hide resolved
@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper PASS 00:16:11
fs_drift_wrapper PASS 00:06:39
hammerdb PASS 00:07:07
iperf PASS 00:07:40
pgbench_wrapper PASS 00:04:14
smallfile_wrapper PASS 00:06:50
sysbench PASS 00:03:04
uperf_wrapper FAIL 00:30:07
ycsb_wrapper PASS 00:10:16

@acalhounRH acalhounRH mentioned this pull request May 28, 2020
Closed
aakarshg
aakarshg reviewed May 28, 2020
View reviewed changes
test-requirements.txt Outdated Show resolved Hide resolved
aakarshg
aakarshg reviewed May 28, 2020
View reviewed changes
utils/py_es_bulk.py Outdated Show resolved Hide resolved
@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper FAIL 00:04:54
fs_drift_wrapper FAIL 00:04:02
hammerdb FAIL 00:12:54
iperf PASS 00:02:49
pgbench_wrapper FAIL 00:03:11
smallfile_wrapper FAIL 00:04:14
sysbench PASS 00:04:16
uperf_wrapper FAIL 00:03:36
ycsb_wrapper FAIL 00:14:22

@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper FAIL 00:04:23
fs_drift_wrapper FAIL 00:04:13
hammerdb FAIL 00:11:35
iperf PASS 00:03:05
pgbench_wrapper FAIL 00:03:29
smallfile_wrapper FAIL 00:05:02
sysbench PASS 00:02:20
uperf_wrapper FAIL 00:03:57
ycsb_wrapper FAIL 00:14:17

@acalhounRH acalhounRH requested a review from bengland2 May 29, 2020 02:22
@acalhounRH
Copy link
Contributor Author

/rerun all

@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper PASS 00:11:32
fs_drift_wrapper PASS 00:06:33
hammerdb PASS 00:06:32
iperf PASS 00:03:45
pgbench_wrapper PASS 00:03:19
smallfile_wrapper PASS 00:06:06
sysbench PASS 00:02:28
uperf_wrapper PASS 00:17:00
ycsb_wrapper PASS 00:06:25

@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper PASS 00:11:49
fs_drift_wrapper PASS 00:05:32
hammerdb PASS 00:06:04
iperf PASS 00:02:51
pgbench_wrapper PASS 00:03:07
smallfile_wrapper PASS 00:06:11
sysbench PASS 00:01:56
uperf_wrapper FAIL 00:07:41
ycsb_wrapper PASS 00:07:11

@acalhounRH
Copy link
Contributor Author

/rerun all

@acalhounRH
Copy link
Contributor Author

functionally tested PR no issues running fio with and without parallel indexing.

@aakarshg
@dry923

ready to merge, pending your review.

@rht-perf-ci
Copy link

Results for SNAFU CI Test

Test Result Runtime
fio_wrapper PASS 00:10:57
fs_drift_wrapper PASS 00:06:25
hammerdb PASS 00:07:14
iperf PASS 00:03:30
pgbench_wrapper PASS 00:03:31
smallfile_wrapper PASS 00:06:58
sysbench FAIL 00:01:17
uperf_wrapper FAIL 00:07:25
ycsb_wrapper PASS 00:08:13

@acalhounRH
Copy link
Contributor Author

I don't have write access, someone will have to resolve the conflict with run_snafu.py. @aakarshg

@bengland2
Copy link
Contributor

@acalhounRH I'll give a quick try

bengland2
bengland2 approved these changes Jun 25, 2020
View reviewed changes
Copy link
Contributor

@bengland2 bengland2 left a comment

Choose a reason for hiding this comment

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

I want to see how well Grafana dashboards are going to work with this, I talked with Alex, we need to get to the point where we are looking at real queries from some benchmark and seeing how well prom data integrates into elastic search. but we have to start doing something to kick the InfluxDB habit + copying prometheus data by hand, this seems like a good start. I want to see a better way to get a comprehensive current list of prom metrics, don't understand how to do that yet. Some documentation of how this is done would be nice.

@aakarshg aakarshg requested a review from dry923 June 25, 2020 22:14
@aakarshg
Copy link
Contributor

I don't have write access, someone will have to resolve the conflict with run_snafu.py. @aakarshg

Hey this is a rebase issue, so you'll need to pull down the pr and the master branch and do a rebase ( fixing the merge conflicts ).

@rsevilla87
Copy link
Member

/rerun all

@rsevilla87
Copy link
Member

rsevilla87 commented Aug 12, 2020 •
edited
Loading

Hey @acalhounRH, I added some commits to this PR required to solve the dependency issues you're hitting.

@comet-perf-ci
Copy link
Collaborator

Results for SNAFU CI Test

Test Result Runtime
src/fio_wrapper PASS 00:13:50
src/fs_drift_wrapper PASS 00:09:22
src/hammerdb PASS 00:07:40
src/iperf PASS 00:05:55
src/pgbench_wrapper PASS 00:05:43
src/smallfile_wrapper PASS 00:09:49
src/sysbench PASS 00:02:56
src/uperf_wrapper PASS 00:27:33
src/vegeta_wrapper PASS 00:08:34
src/ycsb_wrapper FAIL 00:12:11

@rsevilla87
Resolve RPM conflicts
2996b4f 
Signed-off-by: Raul Sevilla <rsevilla@redhat.com>
@comet-perf-ci
Copy link
Collaborator

Results for SNAFU CI Test

Test Result Runtime
src/fio_wrapper PASS 00:13:36
src/fs_drift_wrapper PASS 00:09:13
src/hammerdb PASS 00:07:32
src/iperf PASS 00:06:06
src/pgbench_wrapper PASS 00:05:58
src/smallfile_wrapper PASS 00:09:44
src/sysbench PASS 00:03:03
src/uperf_wrapper PASS 00:28:44
src/vegeta_wrapper PASS 00:08:57
src/ycsb_wrapper PASS 00:08:22

@bengland2
Copy link
Contributor

Thx Raul!

@rsevilla87
Copy link
Member

Hey @acalhounRH, in addition to linting issues there are code issues as well:

./src/utils/get_prometheus_data.py:42:89: F821 undefined name 'disable_ssl'
            self.pc = PrometheusConnect(url=self.url, headers=self.headers, disable_ssl=disable_ssl)
./src/utils/get_prometheus_data.py:104:66: F821 undefined name 'ROC'
                                    "rate_of_change_per_second": ROC,
./src/utils/py_es_bulk.py:177:13: F821 undefined name 'logger'
            logger.warn(resp)

Can you please take a look?

@acalhounRH
Copy link
Contributor Author

Will start fixing those issue, thanks for the help with resolving those dependency issues.

@comet-perf-ci
Copy link
Collaborator

Results for SNAFU CI Test

Test Result Runtime
src/fio_wrapper FAIL 00:06:37
src/fs_drift_wrapper FAIL 00:06:25
src/hammerdb FAIL 00:13:33
src/iperf PASS 00:06:11
src/pgbench_wrapper PASS 00:05:43
src/smallfile_wrapper FAIL 00:06:27
src/sysbench PASS 00:03:06
src/uperf_wrapper FAIL 00:08:09
src/vegeta_wrapper FAIL 00:11:40
src/ycsb_wrapper FAIL 00:12:57

@rsevilla87
Fix version printing
0f0043d 
Signed-off-by: Raul Sevilla <rsevilla@redhat.com>
@comet-perf-ci
Copy link
Collaborator

Results for SNAFU CI Test

Test Result Runtime
src/fio_wrapper PASS 00:15:00
src/fs_drift_wrapper PASS 00:09:38
src/hammerdb PASS 00:08:30
src/iperf PASS 00:06:26
src/pgbench_wrapper PASS 00:06:33
src/smallfile_wrapper PASS 00:10:04
src/sysbench PASS 00:03:10
src/uperf_wrapper PASS 00:29:19
src/vegeta_wrapper PASS 00:13:54
src/ycsb_wrapper PASS 00:10:25

@rsevilla87
Replace md5 by sha256
b479506 
Signed-off-by: Raul Sevilla <rsevilla@redhat.com>
@comet-perf-ci
Copy link
Collaborator

Results for SNAFU CI Test

Test Result Runtime
src/fio_wrapper PASS 00:14:30
src/fs_drift_wrapper PASS 00:09:23
src/hammerdb PASS 00:07:27
src/iperf PASS 00:07:22
src/pgbench_wrapper PASS 00:08:20
src/smallfile_wrapper FAIL 00:01:02
src/sysbench PASS 00:04:31
src/uperf_wrapper FAIL 00:01:03
src/vegeta_wrapper FAIL 00:00:42
src/ycsb_wrapper FAIL 00:00:33

@rsevilla87
Handle boolean conversion for parallel
ffd4a7f 
Signed-off-by: Raul Sevilla <rsevilla@redhat.com>
@comet-perf-ci
Copy link
Collaborator

Results for SNAFU CI Test

Test Result Runtime
src/fio_wrapper PASS 00:15:51
src/fs_drift_wrapper PASS 00:12:43
src/hammerdb PASS 00:09:48
src/iperf PASS 00:06:05
src/pgbench_wrapper PASS 00:08:23
src/smallfile_wrapper FAIL 00:09:42
src/sysbench PASS 00:04:28
src/uperf_wrapper PASS 00:37:35
src/vegeta_wrapper PASS 00:09:16
src/ycsb_wrapper PASS 00:08:43

@acalhounRH
Copy link
Contributor Author

Took a look at the smallfile CI test and its not obvious what exactly was the issue. Could anyone take a look? is this another dependency issue?

@rsevilla87
Copy link
Member

/rerun all

@comet-perf-ci
Copy link
Collaborator

Results for SNAFU CI Test

Test Result Runtime
src/fio_wrapper PASS 00:14:22
src/fs_drift_wrapper PASS 00:09:24
src/hammerdb PASS 00:08:01
src/iperf PASS 00:06:16
src/pgbench_wrapper PASS 00:05:36
src/smallfile_wrapper PASS 00:09:42
src/sysbench PASS 00:03:29
src/uperf_wrapper PASS 00:29:53
src/vegeta_wrapper PASS 00:09:10
src/ycsb_wrapper PASS 00:08:35

@rsevilla87 rsevilla87 self-requested a review August 14, 2020 11:48
rsevilla87
rsevilla87 approved these changes Aug 14, 2020
View reviewed changes
Copy link
Member

@rsevilla87 rsevilla87 left a comment

Choose a reason for hiding this comment

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

LGTM!

jtaleric
jtaleric approved these changes Aug 16, 2020
View reviewed changes
Copy link
Member

@jtaleric jtaleric left a comment

Choose a reason for hiding this comment

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

LGTM

dry923
dry923 approved these changes Aug 17, 2020
View reviewed changes
Copy link
Member

@dry923 dry923 left a comment

Choose a reason for hiding this comment

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

LGTM

@rsevilla87 rsevilla87 merged commit c382cd8 into cloud-bulldozer:master Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bengland2 bengland2 bengland2 requested changes

@aakarshg aakarshg aakarshg left review comments

@jtaleric jtaleric jtaleric approved these changes

@rsevilla87 rsevilla87 rsevilla87 approved these changes

@dry923 dry923 dry923 approved these changes

Assignees

@bengland2 bengland2

Labels
ok to test Kick off our CI framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@acalhounRH @aakarshg @rht-perf-ci @bengland2 @rsevilla87 @comet-perf-ci @dry923 @jtaleric