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

[metrics] getting a more accurate processed record count in rp handler #548

Merged
merged 2 commits into from
Jan 3, 2018
Merged

[metrics] getting a more accurate processed record count in rp handler #548

merged 2 commits into from
Jan 3, 2018

Conversation

ryandeivert
Copy link
Contributor

to: @jacknagz
cc: @airbnb/streamalert-maintainers
size: small
resolves #527

Background

See #527. Metrics should track the records for the processed payload, not the incoming 'records' array.

Changes

  • Using a instance property to track the total records from each processed payload.
  • This value is then logged at the end of processing as the total records metric.

Testing

  • Adding unit test to check for tracking the proper processed log count.

@ryandeivert ryandeivert force-pushed the ryandeivert-more-accurate-record-count-metric branch from b86c8a5 to 59fe3ff Compare January 3, 2018 22:01
@ghost
Copy link

ghost commented Jan 3, 2018

image

jacknagz
jacknagz approved these changes Jan 3, 2018
View reviewed changes
Copy link
Contributor

@jacknagz jacknagz left a comment

Choose a reason for hiding this comment

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

💯

@ryandeivert ryandeivert force-pushed the ryandeivert-more-accurate-record-count-metric branch from 59fe3ff to b5fe094 Compare January 3, 2018 22:30
@airbnb airbnb deleted a comment from coveralls Jan 3, 2018
@airbnb airbnb deleted a comment from coveralls Jan 3, 2018
@ryandeivert ryandeivert added this to the 1.6.0 milestone Jan 3, 2018
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 95.213% when pulling b5fe094 on ryandeivert-more-accurate-record-count-metric into e0f147e on master.

@ryandeivert ryandeivert merged commit 53d4207 into master Jan 3, 2018
@ryandeivert ryandeivert deleted the ryandeivert-more-accurate-record-count-metric branch January 10, 2018 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacknagz jacknagz jacknagz approved these changes

Assignees
No one assigned
Labels
improvement metrics
Projects
None yet
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.

Metrics: TotalRecords Improvement
3 participants
@ryandeivert @coveralls @jacknagz