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.

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

Add s3_request metricset for AWS module #10949

Merged
merged 7 commits into from
Feb 28, 2019
Merged

Add s3_request metricset for AWS module #10949

merged 7 commits into from
Feb 28, 2019

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Feb 26, 2019

This PR is to add s3_request metricset to AWS module.
Metrics:
bucket.name
requests.total
requests.get
requests.put
requests.delete
requests.head
requests.post
requests.select
requests.select.returned.bytes
requests.select.scanned.bytes
list_requests
list_requests.bytes
downloaded.bytes
uploaded.bytes
errors.4xx
errors.5xx
latency.first_byte.ms
latency.total_request.ms

These metrics came from Amazon S3 CloudWatch Request Metrics for Buckets. Request metrics are available at 1-minute intervals after some latency to process in Cloudwatch.

#10055

@kaiyan-sheng kaiyan-sheng requested review from a team as code owners February 26, 2019 21:07
@kaiyan-sheng kaiyan-sheng self-assigned this Feb 26, 2019
@kaiyan-sheng kaiyan-sheng added Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Feb 26, 2019
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Looks good! I don't see anything blocking. Left some comments about field naming but they aren't mandatory (I know that it will require quite some work to change them)

type: keyword
description: >
Name of a S3 bucket.
- name: all_requests
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe create a field group request or nothing at all and then put inside get, put... so you don't have a fields like aws.s3_request.all_requests and, instead, you have aws.s3_request.requests.all or even aws.s3_request.all, aws.s3_request.get

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that aws.s3_request.total instead of aws.s3_request.all is more widely used in Metricbeat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!! requests.total it is!!

type: long
description: >
The number of HTTP POST requests made to an Amazon S3 bucket.
- name: select_requests
Copy link
Contributor

Choose a reason for hiding this comment

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

Here maybe a aws.s3_request.select.total and aws.s3_request.select.scanned.bytes, aws.s3_request.select.returned.bytes

type: long
description: >
The number of HTTP requests that list the contents of a bucket.
- name: bytes_downloaded
Copy link
Contributor

Choose a reason for hiding this comment

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

aws.s3_request.downloaded.bytes I think I read somewhere that the unit is always at the end of the field name

type: long
description: >
The number of HTTP 4xx client error status code requests made to an Amazon S3 bucket with a value of either 0 or 1.
- name: 5xx_errors
Copy link
Contributor

Choose a reason for hiding this comment

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

aws.s3_requests.errors.4xx and aws.s3_requests.errors.5xx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sayden done 😄 thanks for all the great suggestions!!

type: long
description: >
The per-request time from the complete request being received by an Amazon S3 bucket to when the response starts to be returned.
- name: total_request_latency
Copy link
Contributor

Choose a reason for hiding this comment

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

aws.s3_requests.latency.elapsed.sec (if it's in seconds)

@kaiyan-sheng
Copy link
Contributor Author

jenkins, test this please

@kaiyan-sheng
Copy link
Contributor Author

jenkins, test this please

@kaiyan-sheng kaiyan-sheng merged commit e03f966 into elastic:master Feb 28, 2019
@kaiyan-sheng kaiyan-sheng deleted the s3_request branch February 28, 2019 13:25
@cwurm
Copy link
Contributor

cwurm commented Mar 1, 2019

requests.select
requests.select.returned.bytes
requests.select.scanned.bytes

@kaiyan-sheng I just noticed the Metricbeat template import to Elasticsearch is failing because requests.select as a long conflicts with the other two. They should be distinct fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Metricbeat Metricbeat Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants