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

Report RequestIssue stats to DataDog #13770

Merged
merged 22 commits into from
Mar 31, 2020
Merged

Conversation

yoomlam
Copy link
Contributor

@yoomlam yoomlam commented Mar 20, 2020

Resolves #13769

Description

Provides initial capability to run a new job that reports RequestIssues stats to DataDog. This PR will be an example for future metrics, which can be added to this job (if it's related) or implemented as a new job.

A StatsCollectorJob has been implemented to simplify sending metrics to DataDog and to minimize the cost of running jobs as AWS Lambdas. StatsCollectorJob holds sets of Collectors that run daily, weekly, and monthly.

The StatsCollectorJob is scheduled to run in a Lambda in https://github.com/department-of-veterans-affairs/appeals-lambdas/pull/51.

Acceptance Criteria

  • A single StatsCollectorJob that runs daily, weekly, and monthly Collectors.

Testing Plan (after merge)

  1. Ensure no related errors appear in #appeals-job-alerts
  2. Check DataDog for reported stats
  3. Have the job run daily to make sure it's working.
  4. Once confirmed that things are working, change the RequestIssuesStatsCollector to run monthly.

@yoomlam yoomlam self-assigned this Mar 20, 2020
@yoomlam yoomlam added Priority: Low Reported issue, not a blocker. "Last in" priority for new work. Team: Echo 🐬 Type: Metrics or Reporting labels Mar 20, 2020
@yoomlam yoomlam changed the title Yoom/13642 request issue metrics Report RequestIssue stats to DataDog Mar 20, 2020
@codeclimate
Copy link

codeclimate bot commented Mar 20, 2020

Code Climate has analyzed commit 8285642 and detected 0 issues on this pull request.

View more on Code Climate.

@yoomlam
Copy link
Contributor Author

yoomlam commented Mar 21, 2020

For instructions see https://github.com/department-of-veterans-affairs/appeals-team/pull/1266/files

When adding a DataDog widget, select the metric in step 2:
image

Comment on lines 12 to 28
stats[METRIC_NAME_PREFIX] = req_issues.count

req_issues.group(:closed_status).count.each do |ben_type, count|
stats["#{METRIC_NAME_PREFIX}.st.#{ben_type || 'nil'}"] = count
end

req_issues.group(:benefit_type).count.each do |ben_type, count|
stats["#{METRIC_NAME_PREFIX}.ben.#{ben_type}"] = count
end

dr_counts_by_type = req_issues.group(:decision_review_type).count
dr_counts_by_type.each do |dr_type, count|
stats["#{METRIC_NAME_PREFIX}.dr.#{dr_type}"] = count
end

# Could use `req_issues.group(:veteran_participant_id).count.count` but there's a count discrepancy
stats["#{METRIC_NAME_PREFIX}.vet_count"] = req_issues.map(&:decision_review).map(&:veteran_file_number).uniq.count
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Populate stats hash with desire metrics.


# :reek:FeatureEnvy
def collect_stats
start_of_month = Time.zone.now.prev_month.beginning_of_month
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 collector reports on statistics about last month.

Comment on lines 35 to 38
RequestIssue.where.not(contention_reference_id: nil)
.where(is_unidentified: true)
.where("created_at >= ?", start_date)
.where("created_at < ?", end_date)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All Rails queries have been tested in the production environment.

Comment on lines +52 to +60
def emit(name, value, attrs: {})
DataDogService.emit_gauge(
metric_group: METRIC_GROUP_NAME,
metric_name: name,
metric_value: value,
app_name: APP_NAME,
attrs: attrs
)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note the attributes passed to DataDog:

  • app_name: "caseflow_job" - mapped to a app:caseflow_job tag in DataDog
  • metric_group: YOUR_JOB_NAME_job
  • metric_name: YOUR_METRIC_NAME - can have multiple metric_names per job
  • metric_value: YOUR_METRIC_VALUE - value to log in DataDog
  • attrs: { YOUR_ATTR_KEY: YOUR_ATTR_VALUE } - each hash entry is mapped to a YOUR_ATTR_KEY:YOUR_ATTR_VALUE tag in DataDog for selecting subsets of your metric or sets of different metrics

The metric label in DataDog will be dsva_appeals.YOUR_JOB_NAME_job.YOUR_METRIC_NAME as defined in DataDogService.get_stat_name

Comment on lines 17 to 23
DAILY_COLLECTORS = {
"unidentified_request_issues_with_contention" => Collectors::RequestIssuesStatsCollector
}.freeze
WEEKLY_COLLECTORS = {
}.freeze
MONTHLY_COLLECTORS = {
}.freeze
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add new collectors here.


metric_name_prefix = described_class::METRIC_NAME_PREFIX
expect(stats[metric_name_prefix]).to eq(9)
expect(stats["#{metric_name_prefix}.vet_count"]).to be_between(1, 9)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There should be at least 1 unique veteran but no more than 9 (total number relevant request issues).

Copy link
Contributor

@pkarman pkarman left a comment

Choose a reason for hiding this comment

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

I generally like this approach of a single job where we can register various collectors. It's similar to data integrity checker in that respect.

I would move the collector classes outside the app/jobs space since they are not jobs, but POROs.

app/jobs/collectors/request_issues_stats_collector.rb Outdated Show resolved Hide resolved
@yoomlam yoomlam added Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master and removed Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master labels Mar 26, 2020
app/services/collectors/request_issues_stats_collector.rb Outdated Show resolved Hide resolved
app/jobs/stats_collector_job.rb Show resolved Hide resolved
spec/jobs/stats_collector_job_spec.rb Show resolved Hide resolved
app/jobs/stats_collector_job.rb Show resolved Hide resolved
app/jobs/stats_collector_job.rb Outdated Show resolved Hide resolved
spec/jobs/stats_collector_job_spec.rb Show resolved Hide resolved
Copy link
Contributor

@lomky lomky left a comment

Choose a reason for hiding this comment

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

Awesome code! GTG! :shipit:

@yoomlam yoomlam added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Mar 31, 2020
@va-bot va-bot merged commit 8b68462 into master Mar 31, 2020
@va-bot va-bot deleted the yoom/13642-request_issue_metrics branch March 31, 2020 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Reported issue, not a blocker. "Last in" priority for new work. Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master Team: Echo 🐬 Type: Metrics or Reporting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report RequestIssues stats to DataDog
4 participants