-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[exporter/elasticsearch] Workaround TSDB array dimension limitation for metrics OTel mode #35009
[exporter/elasticsearch] Workaround TSDB array dimension limitation for metrics OTel mode #35009
Conversation
document.AddAttributes("attributes", attributeMap) | ||
} | ||
|
||
func mapStringifyArrayValues(m pcommon.Map) { | ||
m.Range(func(k string, v pcommon.Value) bool { | ||
if v.Type() == pcommon.ValueTypeSlice { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This not only affects array values but also other complex attribute values, like ValueTypeMap
. Not quite sure how to deal with ValueTypeBytes
tbh. How do we serialize it normally? Is that compatible with a field type that supports dimensions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to https://opentelemetry.io/docs/specs/otel/common/
The attribute value is either:
A primitive type: string, boolean, double precision floating point (IEEE 754-1985) or signed 64 bit integer.
An array of primitive type values. The array MUST be homogeneous, i.e., it MUST NOT contain values of different types.
I don't think ValueTypeMap and ValueTypeBytes fall into these valid value types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are we dealing with cases where we get invalid attribute values? Are we ignoring them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had a similar issue in apm-server where otlp events with non-compliant attributes cause panics: elastic/apm-server#13703
In es exporter, by looking at the code, ValueTypeMap will be successfully encoded as objects, ValueTypeBytes will be ignored, non-homogeneous arrays will be successfully encoded. Ideally they should be dropped explicitly with a warning, but as these are non-compliant, user should expect undefined behavior, which includes successful or unsuccessful indexing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as we don't serialize non-compliant values and there's no panic, this sounds fine. Ideally, we would increment the dropped attributes count.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
…or metrics OTel mode (open-telemetry#35009) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Elasticsearch TSDB does not support array dimensions. Workaround it by stringifying attribute array values in OTel mapping mode for metrics. Refactor to improve test code reuse. **Link to tracking Issue:** <Issue number if applicable> Fixes open-telemetry#35004 **Testing:** <Describe what testing was performed and which tests were added.> Added exporter test for otel mode logs, metrics and traces **Documentation:** <Describe the documentation added.>
…etrics OTel mode (#35291) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Remove the workaround in #35009 to stringify array dimensions as the limitation has been lifted in Elasticsearch in elastic/elasticsearch#110387 This change is not breaking as array will be coerced as string, meaning that there will be no indexing error. Additionally, the changes are acceptable as OTel mapping mode explicitly marked as in development. **Link to tracking Issue:** <Issue number if applicable> **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:** <Describe the documentation added.> --------- Co-authored-by: Vishal Raj <vishal.raj@elastic.co>
…or metrics OTel mode (open-telemetry#35009) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Elasticsearch TSDB does not support array dimensions. Workaround it by stringifying attribute array values in OTel mapping mode for metrics. Refactor to improve test code reuse. **Link to tracking Issue:** <Issue number if applicable> Fixes open-telemetry#35004 **Testing:** <Describe what testing was performed and which tests were added.> Added exporter test for otel mode logs, metrics and traces **Documentation:** <Describe the documentation added.>
…etrics OTel mode (open-telemetry#35291) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Remove the workaround in open-telemetry#35009 to stringify array dimensions as the limitation has been lifted in Elasticsearch in elastic/elasticsearch#110387 This change is not breaking as array will be coerced as string, meaning that there will be no indexing error. Additionally, the changes are acceptable as OTel mapping mode explicitly marked as in development. **Link to tracking Issue:** <Issue number if applicable> **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:** <Describe the documentation added.> --------- Co-authored-by: Vishal Raj <vishal.raj@elastic.co>
Description:
Elasticsearch TSDB does not support array dimensions. Workaround it by stringifying attribute array values in OTel mapping mode for metrics.
Refactor to improve test code reuse.
Link to tracking Issue:
Fixes #35004
Testing:
Added exporter test for otel mode logs, metrics and traces
Documentation: