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 rds metricset into aws module #11620

Merged
merged 24 commits into from
Jun 27, 2019
Merged

Add rds metricset into aws module #11620

merged 24 commits into from
Jun 27, 2019

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Apr 3, 2019 •
edited
Loading

This is the PR to implement #10054 to add rds metricset into aws module. rds metricset queries AWS DescribeDBInstances to get a list of RDS for each region and loop through each RDS to query Cloudwatch for monitoring metrics.

Also added an overview dashboard for RDS. It shows basic latency, throughput and queries information as well as if there are any blocked transactions and whats the number of database connections in use.

closes: #10054

@kaiyan-sheng kaiyan-sheng requested a review from a team as a code owner April 3, 2019 04:34
@kaiyan-sheng kaiyan-sheng self-assigned this Apr 3, 2019
@kaiyan-sheng kaiyan-sheng added Team:Integrations Label for the Integrations team in progress Pull request is currently in progress. labels Apr 3, 2019
exekias
exekias suggested changes Apr 4, 2019
View reviewed changes
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

This is looking good!, I left a few comments. Will try to test it today

x-pack/metricbeat/module/aws/rds/rds.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/rds/rds.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/rds/rds.go Outdated Show resolved Hide resolved
@exekias
Copy link
Contributor

exekias commented Apr 4, 2019

I'm missing some unit testing here, any chance you can add some?

@kaiyan-sheng kaiyan-sheng requested a review from a team as a code owner April 4, 2019 21:20
houndci-bot
houndci-bot reviewed Apr 5, 2019
View reviewed changes
x-pack/metricbeat/module/aws/rds/rds.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/rds/rds.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/rds/rds.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/rds/rds.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/rds/rds.go Show resolved Hide resolved
@kaiyan-sheng
Copy link
Contributor Author

@exekias Thanks for your review! This PR is only ready for some initial testing 😄 not fully done yet. Also some unit tests need to be added in the future and a dashboard. But comments/testing is always welcome for sure!

@exekias
Copy link
Contributor

exekias commented Apr 5, 2019

ups sorry, I didn't notice the in progress flag 🤦‍♂️

ruflin
ruflin reviewed Apr 8, 2019
View reviewed changes
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@kaiyan-sheng
Copy link
Contributor Author

kaiyan-sheng commented Apr 23, 2019 •
edited
Loading

This can wait till #11882 is merged.

@kaiyan-sheng kaiyan-sheng added review [zube]: In Review and removed [zube]: In Progress in progress Pull request is currently in progress. labels Apr 25, 2019
@kaiyan-sheng kaiyan-sheng added in progress Pull request is currently in progress. [zube]: In Review review and removed [zube]: In Review review in progress Pull request is currently in progress. labels Jun 14, 2019
jsoriano
jsoriano reviewed Jun 21, 2019
View reviewed changes
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

I haven't looked a lot around the code, only some comments about the dashboard. It looks great, but I'd show the number of connections and blocked transactions also as histograms, so you can see the evolution along the time period.

x-pack/metricbeat/module/aws/_meta/kibana/7/dashboard/Metricbeat-aws-rds-overview.json Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/_meta/kibana/7/dashboard/Metricbeat-aws-rds-overview.json Outdated
}
}
},
"title": "AWS RDS Select Throughput",
Copy link
Member

Choose a reason for hiding this comment

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

Is queries per second the unit of these throughputs? We could probably consider Queries Per Second visualization also throughput, but they have inconsistent titles. If all them are throughput in queries per second, could we use consistent naming like Throughput in queries per second, Select throughput in queries per second...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the unit is Count/Second which is the same as Queries per Second :-) But I just realized this Queries metric is only for Aurora-MySQL. I will remove this from the overview dashboard since it is not a general metric for all types of databases.

@kaiyan-sheng
Copy link
Contributor Author

kaiyan-sheng commented Jun 24, 2019 •
edited
Loading

I removed the metricbeat-aws-rds-overview.png because it's very boring with what I have in the test aws account for RDS. I will create and add the overview screenshot for RDS once I get some better data points. @exekias and @jsoriano Is that OK for you?

@kaiyan-sheng kaiyan-sheng added [zube]: In Review review and removed [zube]: In Progress in progress Pull request is currently in progress. labels Jun 26, 2019
@exekias exekias added the v7.3.0 label Jun 27, 2019
@exekias
Copy link
Contributor

exekias commented Jun 27, 2019

I haven't manually tested this but it LGTM

@kaiyan-sheng
Copy link
Contributor Author

I haven't manually tested this but it LGTM

I tested it plenty times 😬after all the rebasing haha

@kaiyan-sheng kaiyan-sheng merged commit 3798e85 into elastic:master Jun 27, 2019
@kaiyan-sheng kaiyan-sheng deleted the add_rds_metricset branch June 27, 2019 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@ruflin ruflin ruflin left review comments

@jsoriano jsoriano jsoriano left review comments

@exekias exekias exekias approved these changes

Assignees

@kaiyan-sheng kaiyan-sheng

Labels
review Team:Integrations Label for the Integrations team v7.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Metricbeat] Add AWS RDS Metricset
6 participants
@kaiyan-sheng @exekias @jsoriano @ruflin @houndci-bot @andresrc