[exporter/elasticsearch] Emit _doc_count for metric documents in OTel mode when data point attribute _doc_count is true #35348
Conversation
| // documents is per-resource. Therefore, there is no need to hash resource attributes | ||
| hash := metricOTelHash(dp, scope.Attributes(), metric.Unit()) | ||
| hash := metricOTelHash(dp, scope.Attributes(), metric.Unit(), docCount) |
There was a problem hiding this comment.
Do we need docCount as a hashing key? Maybe we could add the doc counts for existing hashes and not consider docCount as a hashing key.
There was a problem hiding this comment.
Do we need docCount as a hashing key?
I think we'll need it. If 2 metrics with the same dimension have different doc counts, they cannot share the same document, as there is only 1 doc count in 1 document.
Maybe we could add the doc counts for existing hashes
Could you elaborate?
There was a problem hiding this comment.
If 2 metrics with the same dimension have different doc counts
Having a hard time imagining how this could practically happen. Doesn't this mean that we would drop the metric anyway since they are duplicates w.r.t dimensions?
Maybe we could add the doc counts for existing hashes
I was thinking that if all attributes are same then merging them would be the way to go (merging histograms + doc count values) but honestly, I can't imagine a case when this could happen in practical scenarios.
There was a problem hiding this comment.
Doesn't this mean that we would drop the metric anyway since they are duplicates w.r.t dimensions?
Good point, that appears to be the case for TSDB. @felixbarny did mention something about hashing doc count, not sure if this case was considered.
There was a problem hiding this comment.
Because of that issue, I think it would be easier to map metrics named _doc_count to the top-level _doc_count field. We don't need a _doc_count for all metrics, and it makes it harder to group as many metrics to the same document as possible, because these metrics may differ just by their _doc_count.
There was a problem hiding this comment.
With the new implementation that is based on the _doc_count: true attribute, I'm also wondering whether we can remove the doc count from the hash. If there are multiple doc counts for a given doc, I think it's fine to chose one of them (for example the first, last, or max value).
There was a problem hiding this comment.
Right, I forgot to make that change.
carsonip
changed the title
_doc_count for metric documents in OTel mode when _doc_count is true
carsonip
changed the title
_doc_count for metric documents in OTel mode when _doc_count is true_doc_count for metric documents in OTel mode conditionally
carsonip
changed the title
_doc_count for metric documents in OTel mode conditionally
Description:
Emit _doc_count for metric documents in OTel mode when data point attribute _doc_count is true
Link to tracking Issue:
Testing:
Documentation: