add metric object type #837
Conversation
andrewqian2001datadog
added
the
changelog/Added
datadog/dogstatsd/metrics.py
Outdated
| super(SetMetric, self).__init__(name, tags, rate) | ||
| self.data = set() | ||
| self.data.add(value) | ||
| self.lock = Lock() |
There was a problem hiding this comment.
Similarly - maybe we should wait to add synchronization primitives until we actually have multithreaded code running. We can keep this PR focused to just introducing the metric classes and their associated properties.
There was a problem hiding this comment.
the serialization function needs values like namespace and container_id which is set in the base.py class. I think it would be more convenient to leave it as a function in base.py or move it to aggregator.py? Not sure.
This is what aggregator.py looks like so far
Maybe we can move the serialization function there since it should have access to the DogStatsd client (it does in the existing go code) and its properties? It seems more intuitive that aggregator.py would handle serialization of a metric.
|
/merge |
|
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
dd-devflow
bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
|
/remove |
|
🚂 Devflow: |
dd-devflow
bot
added
mergequeue-status: removed
and removed
mergequeue-status: in_progress
labels
|
/merge |
|
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
|
/merge |
|
🚂 MergeQueue: waiting for PR to be ready This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals. Use |
dd-devflow
bot
added
mergequeue-status: waiting
mergequeue-status: queued
and removed
mergequeue-status: rejected
mergequeue-status: waiting
labels
|
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
dd-devflow
bot
added
mergequeue-status: in_progress
mergequeue-status: rejected
and removed
mergequeue-status: queued
mergequeue-status: in_progress
labels
What does this PR do?
This PR is for adding metric objects that will make aggregation easier by allowing us to have one Metric object instantiation for each unique metric context that we can add samples to which will make aggregation easier. Existing behavior already exists in the client side aggregation for GO
Description of the Change
Create metric object type (
MetricAgggregator) that other metric objects (CountMetric,GaugeMetric, etc.) implement that will enforce the objects to implement functionsaggregateand potentiallyflush_unsafe. This should also allow us to group the different metric types together in one list to be sent when we flush the metrics.Possible Drawbacks
Some future metrics may not have the necessary fields to be of type
MetricAggregatorThe existing code for flushing metrics does NOT use object oriented programming, as a result they have duplicate code for each metric type.
Maybe i'm missing the potential drawbacks to using object oriented programming (specifically inheritance)?
Update:
Verification Process
Unit tests that verify the new metric objects match the existing behavior in the client side aggregator for Go.
Additional Notes
The python dogstatsd API will need to be refactored in the future to use these metric objects.
Release Notes
N/A, no new behavior until the aggregator class is implemented
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.