-
Notifications
You must be signed in to change notification settings - Fork 13
feat: add failure metrics to kinesis outbox event processing #59
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
Changes from 3 commits
9d08e7d
e554537
3724016
0d6e016
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| PATH | ||
| remote: . | ||
| specs: | ||
| journaled (6.2.2) | ||
| journaled (6.2.3) | ||
| activejob | ||
| activerecord | ||
| activesupport | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| PATH | ||
| remote: .. | ||
| specs: | ||
| journaled (6.2.2) | ||
| journaled (6.2.3) | ||
| activejob | ||
| activerecord | ||
| activesupport | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| PATH | ||
| remote: .. | ||
| specs: | ||
| journaled (6.2.2) | ||
| journaled (6.2.3) | ||
| activejob | ||
| activerecord | ||
| activesupport | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,83 +2,92 @@ | |||||
|
|
||||||
| module Journaled | ||||||
| module Outbox | ||||||
| # Handles metric emission for the Worker | ||||||
| # Handles metric emission for the Worker and Kinesis senders | ||||||
| # | ||||||
| # This class is responsible for collecting and emitting metrics about the outbox queue. | ||||||
| # This class provides utility methods for collecting and emitting metrics. | ||||||
| class MetricEmitter | ||||||
| def initialize(worker_id:) | ||||||
| @worker_id = worker_id | ||||||
| end | ||||||
| class << self | ||||||
| # Emit batch processing metrics | ||||||
| # | ||||||
| # @param stats [Hash] Processing statistics with :succeeded, :failed_permanently, :failed_transiently | ||||||
| # @param worker_id [String] ID of the worker processing the batch | ||||||
| def emit_batch_metrics(stats, worker_id:) | ||||||
| total_events = stats[:succeeded] + stats[:failed_permanently] + stats[:failed_transiently] | ||||||
|
|
||||||
| # Emit batch processing metrics | ||||||
| # | ||||||
| # @param stats [Hash] Processing statistics with :succeeded, :failed_permanently, :failed_transiently | ||||||
| def emit_batch_metrics(stats) | ||||||
| total_events = stats[:succeeded] + stats[:failed_permanently] + stats[:failed_transiently] | ||||||
| emit_metric('journaled.worker.batch_process', value: total_events, worker_id:) | ||||||
| emit_metric('journaled.worker.batch_sent', value: stats[:succeeded], worker_id:) | ||||||
| emit_metric('journaled.worker.batch_failed_permanently', value: stats[:failed_permanently], worker_id:) | ||||||
| emit_metric('journaled.worker.batch_failed_transiently', value: stats[:failed_transiently], worker_id:) | ||||||
| end | ||||||
|
|
||||||
| emit_metric('journaled.worker.batch_process', value: total_events) | ||||||
| emit_metric('journaled.worker.batch_sent', value: stats[:succeeded]) | ||||||
| emit_metric('journaled.worker.batch_failed_permanently', value: stats[:failed_permanently]) | ||||||
| emit_metric('journaled.worker.batch_failed_transiently', value: stats[:failed_transiently]) | ||||||
| end | ||||||
| # Collect and emit queue metrics | ||||||
| # | ||||||
| # This calculates various queue statistics and emits individual metrics for each. | ||||||
| # @param worker_id [String] ID of the worker collecting metrics | ||||||
| def emit_queue_metrics(worker_id:) | ||||||
| metrics = calculate_queue_metrics | ||||||
|
|
||||||
| # Collect and emit queue metrics | ||||||
| # | ||||||
| # This calculates various queue statistics and emits individual metrics for each. | ||||||
| def emit_queue_metrics | ||||||
| metrics = calculate_queue_metrics | ||||||
| emit_metric('journaled.worker.queue_total_count', value: metrics[:total_count], worker_id:) | ||||||
| emit_metric('journaled.worker.queue_workable_count', value: metrics[:workable_count], worker_id:) | ||||||
| emit_metric('journaled.worker.queue_erroring_count', value: metrics[:erroring_count], worker_id:) | ||||||
|
||||||
| emit_metric('journaled.worker.queue_erroring_count', value: metrics[:erroring_count], worker_id:) | |
| emit_metric('journaled.worker.queue_failed_count', value: metrics[:erroring_count], worker_id:) |
Should it be "failed count"? If it's a batch query it can really only count the number of hard-failed rows, not the number of transiently-erroring rows, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, updated!
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Journaled | ||
| VERSION = "6.2.2" | ||
| VERSION = "6.2.3" | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤷 nothing says we can't have more than 3
.in a stat name -- but mainly i renamedprocesstoprocessed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I'm wondering if we need
batch_at all. We're emitting stats in batches, but the stats themselves are counts of individual events:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also thoughts on
event.failed_permanentlyandevent.failed_transientlybecomingevent.failedandevent.errored? (Aligning with DJ terminology)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah all sounds good! will update