-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[exporter/elasticsearch] Dynamically route documents by default #38500
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
Conversation
Telemetry data will be written to signal specific data streams by default: | ||
logs to `logs-generic-default`, metrics to `metrics-generic-default`, and traces to `traces-generic-default`. | ||
Documents are routed to the target index / data stream dynamically in the following order. The first routing mode that applies will be used. | ||
1. "Static mode": To `logs_index` (for log records), `metrics_index` (for data points) and `traces_index` (for spans and span events) if they are configured. |
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.
Should we use logs_index
for span events? Or maybe add another option for span events?
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.
It isn't intuitive to have logs_index as the configuration of destination of span events. What about a new config traces_span_events_index
(that is only applicable to otel mapping mode as it sends span events as separate documents)? It should default to empty, such that dynamic routing kicks in.
I have also considered
- using
logs_index
for span events. See reasoning below. - setting the default of
traces_span_events_index
aslogs_index
, but it seems to a big trap. Let's imagine if a user wants logs to be statically routed to anlogs_index
and traces dynamically routed. If the user sets logs_index, span events suddenly are not dynamically routed. This is not great.
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 we model span events as logs in OTel mode, I think it's only logical to use logs_index
. Ack that this may not be intuitive but documenting this seems like the solution. I think we should start simple by just using logs_index
and only adding something like traces_span_events_index
if necessary.
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.
sgtm. It is explicitly stated that it is not recommended to set logs_index
, metrics_index
and traces_index
anyway.
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.
Done in 61a3bfa
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.
Thank you, this is a nice simplification.
Telemetry data will be written to signal specific data streams by default: | ||
logs to `logs-generic-default`, metrics to `metrics-generic-default`, and traces to `traces-generic-default`. | ||
Documents are statically or dynamically routed to the target index / data stream in the following order. The first routing mode that applies will be used. | ||
1. "Static mode": To `logs_index` for log records, `metrics_index` for data points and `traces_index` for spans, if these configs are not empty respectively. In OTel mapping mode (`mapping::mode: otel`), span events are separate documents routed to `logs_index` if non-empty. |
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.
Perhaps instead of repeating the statement about otel mode span events, add a paragraph after the numbered list mentioning that in otel mode, span events are considered log records and routed as such?
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.
Done in 769edb2
…8458) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Breaking change: change default `mapping::mode` config to `otel` for the best user experience and the most intuitive document structure in Elasticsearch. See README to learn more about otel mapping mode. To retain the old behavior, explicitly set `mapping::mode` to `none`. Should be released together with #38500 <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes #37241 <!--Describe the documentation added.--> #### Documentation Updated README
Co-authored-by: Andrzej Stencel <[email protected]>
Co-authored-by: Andrzej Stencel <[email protected]>
Co-authored-by: Andrzej Stencel <[email protected]>
Co-authored-by: Andrzej Stencel <[email protected]>
Co-authored-by: Andrzej Stencel <[email protected]>
Co-authored-by: Andrzej Stencel <[email protected]>
@JaredTan95 any thoughts on it? |
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 is great, dynamic routing is more flexible and logically consistent
Description
Breaking change.
Overhaul in document routing. New document routing logic:
Effective changes:
{logs,metrics,traces}_dynamic_index
config no-op{logs,metrics,traces}_dynamic_index::enabled
and{logs,metrics,traces}_index
set at the same time, as users who rely on dynamic index should not set{logs,metrics,traces}_index
.elasticsearch.index.{prefix,suffix}
handling. Replace it withelasticsearch.index
handling that uses attribute value as index directly. Users rely on the previously supportedelasticsearch.index.prefix
andelasticsearch.index.suffix
should migrate to a transform processor that setselasticsearch.index
.Should be released together with #38458
Link to tracking issue
Fixes #38361
Testing
Documentation