RUMM-3460 feat: Add "Batch Closed" telemetry #1386
Conversation
ncreated
changed the title
Datadog ReportBranch report: ✅ |
| @@ -92,3 +93,23 @@ internal enum BatchDeletedMetric { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| /// Definition of "Batch Closed" telemetry. | |||
| internal enum BatchClosedMetric { | |||
There was a problem hiding this comment.
non blocking, but would have preferred this as struct with method toAttributes() -> expected type which makes it easier handle any conversion.
There was a problem hiding this comment.
I have been thinking of it, but I decided to keep it bare simple, as we introduce a new concept here and not yet sure which direction it will go. Moving from "static constants" model to a "type" won't cost much in the future.
For instance, one idea I had was to define Metric protocol that enforces common fields: name, metric_type and attributes. That could be passed directly to telemetry.metric(_:) in a type-safer way, instead of broad telemetry.metric(name:attributes:) introduced by this PR.
With few options in hand and no foresight future, I bet on simplicity for now, so will keep it as it is.
What and why?
📦⏱️ Adding "Batch Closed" metric.
See (internal) RFC Batching & Upload Observability.
How?
Leveraged metrics API added earlier in #1384.
With this PR, I'm adding full unit tests coverage for both metrics creation in
FilesOrchestrator.Review checklist
Custom CI job configuration (optional)