Skip to content

[BEAM-5984] Enable publishing load test results to Big Query#7090

Merged
lgajowy merged 3 commits intoapache:masterfrom
lgajowy:engage-bq
Dec 3, 2018
Merged

[BEAM-5984] Enable publishing load test results to Big Query#7090
lgajowy merged 3 commits intoapache:masterfrom
lgajowy:engage-bq

Conversation

@lgajowy
Copy link
Contributor

@lgajowy lgajowy commented Nov 20, 2018

This PR provides a way to publish metrics to BQ after specifying appropriate pipeline options.


Follow this checklist to help us incorporate your contribution quickly and easily:

  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

@lgajowy
Copy link
Contributor Author

lgajowy commented Nov 20, 2018

@pabloem or @aaltay could you take a look? Thanks!

@aaltay aaltay requested a review from pabloem November 21, 2018 00:33
Copy link
Member

@pabloem pabloem left a comment

Choose a reason for hiding this comment

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

Thanks! I added a couple Qs

.build();

bigQueryClient.insertRow(record, tableName);
bigQueryClient.insertRow(record, schema, tableName);
Copy link
Member

Choose a reason for hiding this comment

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

Should this test be changed to use the ResultPublisher? I see that the schemas are a tad different? runtimeSec vs runtime, etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually a good question. I refactored a little bit so that the BigQuery publisher part could be reused. Please let me know what do you think of it.

@lgajowy
Copy link
Contributor Author

lgajowy commented Nov 21, 2018

@pabloem I refactored a little bit more so that more code could be reused (please see the new commit description)

What is more, current implementation allows us to easily make code responsible for publishing to console common too. I didn't want to do this now because the change would be huge.

Could you take a look again?

@lgajowy
Copy link
Contributor Author

lgajowy commented Nov 21, 2018

Run Direct Runner Nexmark Tests

@lgajowy
Copy link
Contributor Author

lgajowy commented Nov 21, 2018

Run Spark Runner Nexmark Tests

@lgajowy
Copy link
Contributor Author

lgajowy commented Nov 29, 2018

Run Java PreCommit

@lgajowy
Copy link
Contributor Author

lgajowy commented Nov 29, 2018

@pabloem pinging this kindly :)

@pabloem
Copy link
Member

pabloem commented Nov 29, 2018

Sorry about the long delay : / - Anyway, thanks, this looks great : )

@lgajowy
Copy link
Contributor Author

lgajowy commented Dec 3, 2018

Thanks, @pabloem! I rebased and will wait for the results to be green before merging.

- Console and BigQuery publishers are now separated.
- Additional tests were added for BigQueryPublisher
- LoadTestResult POJO was created for result encapsulation
- Common interface for NexmarkPerf and LoadTestResult was introduced
@lgajowy
Copy link
Contributor Author

lgajowy commented Dec 3, 2018

The results are green - merging.

@lgajowy lgajowy merged commit 90ef4eb into apache:master Dec 3, 2018
@lgajowy lgajowy deleted the engage-bq branch December 3, 2018 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants