[exporter/exporterhelper] Add DroppedItemsErr to report intentionally dropped items#15028
[exporter/exporterhelper] Add DroppedItemsErr to report intentionally dropped items#15028Krishnachaitanyakc wants to merge 1 commit into
Conversation
Krishnachaitanyakc
requested review from
a team,
bogdandrutu and
dmitryax
as code owners
Krishnachaitanyakc
force-pushed
the
fix/issue-13643
branch
from
add5acf to
98359df
Compare
|
@Krishnachaitanyakc Why did I get pinged on this? I'm not involved with this project. |
my bad, i was tagging you on a different PR and got it mixed up, apologies |
|
@bogdandrutu @dmitryax could you take a look at this change when you have a chance? |
Krishnachaitanyakc
force-pushed
the
fix/issue-13643
branch
from
bef083f to
5695c65
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15028 +/- ##
==========================================
+ Coverage 91.24% 91.27% +0.02%
==========================================
Files 705 706 +1
Lines 46084 46217 +133
==========================================
+ Hits 42050 42183 +133
Misses 2826 2826
Partials 1208 1208 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Krishnachaitanyakc
force-pushed
the
fix/issue-13643
branch
from
5695c65 to
0d73dcb
Compare
Merging this PR will not alter performance
|
|
We need #15280 to land first |
… dropped items
Exporters that intentionally discard items (e.g. the Prometheus exporter
dropping non-monotonic DELTA sums) previously had no way to signal this to
the exporterhelper observation layer. Those items were silently counted as
successfully sent, making internal telemetry misleading.
This change adds:
- exporterhelper.DroppedItemsErr / NewDroppedItemsErr: a public, non-failure
sentinel that an exporter's push function can return to report a number of
intentionally dropped items and an optional reason. The type is re-exported
from the internal experr package so external exporters can construct it.
- Four new telemetry counters
(otelcol_exporter_dropped_{spans,metric_points,log_records,profile_samples})
emitted by the obsReportSender whenever a DroppedItemsErr is received.
- At detailed telemetry level, an exporter.dropped.reason metric attribute is
attached to the four counters. A metric view in
service/internal/metricviews filters the attribute out at non-detailed
levels, mirroring the existing handling of error.type / error.permanent on
exporter_send_failed_*, to bound cardinality.
- An items.dropped span attribute is added alongside items.sent / items.failed
on the exporter span.
- A warning is logged when an exporter reports more dropped items than the
request actually contained; the sent counter is clamped to zero in that
case.
- obsReportSender adjusts the sent count so that dropped items are excluded
from "sent" and the DroppedItemsErr is not propagated as a pipeline error.
An end-to-end test exercises the full public API (exporterhelper.NewMetrics +
NewDroppedItemsErr) and asserts the emitted telemetry contract.
Resolves #13643
Signed-off-by: Kc Balusu <kcbalusu@meta.com>
Krishnachaitanyakc
force-pushed
the
fix/issue-13643
branch
from
0d73dcb to
ca51457
Compare
bogdan-st
left a comment
bogdan-st
left a comment
There was a problem hiding this comment.
Id also say that it s a bit confusing for someone reading the code to see numSent as the total number of items instead of the ones actually sent. But I think that s out of scope for now.
| // successfully sent. Clamp to zero in case the exporter reported more | ||
| // dropped items than the total item count, and warn — that condition | ||
| // indicates an exporter bug. | ||
| if numDropped > 0 && numSent <= numDropped { |
There was a problem hiding this comment.
This is true even when all items have been dropped correctly and that should not produce a warning. The else if block handles that correctly without the warning.
| if numDropped > 0 && numSent <= numDropped { | |
| if numDropped > 0 && numSent < numDropped { |
|
Let's have #15280 merged first so we're aligned on the long term plan |
4 participants
Description
Exporters that intentionally discard items (e.g. a Prometheus exporter dropping non-monotonic DELTA sums) previously had no way to signal this to the
exporterhelperobservation layer. Dropped items were silently counted as successfully sent viaotelcol_exporter_sent_metric_points, making internal telemetry misleading and difficult to use for diagnosing data loss.This PR adds a lightweight mechanism for exporters to report intentionally dropped items:
experr.DroppedItemsErr/NewDroppedItemsErr(count, reason): a non-failure sentinel that an exporter's push function can return to report how many items were intentionally dropped and why.otelcol_exporter_dropped_{spans,metric_points,log_records,profile_samples}): emitted byobsReportSenderwhen aDroppedItemsErris received.obsReportSenderadjustments: dropped items are subtracted from the "sent" count, andDroppedItemsErris not propagated as a pipeline error (the pipeline seesnil).A follow-up to the
prometheusexporterinopentelemetry-collector-contribwill be needed to actually returnDroppedItemsErrfor unsupported metric types (e.g. non-monotonic DELTA sums).Resolves open-telemetry/opentelemetry-collector-contrib#48172