Update stats collectors to use tagged metrics #13895
Conversation
yoomlam
added
Type: Metrics or Reporting
Team: Echo 🐬
Priority: Low
|
Code Climate has analyzed commit 6976df6 and detected 0 issues on this pull request. View more on Code Climate. |
app/jobs/stats_collector_job.rb
Outdated
| return emit(hash[:metric], hash[:value], tags: hash.except(:metric, :value)) if hash[:metric] && hash[:value] | ||
|
|
||
| # when hash is { "metric_name" => 123 } | ||
| return emit(hash.first[0], hash.first[1]) if hash.size == 1 |
There was a problem hiding this comment.
Handle metric without tag.
| [].tap do |stats| | ||
| stats.concat flatten_stats(TOTALS_METRIC_NAME_PREFIX, total_counts_hash) | ||
| stats.concat flatten_stats(INCREMENTS_METRIC_NAME_PREFIX, daily_counts) |
There was a problem hiding this comment.
Return array since metric names are no longer unique when using tags.
| METRIC_NAME_PREFIX = "request_issues.unidentified_with_contention" | ||
| include Collectors::StatsCollector | ||
|
|
||
| METRIC_NAME_PREFIX = "req_issues.w_contentions" |
There was a problem hiding this comment.
Shorten name metric name prefix for readability in DataDog.
| class ExampleTaggingCollector | ||
| def collect_stats | ||
| [ | ||
| { "ex_tagged_metric" => 9 }, | ||
| { metric: "ex_tagged_metric.status", value: 3, "status": "open" }, | ||
| { metric: "ex_tagged_metric.status", value: 5, "status": "closed" } | ||
| ] | ||
| end | ||
| end |
There was a problem hiding this comment.
Test with collector that uses the status tag.
| protected | ||
|
|
||
| def add_tags_to_group_counts(prefix, metric_name, group_counts) | ||
| tag_key = to_valid_tag_key(metric_name.split(".").last) |
There was a problem hiding this comment.
Use that last part of the metric_name as the tag_key.
Example: If metric_name="hearing.disposition", then tag_key="disposition"
There was a problem hiding this comment.
I suggest passing in metric_name and doing the split inside of the to_valid_tag_key, that way you have all of the context of creating a valid tag key encapsulated in the function, instead of split up
There was a problem hiding this comment.
I considered this. The reason I did not encapsulate the split call into to_valid_tag_key is in case other collectors want to use it. I don't want to be doing too much manipulation to the name input argument. I could be convinced otherwise, but I'm leaning towards keeping it as it.
| def group_key_to_name(key) | ||
| return "nil" unless key | ||
|
|
||
| return key.map(&:underscore).map(&:parameterize).join(".") if key.instance_of?(Array) |
There was a problem hiding this comment.
Example: when key=["EstablishClaim", "started"], return "establish_claim.started"
|
|
||
| return key.underscore.parameterize if key.instance_of?(String) | ||
|
|
||
| key.to_s.underscore.parameterize |
There was a problem hiding this comment.
For numbers and booleans.
|
|
||
| return key.map(&:underscore).map(&:parameterize).join(".") if key.instance_of?(Array) | ||
|
|
||
| return key.underscore.parameterize if key.instance_of?(String) |
There was a problem hiding this comment.
| protected | ||
|
|
||
| def add_tags_to_group_counts(prefix, metric_name, group_counts) | ||
| tag_key = to_valid_tag_key(metric_name.split(".").last) |
There was a problem hiding this comment.
I suggest passing in metric_name and doing the split inside of the to_valid_tag_key, that way you have all of the context of creating a valid tag key encapsulated in the function, instead of split up
yoomlam
added
the
Ready-to-Merge
|
|
Connect #13769
Description
In order to easily chart related metrics, DataDog recommends using
tagswhen submitting metrics.Also add validation for metric and tag names.
https://docs.datadoghq.com/tagging/
Acceptance Criteria
Details
See:
In DataDog, click on a metric in Metrics Summary to see tags for a metric.