Skip to content

Unify PerformanceTest metric dashboard naming and series - #23914

Merged
pabloem merged 1 commit into
apache:masterfrom
Abacn:grafanadesc
Nov 2, 2022
Merged

Unify PerformanceTest metric dashboard naming and series#23914
pabloem merged 1 commit into
apache:masterfrom
Abacn:grafanadesc

Conversation

@Abacn

@AbacnAbacn commented Nov 1, 2022

Copy link
Copy Markdown
Contributor

Part of #18204

  • Tests are named after 'IO Connector | Specifications | data size'.

  • Python performance tests plot read and write time in same plot as in Java

Please add a meaningful description for your change here


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI.

@Abacn
Abacn marked this pull request as ready for review November 1, 2022 15:26
@Abacn

Abacn commented Nov 1, 2022

Copy link
Copy Markdown
ContributorAuthor

R: @pabloem

Is there a way to show the preview? Previewed locally but without actual data because had no access to the database.

@github-actions

Copy link
Copy Markdown
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

@AbacnAbacn changed the title Unify PerformanceTest metric dashboard namingUnify PerformanceTest metric dashboard naming and seriesNov 1, 2022
@Abacn

Abacn commented Nov 1, 2022

Copy link
Copy Markdown
ContributorAuthor

Have setup a dumb influxdb database and verified the rendering:
image

@Abacn

Abacn commented Nov 1, 2022

Copy link
Copy Markdown
ContributorAuthor

Java IO dashboard also checked. Ready for review now.

@pabloem

Copy link
Copy Markdown
Member

I am not familiar with this beyohnd the normal docs here: https://cwiki.apache.org/confluence/display/BEAM/Community+Metrics

@pabloem

Copy link
Copy Markdown
Member

are you sure we won't need the configuration sections that you removed? are these autopopulated?

@Abacn

Abacn commented Nov 1, 2022

Copy link
Copy Markdown
ContributorAuthor

Yeah, thanks. I followed this documentation and get it working except I am not using the real data from influxDB. The screenshot above shows the results of some manipulated data.

The removed sections are combining read and write time in same plot as in Java. Currently they are plotted separately: http://104.154.241.245/d/gP7vMPqZz/python-io-it-tests-dataflow?orgId=1
This has merged the 6 plots into 3 plots.

I am working on adding more Python performance tests and this is a clean up for current frontend as a preclude.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This figure plots count instead of time. Fixed here.

@Abacn

Abacn commented Nov 1, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks @pabloem . Replied above. PTAL

@Abacn

Abacn commented Nov 2, 2022

Copy link
Copy Markdown
ContributorAuthor

SparkReceiverIO Performance test has been merged. Rebasing and also changing the naming of it.

@Abacn

Abacn commented Nov 2, 2022

Copy link
Copy Markdown
ContributorAuthor

R: @aromanenko-dev

@pabloem

Copy link
Copy Markdown
Member

thanks @Abacn - this LGTM : )

@pabloem
pabloem merged commit 37dcff8 into apache:masterNov 2, 2022
@AbacnAbacn mentioned this pull request Nov 7, 2022
4 tasks
@Abacn
Abacn deleted the grafanadesc branch November 7, 2022 18:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Abacn@pabloem